Re: [RFC PATCH v3 02/18] iomap: Add iomap_page_create_gfp to allocate iomap_pages

Christoph Hellwig
2022-05-19T08:19:12Z

* This function returns a newly allocated iomap for the folio with the settings
> + * specified in the gfp parameter.
> + *
> + **/
> static struct iomap_page *
> -iomap_page_create(struct inode *inode, struct folio *folio)
> +iomap_page_create_gfp(struct inode *inode, struct folio *folio,
> + unsigned int nr_blocks, gfp_t gfp)
> {
> - struct iomap_page *iop = to_iomap_page(folio);
> - unsigned int nr_blocks = i_blocks_per_folio(inode, folio);
> + struct iomap_page *iop;
>
> - if (iop || nr_blocks <= 1)
> + iop = kzalloc(struct_size(iop, uptodate, BITS_TO_LONGS(nr_blocks)), gfp);
> + if (!iop)
> return iop;
>
> - iop = kzalloc(struct_size(iop, uptodate, BITS_TO_LONGS(nr_blocks)),
> - GFP_NOFS | __GFP_NOFAIL);
> spin_lock_init(&iop->uptodate_lock);
> if (folio_test_uptodate(folio))
> bitmap_fill(iop->uptodate, nr_blocks);
> @@ -61,6 +71,18 @@ iomap_page_create(struct inode *inode, struct folio *folio)
> return iop;
> }
>
> +static struct iomap_page *
> +iomap_page_create(struct inode *inode, struct folio *folio)
> +{
> + struct iomap_page *iop = to_iomap_page(folio);
> + unsigned int nr_blocks = i_blocks_per_folio(inode, folio);
> +
> + if (iop || nr_blocks <= 1)
> + return iop;
> +
> + return iomap_page_create_gfp(inode, folio, nr_blocks, GFP_NOFS | __GFP_NOFAIL);

Overly long line here.

Mor importantly why do you need a helper that does not do the number
of blocks check? Why can't we just pass a gfp_t to iomap_page_create?