Skip to content

Commit

Permalink
vfs: reorganize dput() memory accesses
Browse files Browse the repository at this point in the history
This is me being a bit OCD after all the dentry optimization work this
merge window: profiles end up showing 'dput()' as a rather expensive
operation, and there were two unrelated bad reasons for that.

The first reason was reading d_lockref.count for debugging purposes,
which touches the lockref cacheline (for reads) before really need to.
More importantly, the debugging test in question is _wrong_, and has
hidden bugs.  It's true that we can only sleep when the count goes down
to zero, but the test as-is hides the much more subtle bug that happens
if we race with somebody else deleting the file.

Anyway we _will_ touch that cacheline, but let's do it for a write and
in the right routine (ie in "lockref_put_or_lock()") which annotates the
costs better.  So remove the misleading debug code.

The other was an unnecessary access to the cacheline that contains the
d_lru list, just to check whether we already were on the LRU list or
not.  This is exactly what we have d_flags for, so that we can avoid
touching extra cache lines for the common case.  So just add another bit
for "is this dentry on the LRU".

Finally, mark the tests properly likely/unlikely, so that the common
fast-paths are dense in the instruction stream.

This makes the profiles look much saner.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Linus Torvalds committed Sep 8, 2013
1 parent b409624 commit 8aab6a2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
20 changes: 10 additions & 10 deletions fs/dcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,9 @@ static void dentry_unlink_inode(struct dentry * dentry)
*/
static void dentry_lru_add(struct dentry *dentry)
{
if (list_empty(&dentry->d_lru)) {
if (unlikely(!(dentry->d_flags & DCACHE_LRU_LIST))) {
spin_lock(&dcache_lru_lock);
dentry->d_flags |= DCACHE_LRU_LIST;
list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
dentry->d_sb->s_nr_dentry_unused++;
dentry_stat.nr_unused++;
Expand All @@ -320,7 +321,7 @@ static void dentry_lru_add(struct dentry *dentry)
static void __dentry_lru_del(struct dentry *dentry)
{
list_del_init(&dentry->d_lru);
dentry->d_flags &= ~DCACHE_SHRINK_LIST;
dentry->d_flags &= ~(DCACHE_SHRINK_LIST | DCACHE_LRU_LIST);
dentry->d_sb->s_nr_dentry_unused--;
dentry_stat.nr_unused--;
}
Expand All @@ -341,6 +342,7 @@ static void dentry_lru_move_list(struct dentry *dentry, struct list_head *list)
{
spin_lock(&dcache_lru_lock);
if (list_empty(&dentry->d_lru)) {
dentry->d_flags |= DCACHE_LRU_LIST;
list_add_tail(&dentry->d_lru, list);
dentry->d_sb->s_nr_dentry_unused++;
dentry_stat.nr_unused++;
Expand Down Expand Up @@ -509,24 +511,22 @@ static inline struct dentry *dentry_kill(struct dentry *dentry, int ref)
*/
void dput(struct dentry *dentry)
{
if (!dentry)
if (unlikely(!dentry))
return;

repeat:
if (dentry->d_lockref.count == 1)
might_sleep();
if (lockref_put_or_lock(&dentry->d_lockref))
return;

if (dentry->d_flags & DCACHE_OP_DELETE) {
/* Unreachable? Get rid of it */
if (unlikely(d_unhashed(dentry)))
goto kill_it;

if (unlikely(dentry->d_flags & DCACHE_OP_DELETE)) {
if (dentry->d_op->d_delete(dentry))
goto kill_it;
}

/* Unreachable? Get rid of it */
if (d_unhashed(dentry))
goto kill_it;

dentry->d_flags |= DCACHE_REFERENCED;
dentry_lru_add(dentry);

Expand Down
1 change: 1 addition & 0 deletions include/linux/dcache.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ struct dentry_operations {
#define DCACHE_MANAGED_DENTRY \
(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)

#define DCACHE_LRU_LIST 0x80000
#define DCACHE_DENTRY_KILLED 0x100000

extern seqlock_t rename_lock;
Expand Down

0 comments on commit 8aab6a2

Please sign in to comment.