Messages in this thread |  | | Date | Mon, 29 Oct 2001 16:22:26 +1100 | From | Richard Gooch <> | Subject | Re: [2.4.13] [devfs 0.119 (20011009)] base.c: devfsd_ioctl |
| |
Kari Hurtta writes: > > Can you say where I have wrong? > > Code on devfsd_ioctl() [base.c]: > > if (fs_info->devfsd_task == NULL) > { > if ( !spin_trylock (&lock) ) return -EBUSY; > fs_info->devfsd_task = current; > spin_unlock (&lock); > > To me that spinlock looks like it is useless. > Either > > 1) If it is mean that lock protects two CPUs settting > fs_info->devfsd_task when another is set it, > then test about fs_info->devfsd_task == NULL > should be inside of locked code > or 2) this is protected with some other lock as comment > perhaps indicates: > > /* Ensure only one reader has access to the queue. This scheme will > work even if the global kernel lock were to be removed, because it > doesn't matter who gets in first, as long as only one gets it > */ > if (fs_info->devfsd_task == NULL) > { > if ( !spin_trylock (&lock) ) return -EBUSY; > > Should this be: > > --- fs/devfs/base.c.old Thu Oct 11 09:23:24 2001 > +++ fs/devfs/base.c Sun Oct 28 14:59:03 2001 > @@ -3227,6 +3227,11 @@ > if (fs_info->devfsd_task == NULL) > { > if ( !spin_trylock (&lock) ) return -EBUSY; > + if (fs_info->devfsd_task != NULL) { > + /* We lost race ... */ > + spin_unlock (&lock); > + return -EBUSY; > + } > fs_info->devfsd_task = current; > spin_unlock (&lock); > fs_info->devfsd_file = file;
Damn! You're right. I broke that when I simplified the anti-race code back in June and replaced the #ifdef CONFIG_SMP bits with plain spinlocks. Unfortunately I didn't put back in a critcal line. Cut-and-paste error without the paste :-(
OK, I've updated my both my trees. I'll feed this to Linus in my next release.
Regards,
Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |