Re: [PATCH 11/13] mm/writeback: add folio_mark_dirty_lock()

[Joanne Koong <joannelkoong@gmail.com>]

On Fri, Oct 18, 2024 at 1:01 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> On Wed, Oct 02, 2024 at 09:52:51AM -0700, Joanne Koong wrote:
> > Add a new convenience helper folio_mark_dirty_lock() that grabs the
> > folio lock before calling folio_mark_dirty().
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > ---
> >  include/linux/mm.h  |  1 +
> >  mm/page-writeback.c | 12 ++++++++++++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index ecf63d2b0582..446d7096c48f 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2539,6 +2539,7 @@ struct kvec;
> >  struct page *get_dump_page(unsigned long addr);
> >
> >  bool folio_mark_dirty(struct folio *folio);
> > +bool folio_mark_dirty_lock(struct folio *folio);
> >  bool set_page_dirty(struct page *page);
> >  int set_page_dirty_lock(struct page *page);
> >
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index fcd4c1439cb9..9b1c95dd219c 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -2913,6 +2913,18 @@ bool folio_mark_dirty(struct folio *folio)
> >  }
> >  EXPORT_SYMBOL(folio_mark_dirty);
> >
>
> I think you should include the comment description from set_page_dirty_lock() as
> well here, generally good to keep documentation consistent.  Thanks,

Looking at this some more, for v2 I am going to replace
set_page_dirty_lock() to be a wrapper around folio_mark_dirty_lock(),
eg something like

+int set_page_dirty_lock(struct page *page)
+{
+       return folio_mark_dirty_lock(page_folio(page));
+}
+EXPORT_SYMBOL(set_page_dirty_lock);

so that we have one source of truth for the logic. I'll remove your
Reviewed-by for this patch in v2 in case you have disagreements on the
newer v2 implementation.


Thanks,
Joanne

>
> Josef