lkml.org 
[lkml]   [2004]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH 2.6] sysfs_remove_dir Vs dcache_readdir
Hello,

I have re-done the patch fixing the race between sysfs_remove_dir() and
dcache_readdir(). If you recall, sysfs_remove_dir(kobj) manipulates the
->d_subdirs list for the dentry corresponding to the sysfs directory being
removed. It can end up deleting the cursor dentry which is added to the
->d_subdirs list during a concurrent dcache_dir_open() ==> dcache_readdir() for
the same directory. And as a result dcache_readdir() can loop for ever holding
dcache_lock.

The earlier patch which was included in -mm1 created problems which resulted
in list_del() BUG hits in prune_dcache(). The reason I think is that in the
main loop in sysfs_remove_dir(), dcache_lock is dropped and re-acquired, and
this could result in inconsistent ->d_subdirs list and prune_dcache() may try
to delete an already deleted dentry. I have corrected this in the new
patch as below.

I could do sysfs_remove_dir() more neatly on sysfs backing store patch set
as there I don't use the ->d_subdirs list. Instead the list of children
sysfs_dirent works out well. But untill sysfs backing store patch is picked
up the existing code suffer from this race. This can be easily tested by
running following two loops on a SMP box

# while true; do insmod drivers/net/dummy.ko; rmmod dummy; done
# while true; do find /sys/class/net > /dev/null; done

Please review the patch below.

Thanks
Manneesh

===============================================================================


o This patch fixes sysfs_remove_dir race with dcache_readdir. There is
no need for sysfs_remove_dir to modify the d_subdirs list for the directory
being deleted as it is taken care in the final dput. Modifying this list
results in inconsistent d_subdirs list and causes infinite loop in
concurrently occurring dcache_readdir.

o The main loop is restarted every time, dcache_lock is re-acquired in order
to maintain consistency.


fs/sysfs/dir.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff -puN fs/sysfs/dir.c~sysfs_remove_dir-race-fix fs/sysfs/dir.c
--- linux-2.6.3-rc4/fs/sysfs/dir.c~sysfs_remove_dir-race-fix 2004-02-17 11:18:38.000000000 +0530
+++ linux-2.6.3-rc4-maneesh/fs/sysfs/dir.c 2004-02-17 12:21:31.000000000 +0530
@@ -120,13 +120,14 @@ void sysfs_remove_dir(struct kobject * k
down(&dentry->d_inode->i_sem);

spin_lock(&dcache_lock);
+restart:
node = dentry->d_subdirs.next;
while (node != &dentry->d_subdirs) {
struct dentry * d = list_entry(node,struct dentry,d_child);
- list_del_init(node);

+ node = node->next;
pr_debug(" o %s (%d): ",d->d_name.name,atomic_read(&d->d_count));
- if (d->d_inode) {
+ if (!d_unhashed(d) && (d->d_inode)) {
d = dget_locked(d);
pr_debug("removing");

@@ -137,12 +138,12 @@ void sysfs_remove_dir(struct kobject * k
d_delete(d);
simple_unlink(dentry->d_inode,d);
dput(d);
+ pr_debug(" done\n");
spin_lock(&dcache_lock);
+ /* re-acquired dcache_lock, need to restart */
+ goto restart;
}
- pr_debug(" done\n");
- node = dentry->d_subdirs.next;
}
- list_del_init(&dentry->d_child);
spin_unlock(&dcache_lock);
up(&dentry->d_inode->i_sem);

_
-
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 14:01    [W:0.051 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site