linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Hannes Reinecke <hare@suse.de>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Pankaj Raghav <p.raghav@samsung.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 7/7] buffer: Fix more functions for block size > PAGE_SIZE
Date: Fri, 10 Nov 2023 14:26:43 +0000	[thread overview]
Message-ID: <ZU49o9oIfSc84pDt@casper.infradead.org> (raw)
In-Reply-To: <20231110045019.GB6572@sol.localdomain>

On Thu, Nov 09, 2023 at 08:50:19PM -0800, Eric Biggers wrote:
> On Thu, Nov 09, 2023 at 09:06:08PM +0000, Matthew Wilcox (Oracle) wrote:
> > Replace the shift with a divide, which is probably cheaper than first
> > calculating the shift.
> 
> No.  The divs are almost certainly more expensive on most CPUs, especially when
> doing two of them.  On x86_64 for example, it will be two div instructions
> instead of one bsr and two shr instructions.  The two divs are much more costly.
> The block size is always a power of 2; we should take advantage of that.

I just looked it up and it's more expensive than I thought, but I don't
see a huge difference here.

DIV has a 23-cycle latency on Skylake (just to pick a relatively recent
CPU).  But it has reciprocal throughput of 6, so two DIVs have latency
of 29, not 46.  Unless the second DIV depends on the result of the first
(it doesn't).

BSF (the ffs instruction) has latency 3.  SHRD has latency 4, but the
SHR is going to depend on the output of the BSF, so they necessarily
add to 7 cycles latency.  SHR has reciprocal latency of 2, so
BSF,SHR,SHR will be 9 cycles.

So I've cost 20 cycles, which is about the cost of missing in the L1
cache and hitting in the L2 cache.  That doesn't feel too bad to me?
Would you want to invest more engineering time in changing it?

  reply	other threads:[~2023-11-10 14:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-09 21:06 [PATCH v2 0/7] More buffer_head cleanups Matthew Wilcox (Oracle)
2023-11-09 21:06 ` [PATCH v2 1/7] buffer: Return bool from grow_dev_folio() Matthew Wilcox (Oracle)
2023-11-10  3:43   ` Ryusuke Konishi
2023-12-30  9:23     ` Matthew Wilcox
2023-12-30 12:22       ` Ryusuke Konishi
2023-11-09 21:06 ` [PATCH v2 2/7] buffer: Calculate block number inside folio_init_buffers() Matthew Wilcox (Oracle)
2023-11-09 21:06 ` [PATCH v2 3/7] buffer: Fix grow_buffers() for block size > PAGE_SIZE Matthew Wilcox (Oracle)
2023-11-10  6:37   ` Ryusuke Konishi
2023-11-10 11:29     ` Ryusuke Konishi
2023-11-10 13:23       ` Matthew Wilcox
2023-11-12  4:52   ` kernel test robot
2023-11-13 17:10     ` Andrew Morton
2023-11-13 17:20       ` Nathan Chancellor
2023-11-15 11:14         ` Naresh Kamboju
2023-11-09 21:06 ` [PATCH v2 4/7] buffer: Cast block to loff_t before shifting it Matthew Wilcox (Oracle)
2023-11-09 21:06 ` [PATCH v2 5/7] buffer: Fix various functions for block size > PAGE_SIZE Matthew Wilcox (Oracle)
2023-11-10  7:48   ` Ryusuke Konishi
2023-11-10 13:36     ` Matthew Wilcox
2023-11-10 15:23       ` Ryusuke Konishi
2023-11-09 21:06 ` [PATCH v2 6/7] buffer: Handle large folios in __block_write_begin_int() Matthew Wilcox (Oracle)
2023-11-09 21:06 ` [PATCH v2 7/7] buffer: Fix more functions for block size > PAGE_SIZE Matthew Wilcox (Oracle)
2023-11-10  4:50   ` Eric Biggers
2023-11-10 14:26     ` Matthew Wilcox [this message]
2023-11-11 18:06       ` Eric Biggers
2023-11-12  4:52         ` Matthew Wilcox
2023-11-12  6:00           ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZU49o9oIfSc84pDt@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=ebiggers@kernel.org \
    --cc=hare@suse.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=p.raghav@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).