Re: [PATCH v7 1/3] mm: add AS_WRITEBACK_INDETERMINATE mapping flag

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Apr 9, 2025 at 3:05 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Fri, Apr 04, 2025 at 10:13:55PM +0200, David Hildenbrand wrote:
> > On 04.04.25 22:09, Joanne Koong wrote:
> > > On Fri, Apr 4, 2025 at 12:13 PM David Hildenbrand <david@redhat.com> wrote:
> > > >
> > > > On 04.04.25 20:14, Joanne Koong wrote:
> > > > > Add a new mapping flag AS_WRITEBACK_INDETERMINATE which filesystems may
> > > > > set to indicate that writing back to disk may take an indeterminate
> > > > > amount of time to complete. Extra caution should be taken when waiting
> > > > > on writeback for folios belonging to mappings where this flag is set.
> > > > >
> > > > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > > > > Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > > > Acked-by: Miklos Szeredi <mszeredi@redhat.com>
> > > > > ---
> > > > >    include/linux/pagemap.h | 11 +++++++++++
> > > > >    1 file changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> > > > > index 26baa78f1ca7..762575f1d195 100644
> > > > > --- a/include/linux/pagemap.h
> > > > > +++ b/include/linux/pagemap.h
> > > > > @@ -210,6 +210,7 @@ enum mapping_flags {
> > > > >        AS_STABLE_WRITES = 7,   /* must wait for writeback before modifying
> > > > >                                   folio contents */
> > > > >        AS_INACCESSIBLE = 8,    /* Do not attempt direct R/W access to the mapping */
> > > > > +     AS_WRITEBACK_INDETERMINATE = 9, /* Use caution when waiting on writeback */
> > > > >        /* Bits 16-25 are used for FOLIO_ORDER */
> > > > >        AS_FOLIO_ORDER_BITS = 5,
> > > > >        AS_FOLIO_ORDER_MIN = 16,
> > > > > @@ -335,6 +336,16 @@ static inline bool mapping_inaccessible(struct address_space *mapping)
> > > > >        return test_bit(AS_INACCESSIBLE, &mapping->flags);
> > > > >    }
> > > > >
> > > > > +static inline void mapping_set_writeback_indeterminate(struct address_space *mapping)
> > > > > +{
> > > > > +     set_bit(AS_WRITEBACK_INDETERMINATE, &mapping->flags);
> > > > > +}
> > > > > +
> > > > > +static inline bool mapping_writeback_indeterminate(struct address_space *mapping)
> > > > > +{
> > > > > +     return test_bit(AS_WRITEBACK_INDETERMINATE, &mapping->flags);
> > > > > +}
> > > > > +
> > > > >    static inline gfp_t mapping_gfp_mask(struct address_space * mapping)
> > > > >    {
> > > > >        return mapping->gfp_mask;
> > > >
> > > > Staring at this again reminds me of my comment in [1]
> > > >
> > > > "
> > > > b) Call it sth. like AS_WRITEBACK_MIGHT_DEADLOCK_ON_RECLAIM to express
> > > >        that very deadlock problem.
> > > > "
> > > >
> > > > In the context here now, where we really only focus on the reclaim
> > > > deadlock that can happen for trusted FUSE servers during reclaim, would
> > > > it make sense to call it now something like that?
> > >
> > > Happy to make this change. My thinking was that
> > > 'AS_WRITEBACK_INDETERMINATE' could be reused in the future for stuff
> > > besides reclaim, but we can cross that bridge if that ends up being
> > > the case.
> >
> > Yes, but I'm afraid one we start using it in other context we're reaching
> > the point where we are trying to deal with untrusted user space and the page
> > lock would already be a similar problem.
> >
> > Happy to be wrong on this one.
> >
> > Wait for other opinions first. Apart from that, no objection from my side.
>
> I am on-board with keeping it specific to reclaim deadlock avoidance and
> naming it such.

Sounds good, I will submit v8 with this renamed to
AS_WRITEBACK_MIGHT_DEADLOCK_ON_RECLAIM.

Thanks,
Joanne