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@xxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxx -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
