Re: [PATCH v2] block : add larger order folio size instead of pages

Hannes Reinecke
2024-05-05T12:10:21Z

On 5/4/24 18:32, Matthew Wilcox wrote:
> On Sat, May 04, 2024 at 02:35:15PM +0200, Hannes Reinecke wrote:
>>> I think this is wandering into a minefield. I'm pretty sure
>>> it's considered valid to split the bio, and complete the two halves
>>> independently. Each one will put the refcounts for the pages it touches,
>>> and if we do this early putting of references, that's going to fail.
>>
>> Precisesly my worries. Something I want to talk to you about at LSF;
>> refcounting of folios vs refcounting of pages.
>> When one takes a refcount on a folio we are actually taking a refcount
>> on the first page, which is okay if we stick with using the folio throughout
>> the call chain. But if we start mixing between pages and folios (as we do
>> here) we will be getting the refcount wrong.
>>
>> Do you have plans how we could improve the situation?
>> Like a warning 'Hey, you've used the folio for taking the reference, but now
>> you are releasing the references for the page'?
>
> This is a fairly common misunderstanding, but TLDR: problem solved long
> before I started this project.
>
> Individual pages don't actually have a refcount. I know it looks
> like they do, and they kind of do, but for tail pages, the refcount is
> always 0. Functions like get_page() and put_page() always operate on
> the head page (ie folio) refcount.
>
Precisely.

> Specifically, I think you're concerned about pages coming from GUP.
> Take a look at try_get_folio(). We pass in a struct page, explicitly
> get the refcount on a folio, check the page is still part of the
> folio, then return the folio. And we return the page to the caller
> because the caller needs to know the precise page at that address,
> not the folio that contains it.
>
> There are functions which don't surreptitiously call compound_head()
> behind your back. set_page_count(), for example. And page_ref_count()
> (rather than the more normal page_count()).
>
> And none of this is true if you don't use __GFP_COMP. But let's call
> that an aberration that must die.

Ah, right. So the refcount for a page is always unwound to use the
refcount of the enclosing folio.

I was actually concerned with the iov_iter functions, where we take a
reference for each page. Currently iov_iter is iterating in units of
PAGE_SIZE, so there is no easy way of converting that to folios.

But one step at a time, I guess. First get the blocksize > pagesize
patches in.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich