Re: [patch 04/23] dentries: Extract common code to remove dentry from lru

Previous thread: [patch 02/23] SLUB: Rename NUMA defrag_ratio to remote_node_defrag_ratio by Christoph Lameter on Tuesday, November 6, 2007 - 9:11 pm. (11 messages)

Next thread: [patch 03/23] bufferhead: Revert constructor removal by Christoph Lameter on Tuesday, November 6, 2007 - 9:11 pm. (1 message)
To: <akpm@...>
Cc: <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Tuesday, November 6, 2007 - 9:11 pm

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 <riel@redhat.com>
Signed-off-by: Christoph Lameter <clameter@sgi.com>
---
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_dent...

To: Christoph Lameter <clameter@...>
Cc: <akpm@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 4:50 am

Hi Christoph,

Shouldn't this be dentry_lru_remove(loop)?

Hannes
-

To: <akpm@...>
Cc: Johannes Weiner <hannes-kernel@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 2:28 pm

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 <clameter@sgi.com>

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);
}
-

To: Johannes Weiner <hannes-kernel@...>
Cc: Christoph Lameter <clameter@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 5:43 am

Looks like it. Once this is fixed, feel free to add
Acked-by: Joern Engel <joern@logfs.org>

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
-

To: Jörn <joern@...>
Cc: Johannes Weiner <hannes-kernel@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 2:55 pm

Its in the slab defrag git now. I added the spelling fix and this one as a=
=20
result of the discussions today.

From: Christoph Lameter <clameter@sgi.com>
Subject: dcache: use the correct variable.

We need to use "loop" instead of "dentry"

Acked-by: Joern Engel <joern@logfs.org>
Signed-off-by: Christoph Lameter <clameter@sgi.com>

Index: linux-2.6/fs/dcache.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- linux-2.6.orig/fs/dcache.c=092007-11-07 10:26:20.000000000 -0800
+++ linux-2.6/fs/dcache.c=092007-11-07 10:26:27.000000000 -0800
@@ -610,7 +610,7 @@ static void shrink_dcache_for_umount_sub
=09=09=09spin_lock(&dcache_lock);
=09=09=09list_for_each_entry(loop, &dentry->d_subdirs,
=09=09=09=09=09 d_u.d_child) {
-=09=09=09=09dentry_lru_remove(dentry);
+=09=09=09=09dentry_lru_remove(loop);
=09=09=09=09__d_drop(loop);
=09=09=09=09cond_resched_lock(&dcache_lock);
=09=09=09}

To: Christoph Lameter <clameter@...>
Cc: Jörn <joern@...>, Johannes Weiner <hannes-kernel@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 2:54 pm

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
-

To: Jörn <joern@...>
Cc: Johannes Weiner <hannes-kernel@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, Mel Gorman <mel@...>
Date: Wednesday, November 7, 2007 - 3:00 pm

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

Well Andrew will merge it into the earlier patch.

Previous thread: [patch 02/23] SLUB: Rename NUMA defrag_ratio to remote_node_defrag_ratio by Christoph Lameter on Tuesday, November 6, 2007 - 9:11 pm. (11 messages)

Next thread: [patch 03/23] bufferhead: Revert constructor removal by Christoph Lameter on Tuesday, November 6, 2007 - 9:11 pm. (1 message)