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

[Hannes Reinecke <hare@suse.de>]

On 5/3/24 17:26, Matthew Wilcox wrote:
> On Thu, May 02, 2024 at 02:53:40PM +0200, Christoph Hellwig wrote:
>> On Thu, May 02, 2024 at 08:45:33AM +0200, Hannes Reinecke wrote:
>>>> -		nr_pages = (fi.offset + fi.length - 1) / PAGE_SIZE -
>>>> -			   fi.offset / PAGE_SIZE + 1;
>>>> -		do {
>>>> -			bio_release_page(bio, page++);
>>>> -		} while (--nr_pages != 0);
>>>> +		bio_release_page(bio, page);
>>>
>>> Errm. I guess you need to call 'folio_put()' here, otherwise the page
>>> reference counting will be messed up.
>>
>> It shouldn't.  See the rfc patch and explanation that Keith sent in reply
>> to the previous version.  But as I wrote earlier it should be a separate
>> prep patch including a commit log clearly explaining the reason for it
>> and how it works.
> 
> 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'?

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