lkml.org 
[lkml]   [2001]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 13:11    [W:0.050 / U:2.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site