Re: [PATCH 7/7] xfs: use shifting and masking when converting rt extents, if possible

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Oct 16, 2023 at 10:19:47AM -0700, Omar Sandoval wrote:
> On Thu, Oct 12, 2023 at 11:19:08AM -0700, Darrick J. Wong wrote:
> > On Thu, Oct 12, 2023 at 07:25:11AM +0200, Christoph Hellwig wrote:
> > > On Wed, Oct 11, 2023 at 11:06:14AM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Avoid the costs of integer division (32-bit and 64-bit) if the realtime
> > > > extent size is a power of two.
> > > 
> > > Looks good:
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Do you have any data on how common non-power of two rtext sizes are?
> > > Might it be worth to add unlikely annotations?
> > 
> > I don't really know about the historical uses.  There might be old
> > filesystems out there with a non-power-of-2 raid stripe size that are
> > set up for full stripe allocations for speed.
> > 
> > We (oracle) are interested in using rt for PMD allocations on pmem/cxl
> > devices and atomic writes on scsi/nvme devices.  Both of those cases
> > will only ever use powers of 2.
> > 
> > I'll add some if-test annotations and we'll see if anyone notices. ;)
> > 
> > --D
> 
> We are using 1044KB realtime extents (blocksize = 4096, rextsize = 261)
> for our blob storage system. It's a goofy number, but IIRC it was chosen
> because their most common blob sizes were single-digit multiples of a
> megabyte, and they wanted a large-ish (~1MB) realtime extent size to
> reduce external fragmentation, but they also wanted to store a bit of
> extra metadata without requiring an extra realtime extent and blowing up
> internal fragmentation.

LOL, and here I thought I was only pushing weird sizes like 28k to drive
willy crazy. ;)  I wrapped some of the if tests in likely(); they can
get ripped back out.

--D