[pnfs] [PATCH 1/5] pnfs: Several non-critical fixups.

[Dean Hildebrand]

Benny Halevy wrote:
> On May. 07, 2008, 15:13 -0700, Dean Hildebrand <seattleplus at gmail.com> wrote:
>   
>> Benny Halevy wrote:
>>     
>>> On May. 06, 2008, 16:45 -0700, Dean Hildebrand <seattleplus at gmail.com> wrote:
>>> <snip>
>>>
>>>   
>>>       
>>>> @@ -499,10 +499,11 @@ decode_and_add_ds(uint32_t **pp, struct filelayout_mount_type *mt)
>>>>  	 */
>>>>  	if (!ds->ds_clp) {
>>>>  		err = nfs4_pnfs_ds_create(mds_srv, ds);
>>>> -		printk(KERN_ERR
>>>> -		       "%s nfs4_pnfs_ds_create returned %d\n", __func__, err);
>>>> -		if (err)
>>>> +		if (err) {
>>>> +			printk(KERN_ERR "%s nfs4_pnfs_ds_create error %d\n",
>>>> +			       __func__, err);
>>>>  			goto out_err;
>>>> +		}
>>>>  	}
>>>>     
>>>>         
>>> Why is this non-critical?
>>> This looks like a real bug fix to me.... 
>>>   
>>>   
>>>       
>>>> @@ -1184,7 +1184,7 @@ pnfs_update_layout_commit(struct inode *inode,
>>>>  	struct nfs_page *nfs_page = nfs_list_entry(head->next);
>>>>  	int status;
>>>>  
>>>> -	dprintk("--> %s inode %p layout range: %Zd@%Lu\n", __func__, inode,
>>>> +	dprintk("--> %s inode %p layout range: %Zd@%Zu\n", __func__, inode,
>>>>  				(size_t)(npages * PAGE_SIZE),
>>>>  				(loff_t)idx_start * PAGE_SIZE);
>>>>  
>>>>     
>>>>         
>>> Why is that?
>>> Do you get a warning in your environment?
>>> How about this instead?
>>>
>>>  	dprintk("--> %s inode %p layout range: %Zd@%Lu\n", __func__, inode,
>>>  				(size_t)(npages * PAGE_SIZE),
>>> -				(loff_t)idx_start * PAGE_SIZE);
>>> +				(u64)idx_start * PAGE_SIZE);
>>>   
>>>       
>> I don't get any warnings on 64 bit, but good catch.  There is a problem, 
>> but I think the main culprit is the parenthesis.  pgoff_t is unsigned 
>> long, so casting it to loff_t is actually wrong, and so is u64 it 
>>     
>
> You want to do that to prevent overflow when multiplying with PAGE_SIZE
> on 32-bit machines...
>   
I'm not sure what you mean by *that*. So you like my updated patch?
Dean
> Benny
>
>   
>> seems.  So I think the real format is %lu.  There is also a indentation 
>> error.
>>    Let me send a new patch.
>> Dean
>>     
>
>