[PATCH] NVMe: Reduce divide operations

Sam Bradshaw (sbradshaw) sbradshaw at micron.com
Fri Nov 21 09:38:35 PST 2014


> I don't think there's a compiler in the world that doesn't optimise 'x
> * 8' into 'x << 3' if the latter is cheaper.
> 
> Also, I think your nprps is now slightly larger than it used to be.
> Shouldn't it look like this?
> 
> 	unsigned nprps = size >> dev->page_shift;
> 	if (size & (page_size - 1))
> 		nprps++;
> 
> Let's imagine we're sending a misaligned 16k I/O.  We need 5 PRP
> entries, but the first one goes in the command, so we need to allocate
> enough space for 4 entries.  16k / 4k is 4, but your code says to add 1.
> 
> We can actually do slightly better than the original code in the case
> where we're sending a 2MB I/O.  The code in the driver today thinks we
> need a second page, but the last entry on the PRP page will be a PRP
> Entry, not a PRP List Entry.  So I think this is the right calculation:
> 
>  static int nvme_npages(unsigned size, struct nvme_dev *dev)  {
> -	unsigned nprps = DIV_ROUND_UP(size + dev->page_size, dev-
> >page_size);
> -	return DIV_ROUND_UP(8 * nprps, dev->page_size - 8);
> +	unsigned page_size = (1 << dev->page_shift);
> +	unsigned nprps = size >> dev->page_shift;
> +
> +	if (size & (page_size - 1))
> +		nprps++;
> +	if ((nprps * 8) <= page_size)
> +		return 1;
> +	return DIV_ROUND_UP(nprps * 8, page_size - 8);
>  }
> 
> (it still overestimates for I/Os on 2MB boundaries that are larger than
> 2MB, but I'm OK with that.  If somebody wants to come up with a neater
> calculation, feel free).

Seems reasonable to me.

> We could further optimise it by knowing that we need 0 pages if the I/O
> is <= 8k in size:
> 
> +	unsigned nprps,	page_size = (1 << dev->page_shift);
> +
> +	if (size <= page_size * 2)
> +		return 0;
> +	nprps = size >> dev->page_shift;
> +	if (size & (page_size - 1))
> ...
> 
> but I'm not sure that it's worth saving those 8 bytes.

Agreed.  I had originally thought of eliminating nvme_npages()
altogether in favor of a worst case padding for the iod but decided
to stick with the overall theme of saving bytes here and there.




More information about the Linux-nvme mailing list