Re: [PATCH v4 16/16] xfs: add pre-content fsnotify hook for write faults

[Jan Kara <jack@suse.cz>]

On Fri 30-08-24 16:28:33, Darrick J. Wong wrote:
> On Thu, Aug 29, 2024 at 01:17:53PM +0200, Jan Kara wrote:
> > On Wed 14-08-24 17:25:34, Josef Bacik wrote:
> > > xfs has it's own handling for write faults, so we need to add the
> > > pre-content fsnotify hook for this case.  Reads go through filemap_fault
> > > so they're handled properly there.
> > > 
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > 
> > Looks good to me but it would be great to get explicit ack from some XFS
> > guy...  Some selection CCed :)
> 
> Looks decent to me, but I wonder why xfs_write_fault has to invoke
> filemap_maybe_emit_fsnotify_event itself?  Can that be done from
> whatever calls ->page_mkwrite and friends?

So we were discussing this already here [1]. The options we have:

1) Call filemap_maybe_emit_fsnotify_event() from filesystem hooks
(filemap_fault() for those who use it). This is a bit ugly because it
requires modification to filesystems with their own fault handlers.

2) Call filemap_maybe_emit_fsnotify_event() from generic code before
calling ->fault() and if we needed to send event (and thus dropped
mmap_lock), we will retry the fault. This requires no special fs awareness
but the ->fault hook will then be called after retry most of the times on
HSM managed fs and thus without possibility to drop mmap_lock making
contention there possibly worse.

3) (I don't think we've discussed this option yet): Call
filemap_maybe_emit_fsnotify_event() in generic code before calling ->fault
and then continue to call ->fault even if we've dropped mmap_lock. This
will require changing calling convention for ->fault as vmf->vma must not
be touched after we've dropped mmap_lock and practically all users end up
using it to get vmf->vma->vm_file. With per-fs opt in flag to enable HSM
events this could be manageable but frankly I'm not convinced the
complicated calling convention would be better outcome than 1). But I'm
open for discussion.
								Honza

[1] https://lore.kernel.org/all/CAOQ4uxgXEzT=Buwu8SOkQG+2qcObmdH4NgsGme8bECObiobfTQ@mail.gmail.com
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR