On Mon, Jul 12, 2010 at 11:48 AM, Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> On 07/09/2010 07:39 PM, andros@xxxxxxxxxx wrote:
>> e keep the nfs_inode->layout when the segs list is empty, and we remove it
>> from the nfs_client cl_layouts list, but we fail to reset the other fields.
>> Re-initialize the layout (all except for the refcount) so that the next
>> layoutget with potentially new deviceid.... sets the layout fields.
>>
>> Note: API change to layoutdriver_io_operations free_layout
>>
>> 0001-SQUASHME-pnfs-submit-reinitialize-pnfs_layout_type.patch
>> 0002-SQUASHME-pnfs-submit-file-layout-free_layout-init_on.patch
>>
>> Tested
>>
>> CONFIG_V4_I set:
>> Connectathon tests pass against GFS2/pNFS and pyNFS file layout servers. Both
>> return-on-close and not return-on-close tested.
>>
>> CONFIG_V4_I not set:
>> NFSv4.0 mount passes Connectathon tests.
>>
>> -->Andy
>>
>
> Andy sorry to get on your case. But I hate it. It is an hack that takes
> us back not forward. (Code less clean more obscure)
>
> For starts don't save me a vector please. The two functions are unrelated
> see your last patch 0002 there is no common code and a return in the middle
> of an if() which should trigger an alarm for a bad API.
OK, that would be more clear.
>
> Second if at all, then the costume is to initialize on re-use not on end
> of previous operation. (Clearer intentions, might not be reused, etc...)
We need to zero the stateid at the return of the last layout segment.
Once we return the last segment, we signal the server that the layout
stateid is no longer in use and it can dispose of the layout state.
8.2.1. Stateid Types
A stateid represents the set of all layouts held by a particular
client for a particular filehandle with a given layout type.
12.5.3. Layout Stateid
As with all other stateids, the layout stateid consists of a "seqid"
and "other" field. Once a layout stateid is established, the "other"
field will stay constant unless the stateid is revoked or the client
returns all layouts on the file and the server disposes of the
stateid.
Once the stateid is no longer valid, the rest of the layout follows suit.
>
> 3rd, if you'd follow my original proposal you'll see that this hack is not
> needed.
If by you're original proposal you mean have the nfs_inode->layout
follow the life of the nfs_inode it is indeed needed! You still need
to zero the stateid at the return of the last layout segment. You
still can't assume that any fields of the layout received from a
LAYOUTGET after you cleared the nfs_inode->layout->segs list will be
the same as the old layout returned. So, you need to re-initialize the
nfs_inode->layout.
>
> If these patches fix an actual bug you've been hitting, could you just fix
> the particular field in question that needs zeroing. And we can see how to
> solve it better.
The actual bug we are hitting is that we keep the stateid past the
last layout segment, and actually re-use it on a subsequent LAYOUTGET
(send_layout)
I should have added this to the patch comments.
-->Andy
>
> Thanks
> Boaz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
