Extract the common code to remove a dentry from the lru into a new function
dentry_lru_remove().
Two call sites used list_del() instead of list_del_init(). AFAIK the
performance of both is the same. dentry_lru_remove() does a list_del_init().
As a result dentry->d_lru is now always empty when a dentry is freed.
A consistent state is useful to establish dentry state from slab defrag.
[This patch is already in mm]
Reviewed-by: Rik van Riel <[email protected]>
Signed-off-by: Christoph Lameter <[email protected]>
---
fs/dcache.c | 42 ++++++++++++++----------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2007-10-25 18:28:40.000000000 -0700
+++ linux-2.6/fs/dcache.c 2007-11-06 12:56:31.000000000 -0800
@@ -95,6 +95,14 @@ static void d_free(struct dentry *dentry
call_rcu(&dentry->d_u.d_rcu, d_callback);
}
+static void dentry_lru_remove(struct dentry *dentry)
+{
+ if (!list_empty(&dentry->d_lru)) {
+ list_del_init(&dentry->d_lru);
+ dentry_stat.nr_unused--;
+ }
+}
+
/*
* Release the dentry's inode, using the filesystem
* d_iput() operation if defined.
@@ -211,13 +219,7 @@ repeat:
unhash_it:
__d_drop(dentry);
kill_it:
- /* If dentry was on d_lru list
- * delete it from there
- */
- if (!list_empty(&dentry->d_lru)) {
- list_del(&dentry->d_lru);
- dentry_stat.nr_unused--;
- }
+ dentry_lru_remove(dentry);
dentry = d_kill(dentry);
if (dentry)
goto repeat;
@@ -285,10 +287,7 @@ int d_invalidate(struct dentry * dentry)
static inline struct dentry * __dget_locked(struct dentry *dentry)
{
atomic_inc(&dentry->d_count);
- if (!list_empty(&dentry->d_lru)) {
- dentry_stat.nr_unused--;
- list_del_init(&dentry->d_lru);
- }
+ dentry_lru_remove(dentry);
return dentry;
}
@@ -404,10 +403,7 @@ static void prune_one_dentry(struct dent
if (dentry->d_op && dentry->d_op->d_delete)
dentry->d_op->d_delete(dentry);
- if (!list_empty(&dentry->d_lru)) {
- list_del(&dentry->d_lru);
- dentry_stat.nr_unused--;
- }
+ dentry_lru_remove(dentry);
__d_drop(dentry);
dentry = d_kill(dentry);
spin_lock(&dcache_lock);
@@ -596,10 +592,7 @@ static void shrink_dcache_for_umount_sub
/* detach this root from the system */
spin_lock(&dcache_lock);
- if (!list_empty(&dentry->d_lru)) {
- dentry_stat.nr_unused--;
- list_del_init(&dentry->d_lru);
- }
+ dentry_lru_remove(dentry);
__d_drop(dentry);
spin_unlock(&dcache_lock);
@@ -613,11 +606,7 @@ static void shrink_dcache_for_umount_sub
spin_lock(&dcache_lock);
list_for_each_entry(loop, &dentry->d_subdirs,
d_u.d_child) {
- if (!list_empty(&loop->d_lru)) {
- dentry_stat.nr_unused--;
- list_del_init(&loop->d_lru);
- }
-
+ dentry_lru_remove(dentry);
__d_drop(loop);
cond_resched_lock(&dcache_lock);
}
@@ -799,10 +788,7 @@ resume:
struct dentry *dentry = list_entry(tmp, struct dentry, d_u.d_child);
next = tmp->next;
- if (!list_empty(&dentry->d_lru)) {
- dentry_stat.nr_unused--;
- list_del_init(&dentry->d_lru);
- }
+ dentry_lru_remove(dentry);
/*
* move only zero ref count dentries to the end
* of the unused list for prune_dcache
--
Hi Christoph,
On Tue, Nov 06, 2007 at 05:11:34PM -0800, Christoph Lameter wrote:
> @@ -613,11 +606,7 @@ static void shrink_dcache_for_umount_sub
> spin_lock(&dcache_lock);
> list_for_each_entry(loop, &dentry->d_subdirs,
> d_u.d_child) {
> - if (!list_empty(&loop->d_lru)) {
> - dentry_stat.nr_unused--;
> - list_del_init(&loop->d_lru);
> - }
> -
> + dentry_lru_remove(dentry);
Shouldn't this be dentry_lru_remove(loop)?
Hannes
On Wed, 7 November 2007 09:50:27 +0100, Johannes Weiner wrote:
> On Tue, Nov 06, 2007 at 05:11:34PM -0800, Christoph Lameter wrote:
> > @@ -613,11 +606,7 @@ static void shrink_dcache_for_umount_sub
> > spin_lock(&dcache_lock);
> > list_for_each_entry(loop, &dentry->d_subdirs,
> > d_u.d_child) {
> > - if (!list_empty(&loop->d_lru)) {
> > - dentry_stat.nr_unused--;
> > - list_del_init(&loop->d_lru);
> > - }
> > -
> > + dentry_lru_remove(dentry);
>
> Shouldn't this be dentry_lru_remove(loop)?
Looks like it. Once this is fixed, feel free to add
Acked-by: Joern Engel <[email protected]>
Jörn
--
It does not require a majority to prevail, but rather an irate,
tireless minority keen to set brush fires in people's minds.
-- Samuel Adams
On Wed, 7 Nov 2007, Johannes Weiner wrote:
> Hi Christoph,
>
> On Tue, Nov 06, 2007 at 05:11:34PM -0800, Christoph Lameter wrote:
> > @@ -613,11 +606,7 @@ static void shrink_dcache_for_umount_sub
> > spin_lock(&dcache_lock);
> > list_for_each_entry(loop, &dentry->d_subdirs,
> > d_u.d_child) {
> > - if (!list_empty(&loop->d_lru)) {
> > - dentry_stat.nr_unused--;
> > - list_del_init(&loop->d_lru);
> > - }
> > -
> > + dentry_lru_remove(dentry);
>
> Shouldn't this be dentry_lru_remove(loop)?
Correct. Andrew: This needs to go into your tree to fix the patch that is
already there:
[PATCH] dcache: use the correct variable.
We need to use "loop" instead of "dentry"
Signed-off-by: Christoph Lameter <[email protected]>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2007-11-07 10:26:20.000000000 -0800
+++ linux-2.6/fs/dcache.c 2007-11-07 10:26:27.000000000 -0800
@@ -610,7 +610,7 @@ static void shrink_dcache_for_umount_sub
spin_lock(&dcache_lock);
list_for_each_entry(loop, &dentry->d_subdirs,
d_u.d_child) {
- dentry_lru_remove(dentry);
+ dentry_lru_remove(loop);
__d_drop(loop);
cond_resched_lock(&dcache_lock);
}
On Wed, 7 Nov 2007, J?rn Engel wrote:
> Looks like it. Once this is fixed, feel free to add
> Acked-by: Joern Engel <[email protected]>
Its in the slab defrag git now. I added the spelling fix and this one as a
result of the discussions today.
From: Christoph Lameter <[email protected]>
Subject: dcache: use the correct variable.
We need to use "loop" instead of "dentry"
Acked-by: Joern Engel <[email protected]>
Signed-off-by: Christoph Lameter <[email protected]>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2007-11-07 10:26:20.000000000 -0800
+++ linux-2.6/fs/dcache.c 2007-11-07 10:26:27.000000000 -0800
@@ -610,7 +610,7 @@ static void shrink_dcache_for_umount_sub
spin_lock(&dcache_lock);
list_for_each_entry(loop, &dentry->d_subdirs,
d_u.d_child) {
- dentry_lru_remove(dentry);
+ dentry_lru_remove(loop);
__d_drop(loop);
cond_resched_lock(&dcache_lock);
}
On Wed, 7 November 2007 10:55:09 -0800, Christoph Lameter wrote:
>
> From: Christoph Lameter <[email protected]>
> Subject: dcache: use the correct variable.
>
> We need to use "loop" instead of "dentry"
>
> Acked-by: Joern Engel <[email protected]>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> Index: linux-2.6/fs/dcache.c
> ===================================================================
> --- linux-2.6.orig/fs/dcache.c 2007-11-07 10:26:20.000000000 -0800
> +++ linux-2.6/fs/dcache.c 2007-11-07 10:26:27.000000000 -0800
> @@ -610,7 +610,7 @@ static void shrink_dcache_for_umount_sub
> spin_lock(&dcache_lock);
> list_for_each_entry(loop, &dentry->d_subdirs,
> d_u.d_child) {
> - dentry_lru_remove(dentry);
> + dentry_lru_remove(loop);
> __d_drop(loop);
> cond_resched_lock(&dcache_lock);
> }
Erm - wouldn't this break git-bisect?
Jörn
--
Joern's library part 5:
http://www.faqs.org/faqs/compression-faq/part2/section-9.html
On Wed, 7 Nov 2007, J?rn Engel wrote:
> > Acked-by: Joern Engel <[email protected]>
> > Signed-off-by: Christoph Lameter <[email protected]>
> >
> > Index: linux-2.6/fs/dcache.c
> > ===================================================================
> > --- linux-2.6.orig/fs/dcache.c 2007-11-07 10:26:20.000000000 -0800
> > +++ linux-2.6/fs/dcache.c 2007-11-07 10:26:27.000000000 -0800
> > @@ -610,7 +610,7 @@ static void shrink_dcache_for_umount_sub
> > spin_lock(&dcache_lock);
> > list_for_each_entry(loop, &dentry->d_subdirs,
> > d_u.d_child) {
> > - dentry_lru_remove(dentry);
> > + dentry_lru_remove(loop);
> > __d_drop(loop);
> > cond_resched_lock(&dcache_lock);
> > }
>
> Erm - wouldn't this break git-bisect?
Well Andrew will merge it into the earlier patch.