| Message ID | 20241107235614.3637221-2-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: remove temp page copies in writeback | expand |
On Thu, Nov 07, 2024 at 03:56:09PM -0800, Joanne Koong wrote: > Add a new mapping flag AS_WRITEBACK_MAY_BLOCK which filesystems may set > to indicate that writeback operations may block or 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> > --- > include/linux/pagemap.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 68a5f1ff3301..eb5a7837e142 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_MAY_BLOCK = 9, /* Use caution when waiting on writeback */ To me 'may block' does not feel right. For example in reclaim code, folio_wait_writeback() can get blocked and that is fine. However with non-privileged fuse involved, there are security concerns. Somehow 'may block' does not convey that. Anyways, I am not really pushing back but I think there is a need for better name here. > /* 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_may_block(struct address_space *mapping) > +{ > + set_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); > +} > + > +static inline bool mapping_writeback_may_block(struct address_space *mapping) > +{ > + return test_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); > +} > + > static inline gfp_t mapping_gfp_mask(struct address_space * mapping) > { > return mapping->gfp_mask; > -- > 2.43.5 >
On Fri, Nov 8, 2024 at 4:10 PM Shakeel Butt <shakeel.butt@linux.dev> wrote: > > On Thu, Nov 07, 2024 at 03:56:09PM -0800, Joanne Koong wrote: > > Add a new mapping flag AS_WRITEBACK_MAY_BLOCK which filesystems may set > > to indicate that writeback operations may block or 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> > > --- > > include/linux/pagemap.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 68a5f1ff3301..eb5a7837e142 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_MAY_BLOCK = 9, /* Use caution when waiting on writeback */ > > To me 'may block' does not feel right. For example in reclaim code, > folio_wait_writeback() can get blocked and that is fine. However with > non-privileged fuse involved, there are security concerns. Somehow 'may > block' does not convey that. Anyways, I am not really pushing back but > I think there is a need for better name here. Ahh I see where this naming causes confusion - the "MAY_BLOCK" part could be interpreted in two ways: a) may block as in it's possible for the writeback to block and b) may block as in it's permissible/ok for the writeback to block. I intended "may block" to signify a) but you're right, it could be easily interpreted as b). I'll change this to AS_WRITEBACK_BLOCKING. Thanks, Joanne > > > /* 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_may_block(struct address_space *mapping) > > +{ > > + set_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); > > +} > > + > > +static inline bool mapping_writeback_may_block(struct address_space *mapping) > > +{ > > + return test_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); > > +} > > + > > static inline gfp_t mapping_gfp_mask(struct address_space * mapping) > > { > > return mapping->gfp_mask; > > -- > > 2.43.5 > >
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 68a5f1ff3301..eb5a7837e142 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_MAY_BLOCK = 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_may_block(struct address_space *mapping) +{ + set_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); +} + +static inline bool mapping_writeback_may_block(struct address_space *mapping) +{ + return test_bit(AS_WRITEBACK_MAY_BLOCK, &mapping->flags); +} + static inline gfp_t mapping_gfp_mask(struct address_space * mapping) { return mapping->gfp_mask;
Add a new mapping flag AS_WRITEBACK_MAY_BLOCK which filesystems may set to indicate that writeback operations may block or 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> --- include/linux/pagemap.h | 11 +++++++++++ 1 file changed, 11 insertions(+)