lkml.org 
[lkml]   [2006]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Fix shrink_dcache_parent() against shrink_dcache_memory() race (updated patch)
    On Wed, Mar 08, 2006 at 03:51:05PM +0100, Jan Blunck wrote:
    > Andrew, I have test this patch for a while now and none of the users has seen
    > the "busy inodes" message for a while now. Can you please apply and test it in
    > -mm?
    >
    > This is an updated version of the patch which adresses some issues that came
    > up during discussion. Although sb->prunes usually is 0, I'm testing it now
    > before calling wake_up(). Besides that, the shrink_dcache_parent() is only
    > waiting for prunes if we are called through generic_shutdown_super() when
    > sb->s_root is NULL.
    >
    > Original patch description:
    >
    > Kirill Korotaev <dev@sw.ru> discovered a race between shrink_dcache_parent()
    > and shrink_dcache_memory() which leads to "Busy inodes after unmount".
    > When unmounting a file system shrink_dcache_parent() is racing against a
    > possible shrink_dcache_memory(). This might lead to the situation that
    > shrink_dcache_parent() is returning too early. In this situation the
    > super_block is destroyed before shrink_dcache_memory() could put the inode.
    >
    > This patch fixes the problem through introducing a prunes counter which is
    > incremented when a dentry is pruned but the corresponding inoded isn't put
    > yet.When the prunes counter is not null, shrink_dcache_parent() is waiting and
    > restarting its work.
    >
    > Signed-off-by: Jan Blunck <jblunck@suse.de>

    Looks good, small cosmetic comment below

    <snip>

    > +/*
    > + * If we slept on waiting for other prunes to finish, there maybe are
    > + * some dentries the d_lru list that we have "overlooked" the last
    > + * time we called select_parent(). Therefor lets restart in this case.
    > + */
    > void shrink_dcache_parent(struct dentry * parent)
    > {
    > int found;
    > + struct super_block *sb = parent->d_sb;
    >
    > + again:
    > while ((found = select_parent(parent)) != 0)
    > prune_dcache(found);
    > +
    > + /* If we are called from generic_shutdown_super() during
    > + * umount of a filesystem, we want to check for other prunes */
    > + if (!sb->s_root && wait_on_prunes(sb))
    > + goto again;
    > }

    cosmetic - could we do this with a do { } while() loop instead of a goto?

    I think though that after select_parent(), wait_on_prunes() can sleep just
    once, so we do not need a goto. Just calling wait_on_prunes() should
    fix the race. For all the dentries missed in the race, wait_on_parent()
    will ensure that they are dput() by prune_one_dentry() before wait_on_parent()
    returns.

    But, I do not have anything against the goto, so this patch should be just
    fine.

    <snip>

    > if (root) {
    > sb->s_root = NULL;
    > - shrink_dcache_parent(root);
    > shrink_dcache_anon(&sb->s_anon);
    > + shrink_dcache_parent(root);
    > dput(root);

    This change might conflict with the NFS patches in -mm.

    <snip>

    Thanks,
    Balbir
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-03-09 07:36    [W:0.039 / U:239.788 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site