Re: [RESEND PATCH v10 25/25] dept: Track the potential waits of PG_{locked,writeback}

[Byungchul Park]

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