Date:   Wed, 15 Mar 2023 04:40:47 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Ritesh Harjani <ritesh.list@gmail.com>
Cc:     Theodore Tso <tytso@mit.edu>,
        Andreas Dilger <adilger.kernel@dilger.ca>,
        linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 25/31] ext4: Convert ext4_block_write_begin() to take a
 folio
Message-ID: <ZBFMT6L0QqpDcWNm@casper.infradead.org>
References: <ZAWj4FHczOQwwEbK@casper.infradead.org>
 <87r0u129di.fsf@doe.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87r0u129di.fsf@doe.com>
Precedence: bulk

On Mon, Mar 06, 2023 at 08:51:45PM +0530, Ritesh Harjani wrote:
> Matthew Wilcox <willy@infradead.org> writes:
> 
> > On Mon, Mar 06, 2023 at 12:21:48PM +0530, Ritesh Harjani wrote:
> >> "Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
> >>
> >> > All the callers now have a folio, so pass that in and operate on folios.
> >> > Removes four calls to compound_head().
> >>
> >> Why do you say four? Isn't it 3 calls of PageUptodate(page) which
> >> removes calls to compound_head()? Which one did I miss?
> >>
> >> > -	BUG_ON(!PageLocked(page));
> >> > +	BUG_ON(!folio_test_locked(folio));
> >
> > That one ;-)
> 
> __PAGEFLAG(Locked, locked, PF_NO_TAIL)
> 
> #define __PAGEFLAG(uname, lname, policy)				\
> 	TESTPAGEFLAG(uname, lname, policy)				\
> 	__SETPAGEFLAG(uname, lname, policy)				\
> 	__CLEARPAGEFLAG(uname, lname, policy)
> 
> #define TESTPAGEFLAG(uname, lname, policy)				\
> static __always_inline bool folio_test_##lname(struct folio *folio)	\
> { return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }	\
> static __always_inline int Page##uname(struct page *page)		\
> { return test_bit(PG_##lname, &policy(page, 0)->flags); }
> 
> How? PageLocked(page) doesn't do any compount_head() calls no?

You missed one piece of the definition ...

#define PF_NO_TAIL(page, enforce) ({                                    \
                VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page);     \
                PF_POISONED_CHECK(compound_head(page)); })