Skip to contentSkip to conversation

fix waitqueue leak in devfs_d_revalidate_wait

PATCH In review Christoph Hellwig 3 participants 3 messages Mar 18, 2003

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix waitqueue leak in devfs_d_revalidate_wait

devfs_d_revalidate_wait adds to a waitqueue but never removes from it again so we there's one entry full of reused stack space added on each call (I wonder how this ever worked).

The function has a few more bugs (it effectivly does a sleep_on instead of checking for the actual even and can't deal with negative dentries at all), but I just had breakfast and don't want to poke into devfs internals deeper - I still hope Adam's smalldevfs will get merged anyway..

fs/devfs/base.c +3 -0
@@ -2336,6 +2232,8 @@
     wait_queue_head_t wait_queue;
 };
 
+/* XXX: this doesn't handle the case where we got a negative dentry
+        but a devfs entry has been registered in the meanwhile */
 static int devfs_d_revalidate_wait (struct dentry *dentry, int flags)
 {
     struct inode *dir = dentry->d_parent->d_inode;
@@ -2380,6 +2278,7 @@
 	add_wait_queue (&lookup_info->wait_queue, &wait);
 	read_unlock (&parent->u.dir.lock);
 	schedule ();
+	remove_wait_queue (&lookup_info->wait_queue, &wait);
     }
     else read_unlock (&parent->u.dir.lock);
     return 1;
#1 lore raw
Quoted 2 lines · Christoph Hellwig

I still hope Adam's smalldevfs will get merged anyway..

Well me too, but smalldevfs is a bit stalled at present.

It seems to work OK now, but there are a few small incompatibilities with the current devfs. People need to make adjustments to their initscripts, or to untar a tarball-of-device-nodes into smalldevfs at boot time.

Like all other kernel developers I do not use devfs and am not really able to evaluate how serious these problems will be for people.

Adam is disinclined to address these administrative incompatibilites in his user-space tools. smalldevfs is presently at risk of getting dropped out.

#2 lore raw

Andrew Morton wrote:
[...]

Quoted 3 lines

It seems to work OK now, but there are a few small incompatibilities with the current devfs. People need to make adjustments to their initscripts, or to untar a tarball-of-device-nodes into smalldevfs at boot time.

I assume smalldevfs is what you get when you configure devfs in 2.5.64-mm8? It works fine for me, on smp with debian testing. I changed nothing at all, and noticed that devfsd didn't run. But it don't seem necessary, it seems to work even without those compatibility symlinks.

Quoted 2 lines

Adam is disinclined to address these administrative incompatibilites in his user-space tools. smalldevfs is presently at risk of getting dropped out.

Please continue, it works and I understand the code is an improvement. I'll begin testing on my work pc too.

Helge Hafting