On Fri, Apr 04, 2008 at 09:01:26PM +0200, Miklos Szeredi wrote:
> > > > > 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.
> > >
> > > Which sounds racy with umount. A hashed dentry must either have a
> > > refcount greater than one, or be on dentry_unused list. This patch
> > > breaks that assumption.
> > >
> >
> > It should be racy with umount, if we notice that we're being
> > unmounted we just break, as the unmount will free the dentry's
> > itself through another means.
>
> But unmount could have already finished by then. And now you are
> dereferencing a super block, that no longer exists. Not good.
>
> This separate list thing can't work, unfortunately. On the other hand
> we should probably split prune_dcache() into two separate functions:
> the garbage collector one (with sb == NULL argument) and the sb
> specific shrinker. It should I think be possible to share the second
> one with shrink_dcache_sb().
>
Hmm good point. How bout this? Keeps us from doing the skip thing and keeps
the garbage collector path a little cleaner. Thanks much,
Josef
diff --git a/fs/dcache.c b/fs/dcache.c
index 4345577..2d22176 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -413,8 +413,6 @@ 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.
*
* Shrink the dcache. This is done when we need
* more memory, or simply when we need to unmount
@@ -425,7 +423,7 @@ 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)
{
spin_lock(&dcache_lock);
for (; count ; count--) {
@@ -436,18 +434,6 @@ 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)
break;
list_del_init(tmp);
@@ -475,21 +461,10 @@ static void prune_dcache(int count, struct super_block *sb)
}
/*
* 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.
+ * to remove it from the dcache
*/
/*
- * If this dentry is for "my" filesystem, then I can prune it
- * without taking the s_umount lock (I already hold it).
- */
- if (sb && dentry->d_sb == sb) {
- prune_one_dentry(dentry);
- continue;
- }
- /*
- * ...otherwise we need to be sure this filesystem isn't being
+ * we need to be sure this filesystem isn't being
* unmounted, otherwise we could race with
* generic_shutdown_super(), and end up holding a reference to
* an inode while the filesystem is unmounted.
@@ -840,7 +815,7 @@ void shrink_dcache_parent(struct dentry * parent)
int found;
while ((found = select_parent(parent)) != 0)
- prune_dcache(found, parent->d_sb);
+ shrink_dcache_sb(parent->d_sb);
}
/*
@@ -860,7 +835,7 @@ static int shrink_dcache_memory(int nr, gfp_t gfp_mask)
if (nr) {
if (!(gfp_mask & __GFP_FS))
return -1;
- prune_dcache(nr, NULL);
+ prune_dcache(nr);
}
return (dentry_stat.nr_unused / 100) * sysctl_vfs_cache_pressure;
}
--
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
