On Fri, Apr 04, 2008 at 11:29:28AM -0400, Josef Bacik wrote:
> On Fri, Apr 04, 2008 at 06:28:46PM +0300, Alex Lyashkov wrote:
> > Thanks for answer,
> >
> > Yes this commit - fix issue with umount, but not resolve main question:
> > why prune_dcache kill dentry with diffrent super block? this produce
> > long loop with skip loop instead of early exit and move dentry from
> > select_parent again to end of list.
> >
>
> probably worth looking at doing something different in the case of shrinking the
> dcache on the parent, and leaving prune_dcache to only be called in the case of
> trying to free up dcache under memory pressure, where the superblock doesn't
> actually matter. For the RHEL3 issue you are reffering to I fixed it by
> creating a private list when we shrunk the parent, and submitting that list to
> prune_dcache that way we didn't spend all this time looping. I will see what
> can be done for upstream.
>
If you would like to test the current upstream I have a patch that does what I
described above. I only compile tested it, so who knows what it will do :).
Signed-off-by: Josef Bacik <jbacik@xxxxxxxxxx>
diff --git a/fs/dcache.c b/fs/dcache.c
index 4345577..564df98 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -413,8 +413,8 @@ static void prune_one_dentry(struct dentry * dentry)
/**
* prune_dcache - shrink the dcache
* @count: number of entries to try and free
- * @sb: if given, ignore dentries for other superblocks
- * which are being unmounted.
+ * @dispose_list: if given, remove dentries from this
+ * list instead of from the dentry_unused list.
*
* Shrink the dcache. This is done when we need
* more memory, or simply when we need to unmount
@@ -425,8 +425,15 @@ static void prune_one_dentry(struct dentry * dentry)
* all the dentries are in use.
*/
-static void prune_dcache(int count, struct super_block *sb)
+static void prune_dcache(int count, struct list_head *dispose_list)
{
+ struct list_head *list;
+
+ if (dispose_list)
+ list = dispose_list;
+ else
+ list = &dentry_unused;
+
spin_lock(&dcache_lock);
for (; count ; count--) {
struct dentry *dentry;
@@ -435,23 +442,11 @@ static void prune_dcache(int count, struct super_block *sb)
cond_resched_lock(&dcache_lock);
- tmp = dentry_unused.prev;
- if (sb) {
- /* Try to find a dentry for this sb, but don't try
- * too hard, if they aren't near the tail they will
- * be moved down again soon
- */
- int skip = count;
- while (skip && tmp != &dentry_unused &&
- list_entry(tmp, struct dentry, d_lru)->d_sb != sb) {
- skip--;
- tmp = tmp->prev;
- }
- }
- if (tmp == &dentry_unused)
+ tmp = list->prev;
+ if (tmp == list)
break;
list_del_init(tmp);
- prefetch(dentry_unused.prev);
+ prefetch(list->prev);
dentry_stat.nr_unused--;
dentry = list_entry(tmp, struct dentry, d_lru);
@@ -468,26 +463,16 @@ static void prune_dcache(int count, struct super_block *sb)
/* If the dentry was recently referenced, don't free it. */
if (dentry->d_flags & DCACHE_REFERENCED) {
dentry->d_flags &= ~DCACHE_REFERENCED;
- list_add(&dentry->d_lru, &dentry_unused);
+ list_add(&dentry->d_lru, list);
dentry_stat.nr_unused++;
spin_unlock(&dentry->d_lock);
continue;
}
/*
* If the dentry is not DCACHED_REFERENCED, it is time
- * to remove it from the dcache, provided the super block is
- * NULL (which means we are trying to reclaim memory)
- * or this dentry belongs to the same super block that
- * we want to shrink.
- */
- /*
- * If this dentry is for "my" filesystem, then I can prune it
- * without taking the s_umount lock (I already hold it).
+ * to remove it from the dcache
*/
- if (sb && dentry->d_sb == sb) {
- prune_one_dentry(dentry);
- continue;
- }
+
/*
* ...otherwise we need to be sure this filesystem isn't being
* unmounted, otherwise we could race with
@@ -505,13 +490,20 @@ static void prune_dcache(int count, struct super_block *sb)
continue;
}
up_read(s_umount);
+ } else if (dispose_list) {
+ /*
+ * if we're shrinking dcache for an unmounting fs
+ * just go ahead and exit
+ */
+ spin_unlock(&dentry->d_lock);
+ break;
}
spin_unlock(&dentry->d_lock);
/*
* Insert dentry at the head of the list as inserting at the
* tail leads to a cycle.
*/
- list_add(&dentry->d_lru, &dentry_unused);
+ list_add(&dentry->d_lru, list);
dentry_stat.nr_unused++;
}
spin_unlock(&dcache_lock);
@@ -761,19 +753,20 @@ positive:
/*
* Search the dentry child list for the specified parent,
- * and move any unused dentries to the end of the unused
+ * and move any unused dentries to the end of the dispose
* list for prune_dcache(). We descend to the next level
* whenever the d_subdirs list is non-empty and continue
* searching.
*
* It returns zero iff there are no unused children,
* otherwise it returns the number of children moved to
- * the end of the unused list. This may not be the total
+ * the end of the dispose list. This may not be the total
* number of unused children, because select_parent can
* drop the lock and return early due to latency
* constraints.
*/
-static int select_parent(struct dentry * parent)
+static int select_parent(struct dentry * parent,
+ struct list_head *dispose_list)
{
struct dentry *this_parent = parent;
struct list_head *next;
@@ -794,7 +787,7 @@ resume:
* of the unused list for prune_dcache
*/
if (!atomic_read(&dentry->d_count)) {
- list_add_tail(&dentry->d_lru, &dentry_unused);
+ list_add_tail(&dentry->d_lru, dispose_list);
dentry_stat.nr_unused++;
found++;
}
@@ -838,9 +831,14 @@ out:
void shrink_dcache_parent(struct dentry * parent)
{
int found;
+ LIST_HEAD(dispose_list);
- while ((found = select_parent(parent)) != 0)
- prune_dcache(found, parent->d_sb);
+ /*
+ * select_parent will populate dispose_list with the
+ * dentry's that we are removing
+ */
+ while ((found = select_parent(parent, &dispose_list)) != 0)
+ prune_dcache(found, &dispose_list);
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
