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

Stefan Roesch
2022-05-20T18:25:38Z

On 5/19/22 1:18 AM, Christoph Hellwig wrote:
> * 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?

The next version removes iomap_page_create_gfp() and adds the gfp flag to
iomap_page_create.