| Message ID | bce66af61dd98d4f81032b97c73dce09658ae02d.1723144881.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fanotify: add pre-content hooks | expand |
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.
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@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 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
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; + } + return ret; + } else if (fpin) { + fput(fpin); + return VM_FAULT_RETRY; + } + /* Lock ordering: */ if (fdm > mapping) { struct bch_inode_info *fdm_host = to_bch_ei(fdm->host);
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(+)