Re: [PATCH 13/15] pnfs: add CB_LAYOUTRECALL handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2010-12-23 at 15:08 -0500, Fred Isaman wrote:
> On Dec 23, 2010, at 2:25 PM, Trond Myklebust wrote:
> 
> > On Thu, 2010-12-23 at 12:29 -0500, Fred Isaman wrote:
> >> +
> >> +	if (args->cbl_recall_type == RETURN_FILE) {
> >> +		LIST_HEAD(free_me_list);
> >> +
> >> +		args->cbl_inode = NULL;
> >> +		spin_lock(&clp->cl_lock);
> >> +		list_for_each_entry(lo, &clp->cl_layouts, plh_layouts) {
> >> +			if (nfs_compare_fh(&args->cbl_fh,
> >> +					   &NFS_I(lo->plh_inode)->fh))
> >> +				continue;
> >> +			if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags))
> >> +				rv = NFS4ERR_DELAY;
> >> +			else {
> >> +				/* Without this, layout can be freed as soon
> >> +				 * as we release cl_lock.  Matched in
> >> +				 * do_callback_layoutrecall.
> >> +				 */
> >> +				get_layout_hdr(lo);
> >> +				args->cbl_inode = lo->plh_inode;
> >> +				rv = NFS4_OK;
> >> +			}
> >> +			break;
> >> +		}
> >> +		spin_unlock(&clp->cl_lock);
> >> +
> >> +		spin_lock(&lo->plh_inode->i_lock);
> > 
> > What guarantees do you have that the inode still exists here? Does the
> > layout segment hold a reference to it?
> > 
> 
> This section is why I once asked about whether the layout needs to reference the inode.  I believe the answer is nothing.  The lseg/layout definitely does not hold direct references.  I assume doing grab/put of inode here and below is acceptable?

igrab/iput is fine in this context.

There is also the question: what if we don't find a match for the layout
recall arguments? It looks to me as if we end up with 'lo ==
clp->cl_layouts', with disastrous consequences when we then do
spin_lock(lo->plh_inode->i_lock)...

> > Also, How do you know that (!test_bit(NFS_LAYOUT_BULK_RECALL,
> > &lo->plh_flags)) is still valid when you get here?
> > 
> 
> Because it can not be set elsewhere while the NFS4CLNT_LAYOUTRECALL bit is set.

OK. One race averted :-)

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux