Hi, I've been looking at last year's change to dentry refcounting which sets the refcount to -128 (mark_dead()) when the dentry is gone. As this is an "unsigned long" and there are several places where d_lockref.count is compared e.g. "> 1", I start feeling uncomfortable, as "-128" is greater than "1". Most of them look safe because there is locking in place and d_lockref.count will never be seen as "-128" unless you get the reference with only rcu_read_lock(). That brings me to dget_parent(). It only has rcu_read_lock() protection, and yet uses lockref_get_not_zero(). This doesn't seem safe. Is there a reason that it is safe that I'm not seeing? Or is the following needed? Thanks, NeilBrown Signed-off-by: NeilBrown <neilb@xxxxxxx> diff --git a/fs/dcache.c b/fs/dcache.c index 06f65857a855..c48ce95a38f2 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -699,7 +699,7 @@ struct dentry *dget_parent(struct dentry *dentry) */ rcu_read_lock(); ret = ACCESS_ONCE(dentry->d_parent); - gotref = lockref_get_not_zero(&ret->d_lockref); + gotref = lockref_get_not_dead(&ret->d_lockref); rcu_read_unlock(); if (likely(gotref)) { if (likely(ret == ACCESS_ONCE(dentry->d_parent)))
Attachment:
signature.asc
Description: PGP signature