Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > It's fairly important to be able to do streaming writes without having > > to read the old contents for some loads. And read-modify-write cycles > > are death for performance, so you really want to coalesce writes until > > you have the whole page. > > I completely agree with you. The context you're missing is that Dave > wants to do RMW twice. He doesn't do the delaying SetPageUptodate dance. Actually, I do the delaying of SetPageUptodate in the new write helpers that I'm working on - at least to some extent. For a write of any particular size (which may be more than a page), I only read the first and last pages affected if they're not completely changed by the write. Note that I have my own version of generic_perform_write() that allows me to eliminate write_begin and write_end for any filesystem using it. Keeping track of which regions are dirty allows merging of contiguous dirty regions. It has occurred to me that I don't actually need the pages to be uptodate and completely filled out. I'm tracking which bits are dirty - I could defer reading the missing bits till someone wants to read or mmap. But that kind of screws with local caching. The local cache might need to track the missing bits, and we are likely to be using blocks larger than a page. Basically, there are a lot of scenarios where not having fully populated pages sucks. And for streaming writes, wouldn't it be better if you used DIO writes? > If the write is less than the whole page, AFS, Ceph and anybody else > using netfs_write_begin() will first read the entire page in and mark > it Uptodate. Indeed - but that function is set to be replaced. What you're missing is that if someone then tries to read the partially modified page, you may have to do two reads from the server. > Then he wants to track which parts of the page are dirty (at byte > granularity) and send only those bytes to the server in a write request. Yes. Because other constraints may apply, for example the handling of conflicting third-party writes. The question here is how much we care about that - and that's why I'm trying to write back only what's changed where possible. That said, if content encryption is thrown into the mix, the minimum we can write back is whatever the size of the blocks on which encryption is performed, so maybe we shouldn't care. Add disconnected operation reconnection resolution, where it might be handy to have a list of what changed on a file. > So it's worst of both worlds; first the client does an RMW, then the > server does an RMW (assuming the client's data is no longer in the > server's cache. Actually, it's not necessarily what you make out. You have to compare the server-side RMW with cost of setting up a read or a write operation. And then there's this scenario: Imagine I'm going to modify the middle of a page which doesn't yet exist. I read the bit at the beginning and the bit at the end and then try to fill the middle, but now get an EFAULT error. I'm going to have to do *three* reads if someone wants to read the page. > The NFS code moves the RMW from the client to the server, and that makes > a load of sense. No, it very much depends. It might suck if you have the folio partly cached locally in fscache, and it doesn't work if you have content encryption and would suck if you're doing disconnected operation. I presume you're advocating that the change is immediately written to the server, and then you read it back from the server? > > That said, I suspect it's also *very* filesystem-specific, to the > > point where it might not be worth trying to do in some generic manner. > > It certainly doesn't make sense for block filesystems. Since they > can only do I/O on block boundaries, a sub-block write has to read in > the surrounding block, and once you're doing that, you might as well > read in the whole page. I'm not trying to do this for block filesystems! However, a block filesystem - or even a blockdev - might be involved in terms of the local cache. > Tracking sub-page dirty bits still makes sense. It's on my to-do > list for iomap. /me blinks "bits" as in parts of a page or "bits" as in the PG_dirty bits on the pages contributing to a folio? > > [ goes off and looks. See "nfs_write_begin()" and friends in > > fs/nfs/file.c for some of the examples of these things, althjough it > > looks like the code is less aggressive about avoding the > > read-modify-write case than I thought I remembered, and only does it > > for write-only opens ] > > NFS is missing one trick; it could implement aops->is_partially_uptodate > and then it would be able to read back bytes that have already been > written by this client without writing back the dirty ranges and fetching > the page from the server. As mentioned above, I have been considering the possibility of keeping track of partially dirty non-uptodate pages. Jeff and I have been discussing that we might want support for explicit RMW anyway for various reasons (e.g. doing DIO that's not crypto-block aligned, remote-invalidation/reconnection-resolution handling). David
