On Mon 23-09-24 12:36:14, Linus Torvalds wrote: > On Mon, 23 Sept 2024 at 12:13, Jan Kara <jack@xxxxxxx> wrote: > > > > Sure, the details are in some of the commit messages but you're right I > > should have summarized them in the pull request as well: > > So I really only looked at the parts I know - the VM side, and > honestly, I threw up in my mouth a bit there. > > Do we really want to call that horrific fsnotify path for the case > where we already have the page cached? This is a fairly critical > fastpath, and not giving out page cache pages means that now you are > literally violating mmap coherency. > > If the aim is to fill in caches on first access, then if we already > have a page cache page, it's by definition not first access any more! Well, that's what actually should be happening. do_read_fault() will do should_fault_around(vmf) -> yes -> do_fault_around() and filemap_map_pages() will insert all pages in the page cache into the page table page before we even get to filemap_fault() calling our fsnotify hooks. Note that filemap_map_pages() returns VM_FAULT_NOPAGE if it has mapped the page for the faulting address which makes the page fault code bail out even before ->fault handler is even called. That being said now that I'm rereading this code again, write faults will always end up in the fault handler so we'll generate PRE_WRITE events for them on each write fault (if someone is watching for it). Not sure if write faults matter that much and I don't see easy way around that as that's the promise of PRE_WRITE event... Do the above explanations make the VM changes acceptable for you? I agree it isn't exactly beautiful but it should work. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
