Re: [PATCH v2 14/16] bcachefs: add pre-content fsnotify hook to fault

Amir Goldstein
2024-08-09T13:11:46Z

On Thu, Aug 8, 2024 at 9:28 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> bcachefs has its own locking around filemap_fault, so we have to make
> sure we do the fsnotify hook before the locking. Add the check to emit
> the event before the locking and return VM_FAULT_RETRY to retrigger the
> fault once the event has been emitted.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/bcachefs/fs-io-pagecache.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/fs/bcachefs/fs-io-pagecache.c b/fs/bcachefs/fs-io-pagecache.c
> index a9cc5cad9cc9..359856df52d4 100644
> --- a/fs/bcachefs/fs-io-pagecache.c
> +++ b/fs/bcachefs/fs-io-pagecache.c
> @@ -562,6 +562,7 @@ void bch2_set_folio_dirty(struct bch_fs *c,
> vm_fault_t bch2_page_fault(struct vm_fault *vmf)
> {
> struct file *file = vmf->vma->vm_file;
> + struct file *fpin = NULL;
> struct address_space *mapping = file->f_mapping;
> struct address_space *fdm = faults_disabled_mapping();
> struct bch_inode_info *inode = file_bch_inode(file);
> @@ -570,6 +571,18 @@ vm_fault_t bch2_page_fault(struct vm_fault *vmf)
> if (fdm == mapping)
> return VM_FAULT_SIGBUS;
>
> + ret = filemap_maybe_emit_fsnotify_event(vmf, &fpin);
> + if (unlikely(ret)) {
> + if (fpin) {
> + fput(fpin);
> + ret |= VM_FAULT_RETRy;

Typo RETRy

> + }
> + return ret;
> + } else if (fpin) {
> + fput(fpin);
> + return VM_FAULT_RETRY;
> + }
> +

This chunk is almost duplicate in all call sites in filesystems.
Could it maybe be enclosed in a helper.
It is bad enough that we have to spray those in filesystem code,
so at least give the copy&paste errors to the bare minimum?

Thanks,
Amir.