On Sun, Jun 19 2022, Visa Hankala <v...@hankala.org> wrote: > On Sun, Jun 19, 2022 at 11:05:38AM +0200, Jeremie Courreges-Anglas wrote: >> On Fri, Jun 17 2022, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: >> > On Thu, Jun 16 2022, Visa Hankala <v...@hankala.org> wrote: >> >> nfs_inactive() has a lock order reversal. When it removes the silly >> >> file, it locks the directory vnode while it already holds the lock >> >> of the argument file vnode. This clashes for example with name lookups >> >> where directory vnodes are locked before file vnodes. >> >> >> >> The reversal can cause a deadlock when an NFS client has multiple >> >> processes that create, modify and remove files in the same >> >> NFS directory. >> >> >> >> The following patch makes the silly file removal happen after >> >> nfs_inactive() has released the file vnode lock. This should be safe >> >> because the silly file removal is independent of nfs_inactive()'s >> >> argument vnode. >> >> The diff makes sense to me. Did you spot it reviewing the code, or >> using WITNESS? > > I noticed it by code review. > > WITNESS is somewhat helpless with vnode locks because they can involve > multiple levels of lock nesting. In fact, the order checking between > vnodes has been disabled by initializing the locks with RWL_IS_VNODE. > To fix this, the kernel would have to pass nesting information around > the filesystem code. > > This particular deadlock can be triggered for example by quickly > writing and removing temporary files in an NFS directory using one > process while another process lists the directory contents repeatedly.
Dunno how many other problems you may have spotted, but this one looks obvious. Thanks and ok jca@ -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
