[PATCH] sysfs_remove_dir-vs-dcache_readdir race fix

From: Maneesh Soni <maneesh@in.ibm.com>

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

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.

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index ebaf6c40671af75962c4c946f5ff8898f9ee3cd3..add2045e8c2585801db7b9107dc9c00e870af941 100644 (file)
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -120,13 +120,14 @@ void sysfs_remove_dir(struct kobject * kobj)
        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 * kobj)
                        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);