On Mon, Aug 21, 2023 at 05:25:41AM +0100, Matthew Wilcox wrote:
> On Mon, Aug 21, 2023 at 12:46:37PM +0900, Byungchul Park wrote:
> > @@ -377,44 +421,88 @@ static __always_inline int Page##uname(struct page *page) \
> > #define SETPAGEFLAG(uname, lname, policy) \
> > static __always_inline \
> > void folio_set_##lname(struct folio *folio) \
> > -{ set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
> > +{ \
> > + set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); \
> > + dept_page_set_bit(&folio->page, PG_##lname); \
>
> The PG_locked and PG_writeback bits only actually exist in the folio;
> the ones in struct page are just legacy and never actually used.
> Perhaps we could make the APIs more folio-based and less page-based?
Yeah. I need to make it more folio-based. I will work on it. Thank you.
> > static __always_inline void SetPage##uname(struct page *page) \
> > -{ set_bit(PG_##lname, &policy(page, 1)->flags); }
> > +{ \
> > + set_bit(PG_##lname, &policy(page, 1)->flags); \
> > + dept_page_set_bit(page, PG_##lname); \
> > +}
>
> I don't think we ever call this for PG_writeback or PG_locked. If
> I'm wrong, we can probably fix that ;-)
Okay then, I will assume this will never be used. So are you asking me
to get rid of this part, right?
> > static __always_inline void __SetPage##uname(struct page *page) \
> > -{ __set_bit(PG_##lname, &policy(page, 1)->flags); }
> > +{ \
> > + __set_bit(PG_##lname, &policy(page, 1)->flags); \
> > + dept_page_set_bit(page, PG_##lname); \
> > +}
>
> Umm. We do call __SetPageLocked() though ... I'll fix those up to
> be __set_folio_locked().
Haha Okay. Lemme know when you get done on it. Thanks.
Byungchul