Re: [PATCH 1/3] pnfs_submit: move layout segment valid test

[Benny Halevy <bhalevy@panasas.com>]

On 2010-10-07 15:37, andros@netapp.com wrote:
> From: Andy Adamson <andros@rhel6-1.androsmac.org>
> 
> Do not get_lseg for a non-valid lseg.
> Prepare for calling put_lseg outside of inode i_lock.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/pnfs.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 6b2a95d..24620cf 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -845,7 +845,8 @@ pnfs_has_layout(struct pnfs_layout_hdr *lo,
>  	list_for_each_entry(lseg, &lo->segs, fi_list) {
>  		if (is_matching_lseg(lseg, range)) {
>  			ret = lseg;
> -			get_lseg(ret);
> +			if (lseg->valid)
> +				get_lseg(ret);

We shouldn't be hiding this inside pnfs_has_layout
and have different side effects in the different cases.
Since we're called under the lock, pnfs_has_layout
can just return the lseg and never get a reference and its
caller can take it if necessary before releasing the lock.

Also, it doesn't need to be EXPORT_SYMBOL_GPLed as it's
not used outside of the nfs client module.

Benny

>  			break;
>  		}
>  		if (cmp_layout(range, &lseg->range) > 0)
> @@ -889,7 +890,6 @@ pnfs_update_layout(struct inode *ino,
>  	/* Check to see if the layout for the given range already exists */
>  	lseg = pnfs_has_layout(lo, &arg);
>  	if (lseg && !lseg->valid) {
> -		put_lseg_locked(lseg);
>  		/* someone is cleaning the layout */
>  		lseg = NULL;
>  		goto out_unlock;