Skip to contentSkip to conversation

2.4 devfs deadlock on concurrent lookups on non-existent entry

PATCH Superseded Andrey Borzenkov 1 participant 1 message May 7, 2003

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 2.4 devfs deadlock on concurrent lookups on non-existent entry

This problem has first been reported for over two years ago. Usually it happened during system boot using RH initscripts with two minilogds hanging on access to /dev/log and blocking rc.sysinit; this condition was triggered by unrelated bug in minilogd.

Pavel Roskin provided detailed debug info for this problem; details including stack are available at
<https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=85621>. It turned out a deadlock between devfs_lookup and devfs_d_revalidate_wait for cases when ->d_revalidate was called under parent->i_sem (mostly all places where lookup_hash was used). The deadlock looked like:

                               path_lookup("dev/log", LOOKUP_PARENT, &nd);
                                -> yields dentry for  </dev>

path_lookup("dev/log", LOOKUP_PARENT, &nd);
-> yields dentry for </dev>
down(</dev>->i_sem) holds i_sem

                                        down(</dev>->i_sem) - sleeps
lookup_hash("log", </dev>)                          .
devfs_lookup(</dev>, "log")                         .
   MISS                                             .
try_modload
set "log"->d_op to &devfs_wait_dops;                .
init "log"->wait_queue                              .
up(</dev>->i_sem)                                   .
                                                    .
                                             obtains i_sem
                                 lookup_hash("log", </dev>);
                                 cached_lookup(</dev>, "log", 0)
                                 devfs_d_revalidate_wait("log", 0)
                                 wait on "log"->wait_queue
                                ... waits to be waked up by devfs_lookup

down(</dev>->i_sem)

the patch fixes it by moving i_sem re-acquire after wake_up(&lookup_info.wait_queue). It does not look like it adds any additional races. Please check.

Pavel said it applies unchanged to 2.5 tree. Which may indicate it has the same race condition. I do not have 2.5 available.

please consider for 2.4.21

regards

-andrey

fs/devfs/base.c renamed from fs/devfs/base.c.minilogd +1 -1
@@ -3038,7 +3038,6 @@ static struct dentry *devfs_lookup (stru
 	revalidation  */
     up (&dir->i_sem);
     wait_for_devfsd_finished (fs_info);  /*  If I'm not devfsd, must wait  */
-    down (&dir->i_sem);      /*  Grab it again because them's the rules  */
     de = lookup_info.de;
     /*  If someone else has been so kind as to make the inode, we go home
 	early  */
@@ -3068,6 +3067,7 @@ out:
     write_lock (&parent->u.dir.lock);
     wake_up (&lookup_info.wait_queue);
     write_unlock (&parent->u.dir.lock);
+    down (&dir->i_sem);      /*  Grab it again because them's the rules  */
     devfs_put (de);
     return retval;
 }   /*  End Function devfs_lookup  */