On 04/19/2010 07:29 PM, Trond Myklebust wrote:
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.
I forgot to add this in my reply to the cover letter of the original series:
Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
and/or
Tested-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
--
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
