Re: [PATCH 20/21] AUTOFS: Make sure all dentries refs are released before calling kill_anon_super()

[Ian Kent <raven@themaw.net>]

On Thu, 2006-07-06 at 18:56 -0700, Andrew Morton wrote:
> On Thu, 06 Jul 2006 11:11:07 -0400
> Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> 
> > From: David Howells <dhowells@redhat.com>
> > 
> > Make sure all dentries refs are released before calling kill_anon_super() so
> > that the assumption that generic_shutdown_super() can completely destroy the
> > dentry tree for there will be no external references holds true.
> > 
> > What was being done in the put_super() superblock op, is now done in the
> > kill_sb() filesystem op instead, prior to calling kill_anon_super().
> > 
> > This makes the struct autofs_sb_info::root member variable redundant (since
> > sb->s_root is still available), and so that is removed.  The calls to
> > shrink_dcache_sb() are also removed since they're also redundant as
> > shrink_dcache_for_umount() will now be called after the cleanup routine.
> > 
> 
> AFAIK, Ian is still off thinking about this one?

The patch looks fine to me but I wanted to give it a basic sanity test
before ACKing it. I haven't got to that yet partly because I wasn't sure
if the two patches from David would stand alone. Will they or will I
need to apply Davids whole patch set?

Anyway, I'll have a go at it this afternoon.

> 
> > 
> >  fs/autofs4/autofs_i.h |    3 +--
> >  fs/autofs4/init.c     |    2 +-
> >  fs/autofs4/inode.c    |   22 ++++------------------
> >  fs/autofs4/waitq.c    |    1 -
> >  4 files changed, 6 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
> > index d6603d0..47e38f3 100644
> > --- a/fs/autofs4/autofs_i.h
> > +++ b/fs/autofs4/autofs_i.h
> > @@ -96,7 +96,6 @@ #define AUTOFS_TYPE_OFFSET       0x0004
> >  
> >  struct autofs_sb_info {
> >  	u32 magic;
> > -	struct dentry *root;
> >  	int pipefd;
> >  	struct file *pipe;
> >  	pid_t oz_pgrp;
> > @@ -231,4 +230,4 @@ out:
> >  }
> >  
> >  void autofs4_dentry_release(struct dentry *);
> > -
> > +extern void autofs4_kill_sb(struct super_block *);
> > diff --git a/fs/autofs4/init.c b/fs/autofs4/init.c
> > index 5d91933..723a1c5 100644
> > --- a/fs/autofs4/init.c
> > +++ b/fs/autofs4/init.c
> > @@ -24,7 +24,7 @@ static struct file_system_type autofs_fs
> >  	.owner		= THIS_MODULE,
> >  	.name		= "autofs",
> >  	.get_sb		= autofs_get_sb,
> > -	.kill_sb	= kill_anon_super,
> > +	.kill_sb	= autofs4_kill_sb,
> >  };
> >  
> >  static int __init init_autofs4_fs(void)
> > diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
> > index fde78b1..1bf68c5 100644
> > --- a/fs/autofs4/inode.c
> > +++ b/fs/autofs4/inode.c
> > @@ -95,7 +95,7 @@ void autofs4_free_ino(struct autofs_info
> >   */
> >  static void autofs4_force_release(struct autofs_sb_info *sbi)
> >  {
> > -	struct dentry *this_parent = sbi->root;
> > +	struct dentry *this_parent = sbi->sb->s_root;
> >  	struct list_head *next;
> >  
> >  	spin_lock(&dcache_lock);
> > @@ -126,7 +126,7 @@ resume:
> >  		spin_lock(&dcache_lock);
> >  	}
> >  
> > -	if (this_parent != sbi->root) {
> > +	if (this_parent != sbi->sb->s_root) {
> >  		struct dentry *dentry = this_parent;
> >  
> >  		next = this_parent->d_u.d_child.next;
> > @@ -139,15 +139,9 @@ resume:
> >  		goto resume;
> >  	}
> >  	spin_unlock(&dcache_lock);
> > -
> > -	dput(sbi->root);
> > -	sbi->root = NULL;
> > -	shrink_dcache_sb(sbi->sb);
> > -
> > -	return;
> >  }
> >  
> > -static void autofs4_put_super(struct super_block *sb)
> > +void autofs4_kill_sb(struct super_block *sb)
> >  {
> >  	struct autofs_sb_info *sbi = autofs4_sbi(sb);
> >  
> > @@ -162,6 +156,7 @@ static void autofs4_put_super(struct sup
> >  	kfree(sbi);
> >  
> >  	DPRINTK("shutting down");
> > +	kill_anon_super(sb);
> >  }
> >  
> >  static int autofs4_show_options(struct seq_file *m, struct vfsmount *mnt)
> > @@ -188,7 +183,6 @@ static int autofs4_show_options(struct s
> >  }
> >  
> >  static struct super_operations autofs4_sops = {
> > -	.put_super	= autofs4_put_super,
> >  	.statfs		= simple_statfs,
> >  	.show_options	= autofs4_show_options,
> >  };
> > @@ -314,7 +308,6 @@ int autofs4_fill_super(struct super_bloc
> >  
> >  	s->s_fs_info = sbi;
> >  	sbi->magic = AUTOFS_SBI_MAGIC;
> > -	sbi->root = NULL;
> >  	sbi->pipefd = -1;
> >  	sbi->catatonic = 0;
> >  	sbi->exp_timeout = 0;
> > @@ -396,13 +389,6 @@ int autofs4_fill_super(struct super_bloc
> >  	sbi->pipefd = pipefd;
> >  
> >  	/*
> > -	 * Take a reference to the root dentry so we get a chance to
> > -	 * clean up the dentry tree on umount.
> > -	 * See autofs4_force_release.
> > -	 */
> > -	sbi->root = dget(root);
> > -
> > -	/*
> >  	 * Success! Install the root dentry now to indicate completion.
> >  	 */
> >  	s->s_root = root;
> > diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
> > index ce103e7..c0a6c8d 100644
> > --- a/fs/autofs4/waitq.c
> > +++ b/fs/autofs4/waitq.c
> > @@ -45,7 +45,6 @@ void autofs4_catatonic_mode(struct autof
> >  		fput(sbi->pipe);	/* Close the pipe */
> >  		sbi->pipe = NULL;
> >  	}
> > -	shrink_dcache_sb(sbi->sb);
> >  }
> >  
> >  static int autofs4_write(struct file *file, const void *addr, int bytes)