On Wed, May 19, 2021 at 09:08:51PM +0200, Christoph Hellwig wrote:
> Split xfs_buf_allocate_memory into one helper that allocates from
> slab and one that allocates using the page allocator.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
....
> +static int
> +xfs_buf_alloc_slab(
> + struct xfs_buf *bp,
> + unsigned int flags)
> +{
xfs_buf_alloc_kmem() or xfs_buf_alloc_heap() would be better, I
think, because it matches the flag used to indicate how the memory
associated with the buffer was allocated.
> @@ -720,9 +717,17 @@ xfs_buf_get_map(
> if (error)
> return error;
>
> - error = xfs_buf_allocate_memory(new_bp, flags);
> - if (error)
> - goto out_free_buf;
> + /*
> + * For buffers that are contained within a single page, just allocate
> + * the memory from the heap - there's no need for the complexity of
> + * page arrays to keep allocation down to order 0.
> + */
> + if (BBTOB(new_bp->b_length) >= PAGE_SIZE ||
> + xfs_buf_alloc_slab(new_bp, flags) < 0) {
> + error = xfs_buf_alloc_pages(new_bp, flags);
> + if (error)
> + goto out_free_buf;
> + }
Took me a moment to grok the logic pattern here, then I realised the
comment didn't help as it makes no indication that the heap
allocation is best effort and will fall back to pages. A small tweak
like:
/*
* For buffers that fit entirely within a single page, first
* attempt to allocate the memory from the heap to minimise
* memory usage. If we can't get heap memory for these small
* buffers, we fall back to using the page allocator.
*/
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
