From: Neil Brown <neilb@suse.de>
To: balbir@in.ibm.com
Cc: Kirill Korotaev <dev@sw.ru>, Balbir Singh <bsingharora@gmail.com>,
linux-kernel@vger.kernel.org, Andrew Morton <akpm@osdl.org>,
Olaf Hering <olh@suse.de>, Jan Blunck <jblunck@suse.de>,
Kirill Korotaev <dev@openvz.org>, Al Viro <viro@ftp.linux.org.uk>
Subject: Re: [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super
Date: Wed, 8 Mar 2006 13:39:39 +1100 [thread overview]
Message-ID: <17422.17387.691138.193521@cse.unsw.edu.au> (raw)
In-Reply-To: message from Balbir Singh on Wednesday March 8
On Wednesday March 8, balbir@in.ibm.com wrote:
> > So: blending the better bits of various patches together, I've come up
> > with the following. It still needs a changelog entry, but does anyone
> > want to ACK it ???
>
> I think this patch is much more cleaner and refined.
Thanks.
>
> From yesterdays comments I am beginning to wonder if it is enough to solve
> only the unmount race or should the fix be more generic to address the race
> between the shrinkers call to shrink_dcache_memory() and shrink_dcache_parent().
>
> From what I understand of the race, the race occurs when dput of the
> parent fails to happen and because the referecne count is not 0,
> shrink_dcache_parent() skips over those dentries. The race occurs for
> the dentry and its ancestors above
I think that in most cases, the race doesn't matter if
shrink_dcache_memory misses a dentry because someone else is holding a
temporary reference, it really doesn't matter.
Similarly most callers of shrink_dcache_parent are happy with a
best-effort.
Unmount is a special case because it wants to 'shrink' *all* of the
dentries for the filesystem. In that case, someone holding a
transient reference s a bad thing. In other cases it is, at best, a
minor inconvenience.
>
> I was wondering if the following would work (to solve the generic race)
>
> Add a prune_mutex to the super-block. Hold on to it in prune_one_dentry()
> until we hit a parent dentry that is a mount point (d_mounted > 0) or
> the parent has a reference count > 1 or at the end of prune_one_dentry().
> This should ensure that for each super block dentry counts are consistent.
> Also get select_parent() to hold the super block's prune_mutex, so that it
> sees a consistent view of the super block.
>
> Oh! now that I think about it, I think your solution looks like an
> elegant way to do the same thing. The only draw back is that it solves
> only the unmount race and there are some changes in generic_shutdown_super()
> which I do not understand.
>
> > diff ./fs/super.c~current~ ./fs/super.c
> > --- ./fs/super.c~current~ 2006-03-08 10:50:37.000000000 +1100
> > +++ ./fs/super.c 2006-03-08 11:02:12.000000000 +1100
> > @@ -81,6 +81,8 @@ static struct super_block *alloc_super(v
> > mutex_init(&s->s_dquot.dqio_mutex);
> > mutex_init(&s->s_dquot.dqonoff_mutex);
> > init_rwsem(&s->s_dquot.dqptr_sem);
> > + s->s_prunes = 0;
> > + init_waitqueue_head(&s->s_wait_prunes);
> > init_waitqueue_head(&s->s_wait_unfrozen);
> > s->s_maxbytes = MAX_NON_LFS;
> > s->dq_op = sb_dquot_ops;
> > @@ -231,6 +233,7 @@ void generic_shutdown_super(struct super
> >
> > if (root) {
> > sb->s_root = NULL;
> > + wait_on_prunes(sb);
> > shrink_dcache_sb(sb);
>
> Hmm... in 2.6.16-rc5, I see
>
> shrink_dcache_parent(root);
> shrink_dcache_anon(&sb->sb_anon);
>
> without these calls, some dentries might not get moved to LRU list.
>
> Am I missing something here?
I should have been more explicit that the patch was against
2.6.16-rc5-mm2. This contains some dcache patches to allow nfs
filesystem to share superblocks, and one of the patches replaces the
calls to shrink_dcache_parent and shrink_dcache_anon with a single
call to a new function: shrink_dcache_sb.
Thanks for the feedback
NeilBrown
next prev parent reply other threads:[~2006-03-08 2:40 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-03-02 6:57 [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super Neil Brown
2006-03-02 10:48 ` Jan Blunck
2006-03-03 11:42 ` Jan Blunck
2006-03-06 6:09 ` Neil Brown
2006-03-06 7:32 ` Balbir Singh
2006-03-07 1:58 ` Neil Brown
2006-03-07 2:49 ` Balbir Singh
2006-03-07 6:22 ` Kirill Korotaev
2006-03-07 6:16 ` Kirill Korotaev
2006-03-07 7:03 ` Balbir Singh
2006-03-07 7:21 ` Kirill Korotaev
2006-03-07 11:05 ` Balbir Singh
2006-03-08 0:29 ` Neil Brown
2006-03-08 2:17 ` Balbir Singh
2006-03-08 2:39 ` Neil Brown [this message]
2006-03-08 3:05 ` Balbir Singh
2006-03-08 11:01 ` Jan Blunck
2006-03-06 11:56 ` Jan Blunck
2006-03-07 2:15 ` Neil Brown
2006-03-06 11:56 ` Kirill Korotaev
2006-03-07 2:01 ` Neil Brown
2006-03-07 6:20 ` Kirill Korotaev
2006-03-07 23:20 ` Neil Brown
2006-03-09 12:03 ` Kirill Korotaev
-- strict thread matches above, loose matches on Subject: below --
2006-01-16 22:34 Olaf Hering
2006-01-16 23:23 ` Kirill Korotaev
2006-01-16 23:29 ` Olaf Hering
2006-01-17 2:05 ` Andrew Morton
2006-01-17 7:03 ` Kirill Korotaev
2006-01-18 22:49 ` Jan Blunck
2006-01-18 23:10 ` Andrew Morton
2006-01-19 10:08 ` Kirill Korotaev
2006-01-19 9:52 ` Kirill Korotaev
2006-01-19 10:04 ` Jan Blunck
2006-01-19 10:26 ` Kirill Korotaev
2006-01-20 19:06 ` Jan Blunck
2006-01-23 8:14 ` Kirill Korotaev
2006-01-30 11:54 ` Jan Blunck
2006-01-30 14:05 ` Kirill Korotaev
2006-01-30 14:21 ` Jan Blunck
2006-01-30 14:34 ` Kirill Korotaev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17422.17387.691138.193521@cse.unsw.edu.au \
--to=neilb@suse.de \
--cc=akpm@osdl.org \
--cc=balbir@in.ibm.com \
--cc=bsingharora@gmail.com \
--cc=dev@openvz.org \
--cc=dev@sw.ru \
--cc=jblunck@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=olh@suse.de \
--cc=viro@ftp.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).