Skip to content

Commit 504e08c

Browse files
author
Al Viro
committed
fast_dput(): handle underflows gracefully
If refcount is less than 1, we should just warn, unlock dentry and return true, so that the caller doesn't try to do anything else. Taking care of that leaves the rest of "lockref_put_return() has failed" case equivalent to "decrement refcount and rejoin the normal slow path after the point where we grab ->d_lock". NOTE: lockref_put_return() is strictly a fastpath thing - unlike the rest of lockref primitives, it does not contain a fallback. Caller (and it looks like fast_dput() is the only legitimate one in the entire kernel) has to do that itself. Reasons for lockref_put_return() failures: * ->d_lock held by somebody * refcount <= 0 * ... or an architecture not supporting lockref use of cmpxchg - sparc, anything non-SMP, config with spinlock debugging... We could add a fallback, but it would be a clumsy API - we'd have to distinguish between: (1) refcount > 1 - decremented, lock not held on return (2) refcount < 1 - left alone, probably no sense to hold the lock (3) refcount is 1, no cmphxcg - decremented, lock held on return (4) refcount is 1, cmphxcg supported - decremented, lock *NOT* held on return. We want to return with no lock held in case (4); that's the whole point of that thing. We very much do not want to have the fallback in case (3) return without a lock, since the caller might have to retake it in that case. So it wouldn't be more convenient than doing the fallback in the caller and it would be very easy to screw up, especially since the test coverage would suck - no way to test (3) and (4) on the same kernel build. Reviewed-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
1 parent 15220fb commit 504e08c

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

fs/dcache.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,12 +779,12 @@ static inline bool fast_dput(struct dentry *dentry)
779779
*/
780780
if (unlikely(ret < 0)) {
781781
spin_lock(&dentry->d_lock);
782-
if (dentry->d_lockref.count > 1) {
783-
dentry->d_lockref.count--;
782+
if (WARN_ON_ONCE(dentry->d_lockref.count <= 0)) {
784783
spin_unlock(&dentry->d_lock);
785784
return true;
786785
}
787-
return false;
786+
dentry->d_lockref.count--;
787+
goto locked;
788788
}
789789

790790
/*
@@ -842,6 +842,7 @@ static inline bool fast_dput(struct dentry *dentry)
842842
* else could have killed it and marked it dead. Either way, we
843843
* don't need to do anything else.
844844
*/
845+
locked:
845846
if (dentry->d_lockref.count) {
846847
spin_unlock(&dentry->d_lock);
847848
return true;

0 commit comments

Comments
 (0)