Re: [PATCH v2 06/10] nfsd: Protect the nfs4_file delegation fields using the fi_lock

[Christoph Hellwig <hch@infradead.org>]

>  	dp->dl_stid.sc_type = NFS4_CLOSED_DELEG_STID;
> +	spin_lock(&fp->fi_lock);
>  	list_del_init(&dp->dl_perclnt);
>  	list_del_init(&dp->dl_recall_lru);
> -	spin_lock(&fp->fi_lock);
>  	list_del_init(&dp->dl_perfile);
>  	spin_unlock(&fp->fi_lock);
> -	spin_unlock(&state_lock);
>  	if (fp) {
>  		nfs4_put_deleg_lease(fp);
> -		put_nfs4_file(fp);
>  		dp->dl_file = NULL;
>  	}
> +	spin_unlock(&state_lock);
> +	put_nfs4_file(fp);
>  }

Unless I've missed something put_nfs4_file does not handle a NULL
pointer argument in the current tree, so this needs a NULL check
for fp.

> +	spin_lock(&state_lock);
> +	/* Did the lease get broken before we took the lock? */
> +	status = -EAGAIN;
> +	if (!file_has_lease(fl->fl_file))
> +		goto out_unlock;

So if the delegation we just tried to add got broken, but someone else
managed to add one this would also return true.  But could that happen
either in real life or in theory?  Shouldn't we instead have an
atomic flag on the delegation that it got broken which we could check
here?