[pnfs] [PATCH 02/28] pnfs: fix pnfs_{read|write}pages

[Dean Hildebrand]

Fredric Isaman wrote:
> On Thu, 13 Mar 2008, Benny Halevy wrote:
>
>   
>> On Mar. 13, 2008, 20:09 +0200, Fredric Isaman <iisaman at citi.umich.edu> wrote:
>>     
>>> On Wed, 12 Mar 2008, Benny Halevy wrote:
>>>
>>>       
>>>> On Mar. 12, 2008, 18:23 +0200, Fredric Isaman <iisaman at citi.umich.edu> wrote:
>>>>         
>>>>> On Wed, 12 Mar 2008, Benny Halevy wrote:
>>>>>
>>>>>           
>>>>>> On Mar. 11, 2008, 21:31 +0200, Fred Isaman <iisaman at citi.umich.edu> wrote:
>>>>>>             
>>>>>>> THIS WILL BREAK OTHER LAYOUT DRIVERS that rely on old behavior.
>>>>>>>               
>>>>>> Fred, do you have the cycles to fix the files and objects LDs
>>>>>> for the new behavior so we can merge this patch onto the pnfs branch?
>>>>>>
>>>>>>             
>>>>> Yes, I'll work on that.
>>>>>
>>>>>  	Fred
>>>>>           
>>>> Thanks!
>>>>
>>>>
>>>>         
>>> OK, I've been glancing through the code for the other layout drivers.
>>> Basically, in the functions ld_io_ops->{read|write}_pagelist, the
>>> filelayout driver only returns 0, while the panlayout only returns 0 or a
>>> negative error.  So why do we have the checks for positive return
>>> value at all?
>>>       
>> In the past the objects LD used to return a positive count on synchronous
>> completion of the I/O operation (it had nothing to do with falling back
>> to NFS I/O)
>>
>>     
>>> In addition, to clarify the intent of some of the panobject code:
>>>
>>> Does sam_read/sam_write return PAN_ERR_IN_PROGRESS iff called with a
>>> completion callback?
>>>       
>> Not exactly.  It returns that if called with a callback function
>> and the operation continues asynchronously.  If it's already
>> completed (either successfully or aborted with an error) it will
>> return the appropriate success/error status.
>>
>>     
>
> In that case, don't you have the potential for the code in the async 
> case to call the done routines twice, first as the callback, then (if 
> the async call finishes quickly) again in the (rc != PAN_ERR_IN_PROGRESS) 
> condition?
>
>   
>>> Why the check against PNFS_ISSYNC in panlayout_*_done? Don't you still
>>> need to deallocate the memory used by the request?
>>>       
>> Yes.  Historically we wouldn't call the callback on the synchronous
>> return path.  This is no longer true and this code needs to be fixed accordingly.
>>
>>     
>>> On that note, it seems that PNFS_ISSYNC is never currently set/used at
>>> all.  Is this true?
>>>       
>> True.  It used to be set in the 2.6.18.3 codebase in the .{read,write}
>> paths.
>>
>>     
>
> Is this going to be resurrected by Dean's O_SYNC work, 
O_DIRECT :)
> or should a patch be sent in removing references to it?
>   
I can't see why I would use it at this point.
Dean
>
>  	Fred
>
>   
>> Benny
>>
>>     
>>>  	Fred
>>>
>>>
>>>       
>> -- 
>> Benny Halevy
>> Software Architect
>> Tel/Fax: +972-3-647-8340
>> Mobile: +972-54-802-8340
>> US:      +1-412-203-3187
>> bhalevy at panasas.com
>>
>> Panasas, Inc.
>> The Leader in Parallel Storage
>> www.panasas.com
>>
>>     
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>