Re: [PATCH 6/6] ceph: use folios in put_bvecs()

[Tal Zussman]

On 8/26/26 12:48 AM, Tal Zussman wrote:
> Use bvec_folio() to dirty and release the pages as folios. This adds
> one call to compound_head() per page, while folio_mark_dirty_lock() and
> folio_put() each drop one, for a net saving of one call per page when
> dirtying.
> 
> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> ---
>  fs/ceph/file.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 0b6921293f27..89233c5905c9 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -182,11 +182,15 @@ static void put_bvecs(struct bio_vec *bvecs, int num_bvecs, bool should_dirty)
>  	int i;
>  
>  	for (i = 0; i < num_bvecs; i++) {
> -		if (bvecs[i].bv_page) {
> -			if (should_dirty)
> -				set_page_dirty_lock(bvecs[i].bv_page);
> -			put_page(bvecs[i].bv_page);
> -		}
> +		struct folio *folio;
> +
> +		if (!bvecs[i].bv_page)
> +			continue;
> +
> +		folio = bvec_folio(&bvecs[i]);
> +		if (should_dirty)
> +			folio_mark_dirty_lock(folio);
> +		folio_put(folio);

I was overzealous here... Sashiko found an issue:

"Does this code risk a reference count underflow for slab pages?

When the VFS or block layer passes an ITER_BVEC pointing to slab-allocated
memory (such as a kmalloc buffer) to ceph_direct_read_write(), the ceph
code calls iov_iter_get_pages2() via __iter_get_bvecs(). This explicitly
skips taking a reference on slab pages:

if (!folio_test_slab(folio))
        folio_get(folio);

When the IO completes, put_bvecs() cleans up by calling folio_put(folio)
on each page. Since folio_put() lacks the folio_test_slab() and
folio_test_large_kmalloc() protections that put_page() historically
provided, will this unconditionally drop a reference that was never
acquired?"

I think the proper fix here is to switch __iter_get_bvecs() to use
iov_iter_extract_pages() and clean up this path, but that's a separate
change. I'll drop this patch from this series (it wasn't really related
anyways).

>  	}
>  	kvfree(bvecs);
>  }
>