Home » Mailing lists » Devel » [Q] missing unused dentry in prune_dcache()?
| Re: Re: [PATCH 2.6.19-rc3] VFS: per-sb dentry lru list [message #8296 is a reply to message #7970] |
Tue, 14 November 2006 04:51   |
Neil Brown
Messages: 6
Registered: October 2006
|
Junior Member |
|
|
On Wednesday November 1, dev@sw.ru wrote:
> <<<< AFAICS, doing so you introduced a leak of anonymous dentries.
>
> d_alloc_anon() calls d_alloc() with parent == NULL, i.e. dentries have no parent
> and are not linked to the sb->s_root...
Yep, thanks.
> BTW, looking at it, I found that s_anon field on super block is not
> used any more.
I don't know what you mean by that. It is still used...
> we can add BUG_ON(!hlist_empty(&sb->s_anon)) in generic_shutdown_super to avoid such issues like this.
>
> maybe we can fix it adding something like:
> while (!list_empty(&sb->s_anon)))
> prune_dcache(MAX_INT, &sb->s_anon);
It seems that anon dentries can now have children (I think someone
explained that too me - shrink_dcache_for_umount certainly suggests
it).
Also, in this context we cannot be sure that all dentries can be
freed. This is being called a remount time remember, and some stuff
might still be in use.
We probably need to move the s_anon list to a temporary list and
repeatedly:
move the top entry back to s_anon and call shrink_dcache_parent
on it.
Needs more thought.
NeilBrown
|
|
|
|
| Re: Re: [PATCH 2.6.19-rc3] VFS: per-sb dentry lru list [message #8297 is a reply to message #7973] |
Tue, 14 November 2006 05:44 |
Neil Brown
Messages: 6
Registered: October 2006
|
Junior Member |
|
|
On Wednesday November 1, vvs@sw.ru wrote:
>
> Currently we have 3 type of functions that works with dentry_unused list:
>
> 1) prune_dcache(NULL) -- called from shrink_dcache_memory, frees the memory and
> requires global LRU. works well in current implementation.
> 2) prune_dcache(sb) -- called from shrink_dcache_parent(), frees subtree, LRU
> is not need here. Current implementation uses global LRU for these purposes, it
> is ineffective, and patch from Neil Brown fixes this issue.
> 3) shrink_dcache_sb() -- called when we need to free the unused dentries for
> given super block. Current implementation is not effective too, and per-sb LRU
> would be the best solution here. On the other hand patch from Neil Brown is much
> better than current implementation.
>
> In general I think that we should approve Neil Brown's patch. We (I and Kirill
> Korotaev) are ready to acknowledge it when the following remarks fill be fixed:
>
> - it seems for me list_splice() is not required inside
> prune_dcache(),
Yes, the list should be empty when we finish so you are right.
> - DCACHE_REFERENCED dentries should not be removed from private list to
> dentry_unused list, this flag should be ignored if the private list is used,
Agreed.
> - count argument should be ignored in this case too, we want to free all the
> dentries in private list,
Agreed.
> - when we shrink the whole super block we should free per-sb anonymous dentries
> too (please see Kirill Korotaev's letter)
>
Yes. Unfortunately I don't think it is as easy as it sounds.
I'll have a closer look.
> Then I'm going to prepare new patch that will enhance the shrink_dcache_sb()
> performance:
> - we can add new list head into struct superblock and use it in
> shrink_dcache_sb() instead of temporal private list. We will check is it empty
> in dput() and add the new unused dentries to per-sb list instead of
> dentry_unused list.
I think that makes sense. It means that you end up doing less work in
select_parent, because the work has already been done in dput.
How is the patch going?
NeilBrown
|
|
|
|
| Re: [PATCH 2.6.19-rc3] VFS: per-sb dentry lru list [message #8298 is a reply to message #8297] |
Tue, 14 November 2006 06:12 |
vaverin
Messages: 708
Registered: September 2005
|
Senior Member |
|
|
Hello Neil
Neil Brown wrote:
> On Wednesday November 1, vvs@sw.ru wrote:
>> Currently we have 3 type of functions that works with dentry_unused list:
>>
>> 1) prune_dcache(NULL) -- called from shrink_dcache_memory, frees the memory and
>> requires global LRU. works well in current implementation.
>> 2) prune_dcache(sb) -- called from shrink_dcache_parent(), frees subtree, LRU
>> is not need here. Current implementation uses global LRU for these purposes, it
>> is ineffective, and patch from Neil Brown fixes this issue.
>> 3) shrink_dcache_sb() -- called when we need to free the unused dentries for
>> given super block. Current implementation is not effective too, and per-sb LRU
>> would be the best solution here. On the other hand patch from Neil Brown is much
>> better than current implementation.
>>
>> In general I think that we should approve Neil Brown's patch. We (I and Kirill
>> Korotaev) are ready to acknowledge it when the following remarks fill be fixed:
>
>
>> - it seems for me list_splice() is not required inside
>> prune_dcache(),
>
> Yes, the list should be empty when we finish so you are right.
>
>> - DCACHE_REFERENCED dentries should not be removed from private list to
>> dentry_unused list, this flag should be ignored if the private list is used,
>
> Agreed.
>
>> - count argument should be ignored in this case too, we want to free all the
>> dentries in private list,
>
> Agreed.
>
>> - when we shrink the whole super block we should free per-sb anonymous dentries
>> too (please see Kirill Korotaev's letter)
>>
>
> Yes. Unfortunately I don't think it is as easy as it sounds.
> I'll have a closer look.
>
>
>> Then I'm going to prepare new patch that will enhance the shrink_dcache_sb()
>> performance:
>> - we can add new list head into struct superblock and use it in
>> shrink_dcache_sb() instead of temporal private list. We will check is it empty
>> in dput() and add the new unused dentries to per-sb list instead of
>> dentry_unused list.
>
> I think that makes sense. It means that you end up doing less work in
> select_parent, because the work has already been done in dput.
>
> How is the patch going?
Please see patches in attach files:
first one is incremental for your patch,
second one is merged version.
It would be very interesting to know your opinion about these changes.
thank you,
Vasily Averin
--- linux-2.6.19-rc4/fs/dcache.c.vshud1 2006-11-02 14:23:03.000000000 +0300
+++ linux-2.6.19-rc4/fs/dcache.c 2006-11-03 14:57:58.000000000 +0300
@@ -171,8 +171,14 @@ repeat:
if (d_unhashed(dentry))
goto kill_it;
if (list_empty(&dentry->d_lru)) {
- dentry->d_flags |= DCACHE_REFERENCED;
- list_add(&dentry->d_lru, &dentry_unused);
+ struct list_head *list;
+
+ list = &dentry->d_sb->s_shrink_list;
+ if (list_empty(list)) {
+ list = &dentry_unused;
+ dentry->d_flags |= DCACHE_REFERENCED;
+ }
+ list_add(&dentry->d_lru, list);
dentry_stat.nr_unused++;
}
spin_unlock(&dentry->d_lock);
@@ -394,10 +400,6 @@ static void prune_one_dentry(struct dent
*
* This function may fail to free any resources if
* all the dentries are in use.
- *
- * Any dentries that were not removed due to the @count
- * limit will be splice on to the end of dentry_unused,
- * so they should already be founded in dentry_stat.nr_unused.
*/
static void prune_dcache(int count, struct list_head *list)
@@ -409,7 +411,7 @@ static void prune_dcache(int count, stru
/* use the dentry_unused list */
list = &dentry_unused;
- for (; count ; count--) {
+ for (; have_list || count-- ;) {
struct dentry *dentry;
struct list_head *tmp;
struct rw_semaphore *s_umount;
@@ -434,6 +436,17 @@ static void prune_dcache(int count, stru
spin_unlock(&dentry->d_lock);
continue;
}
+ /*
+ * If this dentry is for "my" filesystem, then I can prune it
+ * without taking the s_umount lock: either I already hold it
+ * (called from shrink_dcache_sb) or are called from some
+ * filesystem opernations and therefore cannot race with
+ * generic_shutdown_super().
+ */
+ if (have_list) {
+ prune_one_dentry(dentry);
+ continue;
+ }
/* If the dentry was recently referenced, don't free it. */
if (dentry->d_flags & DCACHE_REFERENCED) {
dentry->d_flags &= ~DCACHE_REFERENCED;
@@ -443,21 +456,6 @@ static void prune_dcache(int count, stru
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).
- */
- if (have_list) {
- prune_one_dentry(dentry);
- continue;
- }
- /*
* ...otherwise 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
@@ -483,27 +481,9 @@ static void prune_dcache(int count, stru
list_add(&dentry->d_lru, &dentry_unused);
dentry_stat.nr_unused++;
}
- /* split any remaining entries back onto dentry_unused */
- if (have_list)
- list_splice(list, dentry_unused.prev);
spin_unlock(&dcache_lock);
}
-/**
- * shrink_dcache_sb - shrink dcache for a superblock
- * @sb: superblock
- *
- * Shrink the dcache for the specified super block. This
- * is used to reduce the dcache presence of a file system
- * before re-mounting, and when invalidating the device
- * holding a file system.
- */
-
-void shrink_dcache_sb(struct super_block * sb)
-{
- shrink_dcache_parent(sb->s_root);
-}
-
/*
* destroy a single subtree of dentries for unmount
* - see the comments on shrink_dcache_for_umount() for a description of the
@@ -706,11 +686,10 @@ positive:
* drop the lock and return early due to latency
* constraints.
*/
-static int select_parent(struct dentry * parent, struct list_head *new)
+static void select_parent(struct dentry * parent, struct list_head *new)
{
struct dentry *this_parent = parent;
struct list_head *next;
- int found = 0;
spin_lock(&dcache_lock);
repeat:
@@ -732,7 +711,6 @@ resume:
if (!atomic_read(&dentry->d_count)) {
list_add_tail(&dentry->d_lru, new);
dentry_stat.nr_unused++;
- found++;
}
/*
@@ -740,7 +718,7 @@ resume:
* ensures forward progress). We'll be coming back to find
* the rest.
*/
- if (found && need_resched())
+ if (!list_empty(new) && need_resched())
goto out;
/*
@@ -761,7 +739,37 @@ resume:
}
out:
spin_unlock(&dcache_lock);
- return found;
+}
+
+/**
+ * select_anon - further prune the cache
+ * @sb: superblock
+ *
+ * Prune the dentries that are anonymous
+ */
+
+static void select_anon(struct super_block *sb)
+{
+ struct hlist_node *lp;
+ struct hlist_head *head = &sb->s_anon;
+
+ spin_lock(&dcache_lock);
+ hlist_for_each(lp, head) {
+ struct dentry *this = hlist_entry(lp, struct dentry, d_hash);
+ if (!list_empty(&this->d_lru)) {
+ dentry_stat.nr_unused--;
+ list_del_init(&this->d_lru);
+ }
+ /*
+ * move only zero ref count dentries to the end
+ * of list for prune_dcache
+ */
+ if (!atomic_read(&this->d_count)) {
+ list_add(&this->d_lru, &sb->s_shrink_list);
+ dentry_stat.nr_unused++;
+ }
+ }
+ spin_unlock(&dcache_lock);
}
/**
@@ -773,11 +781,38 @@ out:
void shrink_dcache_parent(struct dentry * parent)
{
- int found;
LIST_HEAD(list);
- while ((found = select_parent(parent, &list)) != 0)
- prune_dcache(found, &list);
+ for (;;) {
+ select_parent(parent, &list);
+ if (list_empty(&list))
+ break;
+ prune_dcache(0, &list);
+ }
+}
+
+/**
+ * shrink_dcache_sb - shrink dcache for a superblock
+ * @sb: superblock
+ *
+ * Shrink the dcache for the specified super block. This
+ * is used to reduce the dcache presence of a file system
+ * before re-mounting, and when invalidating the device
+ * holding a file system.
+ */
+
+void shrink_dcache_sb(struct super_block * sb)
+{
+ struct list_head *list;
+
+ list = &sb->s_shrink_list;
+ for (;;) {
+ select_parent(sb->s_root, list);
+ select_anon(sb);
+ if (list_empty(list))
+ break;
+ prune_dcache(0, list);
+ }
}
/*
--- linux-2.6.19-rc4/include/linux/fs.h.vshud1 2006-11-03 15:28:04.000000000 +0300
+++ linux-2.6.19-rc4/include/linux/fs.h 2006-11-03 14:52:16.000000000 +0300
@@ -941,6 +941,7 @@ struct super_block {
struct hlist_head s_anon; /* anonymous dentries for (nfs) exporting */
struct list_head s_files;
+ struct list_head s_shrink_list;
struct block_device *s_bdev;
struct list_head s_instances;
struct quota_info s_dquot; /* Diskquota specific options */
--- linux-2.6.19-rc4/fs/dcache.c.vdch 2006-11-03 14:22:54.000000000 +0300
+++ linux-2.6.19-rc4/fs/dcache.c 2006-11-03 14:57:58.000000000 +0300
@@ -171,8 +171,14 @@ repeat:
if (d_unhashed(dentry))
goto kill_it;
if (list_empty(&dentry->d_lru)) {
- dentry->d_flags |= DCACHE_REFERENCED;
- list_add(&dentry->d_lru, &dentry_unused);
+ struct list_head *list;
+
+ list = &dentry->d_sb->s_shrink_list;
+ if (list_empty(list)) {
+ list = &dentry_unused;
+ dentry->d_flags |= DCACHE_REFERENCED;
+ }
+ list_add(&dentry->d_lru, list);
dentry_stat.nr_unused++;
}
spin_unlock(&dentry->d_lock);
@@ -384,8 +390,8 @@ static void prune_one_dentry(struct dent
/**
* prune_dcac
...
|
|
|
|
|
|
Goto Forum:
Current Time: Fri May 17 05:48:44 GMT 2024
Total time taken to generate the page: 0.00500 seconds
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
