Re: [PATCH v8 1/5] block: Added folio-ized version of bvec_try_merge_hw_page()

[Kundan Kumar <kundan.kumar@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On 17/08/24 05:23AM, Matthew Wilcox wrote:
>On Thu, Jul 11, 2024 at 10:37:46AM +0530, Kundan Kumar wrote:
>> -bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv,
>> -		struct page *page, unsigned len, unsigned offset,
>> +bool bvec_try_merge_hw_folio(struct request_queue *q, struct bio_vec *bv,
>> +		struct folio *folio, size_t len, size_t offset,
>>  		bool *same_page)
>>  {
>> +	struct page *page = folio_page(folio, 0);
>>  	unsigned long mask = queue_segment_boundary(q);
>>  	phys_addr_t addr1 = bvec_phys(bv);
>>  	phys_addr_t addr2 = page_to_phys(page) + offset + len - 1;
>[...]
>> +bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv,
>> +		struct page *page, unsigned int len, unsigned int offset,
>> +		bool *same_page)
>> +{
>> +	struct folio *folio = page_folio(page);
>> +
>> +	return bvec_try_merge_hw_folio(q, bv, folio, len,
>> +			((size_t)folio_page_idx(folio, page) << PAGE_SHIFT) +
>> +			offset, same_page);
>> +}
>
>This is the wrong way to do it.  bio_add_folio() does it correctly
>by being a wrapper around bio_add_page().
>
>The reason is that in the future, not all pages will belong to folios.
>For those pages, page_folio() will return NULL, and this will crash.
>

I can change in this fashion. page_folio is getting used at many other
places in kernel and currently there are no NULL checks. Will every place
need a change?
In this series we use page_folio to fetch folio for pages returned by
iov_iter_extract_pages. Then we iterate on the folios instead of pages.
We were progressing to change all the page related functions to accept
struct folio.
If page_folio may return NULL in future, it will require us to maintain
both page and folio versions. Do you see it differently ?


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]