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

[Fredric Isaman]

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, or should a patch 
be sent in removing references to it?


 	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
>