[PATCH RFC/RFT v1 00/12] Fix easy bits of the negative dentry problem

NeilBrown <[email protected]>
Newsgroups gmane.linux.file-systems,gmane.linux.kernel.autofs,gmane.linux.kernel,gmane.comp.file-systems.ceph.devel,gmane.comp.file-systems.coda.general,gmane.linux.nfs
Message-ID <[email protected]>
Hi all,

 I was reading about the negative dentry problem in
 https://lwn.net/Articles/1079407/ and thought I would have ago at the
 easy bits.  Given how involved some of that was I'm glad I didn't try
 the tricky bits!

 So I didn't try to reduce the number of negative dentries or handle
 them differently to positive dentries.  I just tried to address
 specific symptoms.

 This series addresses two reported symptoms.
  1/ refcounts on parents can potentially overflow if there are billions
     of (negative) children.  This was the easiest easy bit.  The first
     patch is all that is needed.

  2/ Walking the ->d_children list can take arbitrarily long and can
     trigger soft lockups.  I haven't addresses the "arbitrarily long"
     part, but the "soft lockups" only takes another 11 patches.
     With these in place I would feel more confident addressing
     the "arbitrarily long" part.

 The core idea is to use a common helper for all walking of
 ->d_children, and teach it to drop the lock and schedule when needed,
 inserting a cursor to keep its place.
 This simple idea requires:
   - nothing should be holding any other lock while walking ->d_children
   - dentries shouldn't be moved around needlessly, else things might
     get missed.
   - quite a bit of refactoring in libfs for readdir code.

 so I have probably missed some bits and would be very happy to be told
 what I have missed.
 I would also be very happy if someone who can trigger the soft lockups
 can try this code and confirm that the locks are gone, and nothing bad
 happens.
 I have done some basic testing including the "notify" parts of
 ltp, but that is all.

 I haven't addressed d_walk() in this series - I thought it was big
 enough for now.  Most of d_walk() already schedules as needed, but I
 think there is still room for improvement, but that can come later.

Details worth checking in various modules:

NFS: patch 02 drops the parent lock between calling
        nfs_unset_verifier_delegated()
     on the parent and calling it on all the children.  I don't
     think this is significant.

autofs:  patch 09 removes the ->lookup_lock from
             get_next_positive_subdir() 
         and
             get_next_positive_dentry()
         I cannot see that this is serving any purpose

cephfs:  nothing interesting
coda:  patch 02 dropped the rcu_read_lock() because the spinlock() implies it.

libfs: the offset_dir code has been changed (08) quite a bit but should
      have identical behaviour.  It now uses scan_positives() (which has
      been substantially refactored (05, 06, 07, 12)) in place of
      find_positive_dentry()

fsnotify: a few changes (03 04) to move the ->d_children walk out from under
     various spinlocks.  This is the change I'm least confident of
     as it is non-trivial code that I'm not familiar with.

dcache: patch 11 uses an on-stack dentry as a dcache cursor.  This is
     not something that other code does so I would appreciate it if
     someone checked my logic.

dcache: patch 10 changed d_move to *not* move dentries in the
     ->d_children list when they are both in the same parent.
     This brings no value and could cause some instability in
     a d_children walk.

Thanks for any review or testing that you have time for.

NeilBrown

This series is against v7.2-rc6 and can be found at the 
   vfs-neg-dentries
branch of
   https://github.com/neilbrown/linux
where top commit is
   56768a600dd9145fea601649dbc74031aa25e459

 [PATCH v1 01/12] VFS: don't count references through ->d_parent
 [PATCH v1 02/12] Add and use d_for_each_positive_child family of
 [PATCH v1 03/12] fsnotify: don't hold a spin_lock across
 [PATCH v1 04/12] fsnotify: reduce i_lock hold time in
 [PATCH v1 05/12] libfs: simplify scan_positives()
 [PATCH v1 06/12] libfs: change scan_positives() to use
 [PATCH v1 07/12] libfs: allow scan_positives() to be called without a
 [PATCH v1 08/12] libfs: replace find_positive_dentry() with
 [PATCH v1 09/12] autofs: don't hold ->lookup_lock in
 [PATCH v1 10/12] VFS: don't move dentries in d_sib list when they
 [PATCH v1 11/12] Call cond_reshed() as needed in
 [PATCH v1 12/12] libfs: remove cond_resched() from scan_positives()
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.