> On Sep 10, 2022, at 6:13 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > On Sat, Sep 10, 2022 at 09:21:11PM +0000, Chuck Lever III wrote: > >> It's also possible that recent simplifications I've done to the splice >> read actor accidentally removed the ability to deal with compound pages. >> You might want to review the commit history of nfsd_splice_actor() to >> see if there is an historic version that would behave correctly with >> the new copy_page_to_iter(). > > Nah, that's unrelated... > >> Is the need to deal with CompoundPage documented somewhere? If not, >> perhaps nfsd_splice_actor() could mention it so that overzealous >> maintainers don't break it again. > >>> + struct page *page = buf->page; // may be a compound one > > Does that qualify? ;-) Well, no, since you just added it :-) I meant pre-existing documentation of the API. I take your remark as polite encouragement to go and look for it, because this is an area where I need deeper understanding. > FWIW, there's a separate problem in that thing - it assumes that > pipe_buffer boundaries will end up PAGE_SIZE-aligned. Usually > that's going to be true, but foofs_splice_read() is not required to > maintain that. E.g. it might be working in terms of chunks > used by weird protocol used by foofs, with e.g. 1024-byte payloads > + 300-odd bytes of framing/checskums/whatnot. In that case it > might do 1024 bytes per pipe_buffer, with non-zero offset in page > in each of them; normal read()/splice()/etc. would work just fine > with that, but for nfsd_splice_actor() results would not be > nice. > > AFAICS, sunrpc assumes that we have several pages, offset in the > first one and total size; no provisions exist for e.g. 5Kb of > data scattered in 5 chunks over 5 pages. Correct? struct xdr_buf assumes the first page might contain payload that starts somewhere in the middle of that page; the remaining segments of the payload are contiguous page-sized chunks, with the last segment starting on a page boundary but possibly being short of a full page in length. It's not a true scatter-gather arrangement. A 5K payload can only be contained in two pages. Someday we hope to switch the whole stack to use a bvec or something more general than an array of pages, where each segment can represent something that is not page-sized. There is a struct bvec inside of struct xdr_buf already, but not everyone uses that yet. RDMA, for example, still uses SGLs or page vectors... so I've been waiting for some forward movement in our transport APIs before doing more heavy lifting. -- Chuck Lever
