[pnfs] [PATCH 21/38] [v1] pnfs: client layout cache: {get_lock, put_unlock}_current_layout

[Benny Halevy]

On Jan. 08, 2008, 1:57 +0200, Dean Hildebrand <seattleplus at gmail.com> wrote:
> 
> Benny Halevy wrote:
>>  
>>  /*
>> - * get, possibly allocate current_layout
>> + * get, possibly allocate, and lock current_layout
>>   
> What I wouldn't mind here is a comment saying something to the effect of:
> "Grab's lock on layout.  If successful, caller must unlock current_layout
> by  ___fill in how layout lock is released____"
> 
> I think this will help future developers figure out the start and end 
> points for the layout lock.
> Thanks for re-sending the patches btw.

Sore, no problem, and thanks for the feedback!

Benny

> Dean
>>   */
>>  static struct pnfs_layout_type *
>>  get_alloc_layout(struct inode *ino,
>> @@ -467,7 +509,7 @@ get_alloc_layout(struct inode *ino,
>>  
>>  	dprintk("%s Begin\n", __FUNCTION__);
>>  
>> -	while ((lo = nfsi->current_layout) == NULL) {
>> +	while ((lo = get_lock_current_layout(nfsi)) == NULL) {
>>  		/* Compete against other threads on who's doing the allocation,
>>  		 * wait until bit is cleared if we lost this race.
>>  		 */
>> @@ -480,22 +522,22 @@ get_alloc_layout(struct inode *ino,
>>  		}
>>  
>>  		/* Was current_layout already allocated while we slept?
>> -		 * If not, allocate it.
>> +		 * If so, retry get_lock'ing it. Otherwise, allocate it.
>>  		 */
>> -		lo = nfsi->current_layout;
>> -		if (!lo)
>> -			lo = nfsi->current_layout =
>> -				alloc_init_layout(ino, io_ops);
>> +		if (nfsi->current_layout)
>> +			continue;
>> +
>> +		lo = alloc_init_layout(ino, io_ops);
>> +		if (lo) {
>> +			/* must grab the layout lock */
>> +			spin_lock(&nfsi->lo_lock);
>>   
>> +			nfsi->current_layout = lo;
>> +		} else
>> +			lo = ERR_PTR(-ENOMEM);
>>  
>>  		/* release the NFS_INO_LAYOUT_ALLOC bit and wake up waiters */
>>  		clear_bit_unlock(NFS_INO_LAYOUT_ALLOC, &nfsi->pnfs_layout_state);
>>  		wake_up_bit(&nfsi->pnfs_layout_state, NFS_INO_LAYOUT_ALLOC);
>> -
>> -		/* we're done here.
>> -		 * just check whether alloc_init_layout succeeded.
>> -		 */
>> -		if (!lo)
>> -			lo = ERR_PTR(-ENOMEM);
>>  		break;
>>  	}
>>  
>> @@ -505,7 +547,6 @@ get_alloc_layout(struct inode *ino,
>>  	else
>>  		dprintk("%s Return error %ld\n", __FUNCTION__, PTR_ERR(lo));
>>  #endif
>> -	return lo;
>>  }
>>  
>>  /* Update the file's layout for the given range and iomode.
>> @@ -631,6 +672,7 @@ out:
>>  
>>  	/* res.layout.buf kalloc'ed by the xdr decoder? */
>>  	kfree(res.layout.buf);
>> +	put_unlock_current_layout(nfsi, layout_new);
>>  ret:
>>  	dprintk("%s end (err:%d) state 0x%lx\n",
>>  		__FUNCTION__, result, nfsi->pnfs_layout_state);
>> @@ -721,9 +763,11 @@ pnfs_getboundary(struct inode *inode)
>>  		goto out;
>>  
>>  	nfsi = NFS_I(inode);
>> -	lo = nfsi->current_layout;
>> -	if (lo)
>> +	lo = get_lock_current_layout(nfsi);;
>> +	if (lo) {
>>  		stripe_size = policy_ops->get_stripesize(lo);
>> +		put_unlock_current_layout(nfsi, lo);
>> +	}
>>  out:
>>  	return stripe_size;
>>  }
>> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
>> index 1a093b1..99dfe9c 100644
>> --- a/include/linux/nfs_fs.h
>> +++ b/include/linux/nfs_fs.h
>> @@ -181,6 +181,7 @@ struct nfs_inode {
>>  #define NFS_INO_LAYOUT_ALLOC	0x0002	/* get layout failed, stop trying */
>>  	time_t pnfs_layout_suspend;
>>  	wait_queue_head_t lo_waitq;
>> +	spinlock_t lo_lock;
>>  	struct pnfs_layout_type *current_layout;
>>  	/* use rpc_creds in this open_context to send LAYOUTCOMMIT to MDS */
>>  	struct nfs_open_context *layoutcommit_ctx;
>>   
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>