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

[Benny Halevy]

On May. 07, 2008, 18:29 -0700, Dean Hildebrand <seattleplus at gmail.com> wrote:
> 
> 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?

Not really :-(

(unsigned long)(idx_start * PAGE_SIZE) can overflow when unsigned long
is 32-bit wide so you want to cast idx_start to u64.

Benny


> Dean
>> Benny
>>
>>   
>>> seems.  So I think the real format is %lu.  There is also a indentation 
>>> error.
>>>    Let me send a new patch.
>>> Dean
>>>     
>>