On Wednesday March 8, [email protected] 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
