On Thu, Aug 14, 2025 at 4:08 AM NeilBrown <n...@brown.name> wrote: > > On Wed, 13 Aug 2025, Al Viro wrote: > > On Tue, Aug 12, 2025 at 12:25:14PM +1000, NeilBrown wrote: > > > Several filesystems use the results of readdir to prime the dcache. > > > These filesystems use d_alloc_parallel() which can block if there is a > > > concurrent lookup. Blocking in that case is pointless as the lookup > > > will add info to the dcache and there is no value in the readdir waiting > > > to see if it should add the info too. > > > > > > Also these calls to d_alloc_parallel() are made while the parent > > > directory is locked. A proposed change to locking will lock the parent > > > later, after d_alloc_parallel(). This means it won't be safe to wait in > > > d_alloc_parallel() while holding the directory lock. > > > > > > So this patch introduces d_alloc_noblock() which doesn't block > > > but instead returns ERR_PTR(-EWOULDBLOCK). Filesystems that prime the > > > dcache now use that and ignore -EWOULDBLOCK errors as harmless. > > > > > > A few filesystems need more than -EWOULDBLOCK - they need to be able to > > > create the missing dentry within the readdir. procfs is a good example > > > as the inode number is not known until the lookup completes, so readdir > > > must perform a full lookup. > > > > > > For these filesystems d_alloc_locked() is provided. It will return a > > > dentry which is already d_in_lookup() but will also lock it against > > > concurrent lookup. The filesystem's ->lookup function must co-operate > > > by calling lock_lookup() before proceeding with the lookup. This way we > > > can ensure exclusion between a lookup performed in ->iterate_shared and > > > a lookup performed in ->lookup. Currently this exclusion is provided by > > > waiting in d_wait_lookup(). The proposed changed to dir locking will > > > mean that calling d_wait_lookup() (in readdir) while already holding > > > i_rwsem could deadlock. > > > > The last one is playing fast and loose with one assertion that is used > > in quite a few places in correctness proofs - that the only thing other > > threads do to in-lookup dentries is waiting on them (and that - only > > in d_wait_lookup()). I can't tell whether it will be a problem without > > seeing what you do in the users of that thing, but that creates an > > unpleasant areas to watch out for in the future ;-/ > > Yeah, it's not my favourite part of the series. > > > > > Which filesystems are those, aside of procfs? > > > > afs in afs_lookup_atsys(). While looking up a name that ends "@sys" it > need to look up the prefix with various alternate suffixes appended. > So this isn't readdir related, but is a lookup-within-a-lookup. > > The use of d_add_ci() in xfs is the same basic pattern. > > overlayfs does something in ovl_lookup_real_one() that I don't > understand yet but it seems to need a lookup while the directory is > locked.
We decoded a connected real directory path (from file handle) and we are trying to lookup in overlay a directory that is referencing the underlying real dir that we decoded. This is the context. Not sure what problem exactly this code gives you. > > ovl_cache_update is in the ovl iterate_shared code (which in fact holds > an exclusive lock). I think this is the same pattern as procfs in that > an inode number needs to be allocated at lookup time, but there might be > more too it. > It's kind of a hack I guess. ovl has those rules (see xino) to compose a consistent inode number from real inode number and layer number. lookup of children during readdir composes the child stack to realize the consistent xino. We could do this internally in ovl by doing lookups on the real layers and composing the xino, but calling lookup on ovl during readdir was so much easier :/ Thanks, Amir.
