Re: [PATCH v4 15/16] gfs2: add pre-content fsnotify hook to fault

Jan Kara
2024-08-29T11:43:34Z

On Thu 29-08-24 13:26:17, Amir Goldstein wrote:
> On Thu, Aug 29, 2024 at 1:15 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 14-08-24 17:25:33, Josef Bacik wrote:
> > > gfs2 takes the glock before calling into filemap fault, so add the
> > > fsnotify hook for ->fault before we take the glock in order to avoid any
> > > possible deadlock with the HSM.
> > >
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >
> > The idea of interactions between GFS2 cluster locking and HSM gives me
> > creeps. But yes, this patch looks good to me. Would be nice to get ack from
> > GFS2 guys. Andreas?
>
> If we are being honest, I think that the fact that HSM events require careful
> handling in ->fault() and not to mention no documentation of this fact,
> perhaps we should let HSM events be an opt-in file_system_type feature?
>
> Additionally, we had to introduce FS_DISALLOW_NOTIFY_PERM
> and restrict sb marks on SB_NOUSER, all because these fanotify
> features did not require fs opt-in to begin with.
>
> I think we would be repeating this mistake if we do not add
> FS_ALLOW_HSM from the start.
>
> After all, I cannot imagine HSM being used on anything but
> the major disk filesystems.
>
> Hmm?

Yeah, I was considering this already when thinking about btrfs quirks with
readahead and various special filesystem ioctls and I agree that a need to be
careful with page faults is another good reason to make this a
per-filesystem opt in. Will you send a patch?

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR