On May 14, 2014, at 10:26 AM, Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx> wrote: > On 5/13/2014 4:44 PM, Chuck Lever wrote: >> >>> I'm not sure what "segment count” you are talking about? >> AFAICT a chunk is a list of segments. Thus one chunk can reference >> many pages, one per segment. > > Ok yes. rdma_read_chunk_frmr()/rdma_read_chunk_lcl() walk the segments creating rdma read work requests to pull the data for this chunk. > >> If you print ch_count, it is 2 for NFS WRITEs from a Linux client, >> no matter how large the write payload is. Therefore I think the check >> as it is written is not particularly useful. > > Why are there 2? The first chunk lists the pages the server is to read, and the second chunk has the zero pad for XDR alignment. If pad optimization is enabled on the client, there is just 1 chunk in the RPC’s Read list. > >> The returned ch_count here is not used beyond that check, after the >> refactoring patch is applied. The returned byte_count includes all >> chunks in the passed-in chunk list, whereas I think it should count >> only the first chunk in the list. > > So I'll investigate this more with the goal of only grabbing the first chunk. [ . . . snipped . . . ] >>>> >>>> What I'm asking is: >>>> >>>> Do you agree the server should be changed to comply with section 4 of 5667? >>>> >>> I think it is complying and you are misinterpreting. But I could be misinterpreting it >>> too :) >>> >>> But we should fix the pad thing. >> That may be enough to support a client that does not send the >> pad bytes in a separate chunk. However, I wonder how such a >> server would behave with the current client. >> >> True, this isn’t a regression, per se. The server is already >> behaving incorrectly without the refactoring patch. >> >> However, Tom’s refactoring rewrites all this logic anyway, and >> it wouldn’t be much of a stretch to make the server comply with >> RFC 5667, at this point. > > Can we defer this until the refactor patch is in, then I'll work on a new patch for the pad stuff. Yes, we can defer it. A bug has been filed to track this issue: https://bugzilla.linux-nfs.org/show_bug.cgi?id=246 -- Chuck Lever chucklever@xxxxxxxxx -- 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
