Re: spinlock protection on data question

[Jan Hudec <bulb@ucw.cz>]

On Wed, Aug 07, 2002 at 03:30:14PM -0400, anton wilson wrote:
> I have a array of data that is used globally and an array of locks to protect 
> them.
> 
> struct context_data data[10];
> spinlock_t locks[10];

Where does the length of the array come from? Shouln't it be a linked
list, tree, hash table... ? (That has nothing to do with locking)

> The problem I'm having is that to find out which spinlock to use to lock any 
> struct in that array, I have to first access the data like this:
> 
> num = mydata->num;
> spin_lock_irqaave( &locks[num], &flags);

then why not spin_lock(&mydata->lock)? Are the locks shared between more
structures? Then it would still be mydata->lock->lock (the first one
must be a structure holding a lock and refcount).

> but one of my driver functions frees that same data:
> 
> static void disconnect_rasid(struct usb_device *dev, void *drv_context)
> {
>    num = ((context_data *) drv_context)->num;
>    spin_lock_irqaave( &locks[num], &flags);
> 
>    kfree(drv_context);
> 
>    /* ....  */
> 
> }

Then you must have a lock protecting the array. Just the way there is
inode_lock and dcache_lock and.... Make a generic global

DECLARE_SPINLOCK(data_lock);

and then you have to:
access as:
spin_lock(&data_lock);
num=mydata->num;
spin_lock_irqaave( &locks[num], &flags);
spin_unlock(&data_lock);
/* You can release here immediately - ordering (locks[n] is always taken
 * under data_lock) is sufficient to avoid dead-lock, locking and then
 * unlocking is enough to serialize. */

And you must free undef data_lock and lock of given object. Like this:
spin_lock(&data_lock);
num = ((context_data *) drv_context)->num;
spin_lock_irqaave( &locks[num], &flags);
spin_unlock_irqrestore( &locks[num], &flags);
/* Noone new can aquire it while we hold data_lock! */
/* however here you MUST: */
data[num] = NULL;
/* or anything else that will tell users there is no such data */
spin_unlock(&data_lock);
kfree(drv_context);
...

> so i think that any other function that uses that data is going to have 
> problems if it is called at the same time as disconnect because, before it 
> can lock the data structure, it has to access the data structure to find out 
> where in the array of locks the spinlock is .. . if disconnect has freed that 
> data structure, it no longer exists and you're accessing a null pointer.
> 
> Any suggestions?
> 
> Anton 

Anyway. I would still prefer refcounting. When a user needs the data, it
increments counter on the data and drop the data_lock. Only data with
zero counter can be freed. That way you can make operations on the data
concurrent to some extent (you still need to lock when changing the
data).

-------------------------------------------------------------------------------
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--
Kernelnewbies: Help each other learn about the Linux kernel.
Archive:       http://mail.nl.linux.org/kernelnewbies/
FAQ:           http://kernelnewbies.org/faq/