Re: [PATCH 00/24] Reduce the stack foot print of the NFS client

[Trond Myklebust <Trond.Myklebust@netapp.com>]

On Mon, 2010-04-19 at 17:43 -0400, Chuck Lever wrote: 
> On 04/16/2010 04:30 PM, Trond Myklebust wrote:
> > The following patch series aims to significantly reduce the stack foot
> > print of the NFS client by dynamically allocating the struct nfs_fattr
> > and struct nfs_fh.
> 
> Random comments:
> 
> 1.  There's an open-coded kzalloc of an nfs_fh in nfs_get_sb.  This can 
> be replaced with nfs_alloc_fh().

Added a clean up patch to do this.

> 2.  root_nfs_get_handle allocates an nfs_fh on the stack.  This can be 
> replaced with nfs_alloc_fh().

I suppose, although there will never be any danger of anyone calling
this function from any unexpected contexts. Anyhow, added a patch.

> 3.  There is an unneeded nfs_fattr_init() call in nfs_probe_fsinfo().  I 
> didn't look for others that are similarly made obsolete.

Fixed.

> 4.  Where ever you have "if (fattr == NULL) goto out;" (and similarly 
> for filehandles) you could add "unlikely()" to improve branch prediction 
> slightly.

No. While I agree that it is unlikely to fail, I prefer _not_ to have to
wade through all these likely()/unlikely() markups in order to read the
code. A quick perusal of other examples shows that I'm not alone in that
preference.

> 5.  The documenting comment before nfs_do_refmount() is out of date.

Fixed.

Cheers
  Trond