| Message ID | 20231109062056.3181775-8-viro@zeniv.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/22] struct dentry: get rid of randomize_layout idiocy | expand |
On Thu, Nov 09, 2023 at 06:20:42AM +0000, Al Viro wrote: > ->d_delete() is a way for filesystem to tell that dentry is not worth > keeping cached. It is not guaranteed to be called every time a dentry > has refcount drop down to zero; it is not guaranteed to be called before > dentry gets evicted. In other words, it is not suitable for any kind > of keeping track of dentry state. > > None of the in-tree filesystems attempt to use it that way, fortunately. > > So the contortions done by fast_dput() (as well as dentry_kill()) are > not warranted. fast_dput() certainly should treat having ->d_delete() > instance as "can't assume we'll be keeping it", but that's not different > from the way we treat e.g. DCACHE_DONTCACHE (which is rather similar > to making ->d_delete() returns true when called). > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- Reasoning seems sane to me, Reviewed-by: Christian Brauner <brauner@kernel.org>
diff --git a/fs/dcache.c b/fs/dcache.c index 5371f32eb4bb..0d15e8852ac1 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -768,15 +768,7 @@ static inline bool fast_dput(struct dentry *dentry) unsigned int d_flags; /* - * If we have a d_op->d_delete() operation, we sould not - * let the dentry count go to zero, so use "put_or_lock". - */ - if (unlikely(dentry->d_flags & DCACHE_OP_DELETE)) - return lockref_put_or_lock(&dentry->d_lockref); - - /* - * .. otherwise, we can try to just decrement the - * lockref optimistically. + * try to decrement the lockref optimistically. */ ret = lockref_put_return(&dentry->d_lockref); @@ -830,7 +822,7 @@ static inline bool fast_dput(struct dentry *dentry) */ smp_rmb(); d_flags = READ_ONCE(dentry->d_flags); - d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | + d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_OP_DELETE | DCACHE_DISCONNECTED | DCACHE_DONTCACHE; /* Nothing to do? Dropping the reference was all we needed? */
->d_delete() is a way for filesystem to tell that dentry is not worth keeping cached. It is not guaranteed to be called every time a dentry has refcount drop down to zero; it is not guaranteed to be called before dentry gets evicted. In other words, it is not suitable for any kind of keeping track of dentry state. None of the in-tree filesystems attempt to use it that way, fortunately. So the contortions done by fast_dput() (as well as dentry_kill()) are not warranted. fast_dput() certainly should treat having ->d_delete() instance as "can't assume we'll be keeping it", but that's not different from the way we treat e.g. DCACHE_DONTCACHE (which is rather similar to making ->d_delete() returns true when called). Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/dcache.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)