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

[Benny Halevy]

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...

Benny

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