On Thu, Jan 18, 2024 at 05:31:00PM -0800, Darrick J. Wong wrote: > On Fri, Jan 19, 2024 at 09:19:41AM +1100, Dave Chinner wrote: > > +/* > > + * Allocating a high order folio makes the assumption that buffers are a > > + * power-of-2 size so that ilog2() returns the exact order needed to fit > > + * the contents of the buffer. Buffer lengths are mostly a power of two, > > + * so this is not an unreasonable approach to take by default. > > + * > > + * The exception here are user xattr data buffers, which can be arbitrarily > > + * sized up to 64kB plus structure metadata. In that case, round up the order. > > + */ > > +static bool > > +xfs_buf_alloc_folio( > > + struct xfs_buf *bp, > > + gfp_t gfp_mask) > > +{ > > + int length = BBTOB(bp->b_length); > > + int order; > > + > > + order = ilog2(length); > > + if ((1 << order) < length) > > + order = ilog2(length - 1) + 1; > > + > > + if (order <= PAGE_SHIFT) > > + order = 0; > > + else > > + order -= PAGE_SHIFT; > > + > > + bp->b_folio_array[0] = folio_alloc(gfp_mask, order); > > + if (!bp->b_folio_array[0]) > > + return false; > > + > > + bp->b_folios = bp->b_folio_array; > > + bp->b_folio_count = 1; > > + bp->b_flags |= _XBF_FOLIOS; > > + return true; > > Hmm. So I guess with this patchset, either we get one multi-page large > folio for the whole buffer, or we fall back to the array of basepage > sized folios? Yes. > /me wonders if the extra flexibility from alloc_folio_bulk_array and a > folioized vm_map_ram would eliminate all this special casing? IMO, no. The basic requirement for a buffer is to provide contiguous memory space unless XBF_UNMAPPED is specified. In the case of contiguous space, we either get a single contiguous range allocated or we have to vmap multiple segments. The moment we can't get a contiguous memory range, we've lost, we're in the slow path and we should just do what we know will reliably work as efficiently as possible. In the case of XBF_UNMAPPED, if we get a single contiguous range we can ignore XBF_UNMAPPED, otherwise we should just do what we know will reliably work as efficiently as possible. Hence I don't think trying to optimise the "we didn't get a contiguous memory allocation for the buffer" case for smaller multi-page folios because it just adds complexity and doesn't provide any real advantage over the PAGE_SIZE allocation path we do now. > Uhoh, lights flickering again and ice crashing off the roof. I better > go before the lights go out again. :( Fun fun! -Dave. -- Dave Chinner david@xxxxxxxxxxxxx