On Fri, Aug 09, 2024 at 03:11:34PM +0200, Amir Goldstein wrote:
> On Thu, Aug 8, 2024 at 9:28 PM Josef Bacik <josef@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > ---
> > 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
Hmm I swear I had bcachefs turned on in my config, I'll fix this and also fix my
config.
>
> > + }
> > + 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?
You should have seen what I had to begin with ;). I agree, I'll rework this to
reduce how much we're carrying around. Thanks,
Josef
