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

[Christoph Hellwig <hch@lst.de>]

> - Changed functions bio_iov_add_page() and bio_iov_add_zone_append_page() to
>   accept a folio

Those should be separate prep patches.

> - Added change in NVMe driver to use nvme_setup_prp_simple() by ignoring
>   multiples of NVME_CTRL_PAGE_SIZE in offset

This should also be a prep patch.

> - Added change to unpin_user_pages which were added as folios. Also stopped
>   the unpin of pages one by one from __bio_release_pages()(Suggested by
>   Keith)

and this as well.

> @@ -1289,16 +1285,30 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  
>  	for (left = size, i = 0; left > 0; left -= len, i++) {
>  		struct page *page = pages[i];
> +		folio = page_folio(page);

Please keep an empty line after declarations.  But I think you can also
just move the folio declaration here and combine the lines, i.e.

		struct page *page = pages[i];
		struct folio *folio = page_folio(page);

		...

> +		/* See the offset in folio and the size */
> +		folio_offset = (folio_page_idx(folio, page)
> +				<< PAGE_SHIFT) + offset;

Kernel coding style keeps the operators on the previous line, i.e.

		folio_offset = (folio_page_idx(folio, page) << PAGE_SHIFT) +
				offset;

> +		size_folio = folio_size(folio);
> +
> +		/* Calculate the length of folio to be added */
> +		len = min_t(size_t, (size_folio - folio_offset), left);

size_folio is only used in this expression, so we can simplify the code
by just removing the variable:

		/* Calculate how much of the folio we're going to add: */
		len = min_t(size_t, folio_size(folio) - folio_offset, left);

> +		/* Skip the pages which got added */
> +		if (bio_flagged(bio, BIO_PAGE_PINNED) && num_pages > 1)
> +			unpin_user_pages(pages + i, num_pages - 1);

The comment doesn't sound quite correct to me: we're not really skipping
the pages, but we are dropping the extra references early here.

>  		if (!is_pci_p2pdma_page(bv.bv_page)) {
> -			if (bv.bv_offset + bv.bv_len <= NVME_CTRL_PAGE_SIZE * 2)
> +			if ((bv.bv_offset & (NVME_CTRL_PAGE_SIZE - 1))
> +				+ bv.bv_len <= NVME_CTRL_PAGE_SIZE * 2)

Sme comment about overator placement as above.