[pnfs] [PATCH 16/37] pnfs: allocate layout driver data in one piece with pnfs_layout_type

[Benny Halevy]

On Jan. 03, 2008, 0:39 +0200, Dean Hildebrand <seattleplus at gmail.com> wrote:
>> layout driver data comes right after the pnfs_layout_type fields
>> and can be accessed with the PNFS_LD_DATA accessor function.
> 
> 
>>  
>>  /* Free a filelayout layout structure
>> @@ -429,17 +418,11 @@ void
>>  filelayout_free_layout(struct pnfs_layout_type **layoutidp,
>>  		       struct nfs4_pnfs_layout_segment *range)
>>  {
>> -	struct nfs4_filelayout *nfslay = NULL;
>>  	struct pnfs_layout_type *layoutid;
>>  
>>  	dprintk("NFS_FILELAYOUT: freeing layout\n");
>>  
>>  	layoutid = *layoutidp;
>> -	if (layoutid)
>> -		nfslay = (struct nfs4_filelayout *)layoutid->layoutid;
>> -	if (nfslay != NULL)
>> -		kfree(nfslay);
>> -	kfree(layoutid);
>>  	*layoutidp = NULL;
>>  }
>>  
>>   
> The above code seems to remove some parts of the function but leaves the 
> "layoutid = *layoutidp" part for some reason?  Where does the memory get 
> freed?

Oops. You're right, kfree(layoutid) should've stayed.
Thanks!

> 
> There seems to be a lot more happening in this patch than just a 
> conversion to accessor functions as described at the top.  Does it need 
> a more expansive comment or is extra code being included?

What exactly? The patch just replaces the separate allocations of
pnfslayout and pnfslayout->layoutid with a single allocation of the unified
structure and replaces all references to pnfslayout->layoutid to
PNFS_LD_DATA(pnfslayout).

> 
> 
>> @@ -459,9 +442,7 @@ filelayout_set_layout(struct pnfs_layout_type *layoutid,
>>  
>>  	if (!layoutid)
>>  		goto nfserr;
>> -	fl = (struct nfs4_filelayout *)layoutid->layoutid;
>> -	if (!fl)
>> -		goto nfserr;
>> +	fl = PNFS_LD_DATA(layoutid);
>>  
>>  	READ32(fl->dev_id);
>>  	READ32(nfl_util);
>> @@ -503,7 +484,6 @@ filelayout_commit(struct pnfs_layout_type *layoutid, struct list_head *pages,
>>  {
>>  	struct inode *ino = PNFS_INODE(layoutid);
>>  	struct nfs_write_data   *dsdata = NULL;
>> -	struct pnfs_layout_type *laytype;
>>  	struct nfs4_filelayout *nfslay;
>>  	struct nfs4_pnfs_dev_item *dev;
>>  	struct nfs4_pnfs_dev *fdev;
>> @@ -514,8 +494,7 @@ filelayout_commit(struct pnfs_layout_type *layoutid, struct list_head *pages,
>>  	struct list_head *pos, *tmp;
>>  	int i;
>>  
>> -	laytype = NFS_I(ino)->current_layout;
>> -	nfslay = (struct nfs4_filelayout *)layoutid->layoutid;
>> +	nfslay = PNFS_LD_DATA(layoutid);
>>  
>>  	dprintk("%s data %p pnfs_client %p nfslay %p\n",
>>  			__FUNCTION__, data, data->pnfs_client, nfslay);
>> @@ -599,7 +578,7 @@ out_bad:
>>  ssize_t
>>  filelayout_get_stripesize(struct pnfs_layout_type *layoutid)
>>  {
>> -	struct nfs4_filelayout *fl = (struct nfs4_filelayout *)layoutid->layoutid;
>> +	struct nfs4_filelayout *fl = PNFS_LD_DATA(layoutid);
>>  	ssize_t stripesize = fl->stripe_unit;
>>  	return stripesize;
>>  }
>> diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
>> index 6a98f59..d862aac 100644
>> --- a/include/linux/nfs4_pnfs.h
>> +++ b/include/linux/nfs4_pnfs.h
>> @@ -37,9 +37,9 @@ struct pnfs_mount_type {
>>   * A reference is stored in the nfs_inode structure.
>>   */
>>  struct pnfs_layout_type {
>> -	void *layoutid;
>> -	int roc_iomode;	/* iomode to return on close, 0=none */
>> +	int roc_iomode;			/* iomode to return on close, 0=none */
>>  	struct inode *inode;
>> +	u8 ld_data[];			/* layout driver private data */
>>  };
>>  
>>  static inline struct inode *
>> @@ -66,6 +66,12 @@ PNFS_MOUNTID(struct pnfs_layout_type *lo)
>>  	return NFS_SERVER(PNFS_INODE(lo))->pnfs_mountid;
>>  }
>>  
>> +static inline void *
>> +PNFS_LD_DATA(struct pnfs_layout_type *lo)
>> +{
>> +	return lo->ld_data;
>> +}
>> +
>>  static inline struct pnfs_layoutdriver_type *
>>  PNFS_LD(struct pnfs_layout_type *lo)
>>  {
>>