public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Kent Overstreet <kent.overstreet@gmail.com>
Cc: Ming Lei <ming.lei@redhat.com>, Mike Snitzer <snitzer@redhat.com>,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	Eric Biggers <ebiggers@google.com>,
	Dmitry Monakhov <dmonakhov@openvz.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH 5.20 1/4] block: add bio_rewind() API
Date: Wed, 29 Jun 2022 11:16:10 -0600	[thread overview]
Message-ID: <6f8db146-d4b3-d17b-4e58-08adc0010cba@kernel.dk> (raw)
In-Reply-To: <20220628183247.bcaqvmnav34kp5zd@moria.home.lan>

On 6/28/22 12:32 PM, Kent Overstreet wrote:
> On Tue, Jun 28, 2022 at 12:13:06PM -0600, Jens Axboe wrote:
>> It's much less about using whatever amount of memory for inflight IO,
>> and much more about not bloating fast path structures (of which the
>> bio is certainly one). All of this gunk has to be initialized for
>> each IO, and that's the real issue.
>>
>> Just look at the recent work for iov_iter and why ITER_UBUF makes
>> sense to do.
>>
>> This is not a commentary on this patchset, just a general
>> observation. Sizes of hot data structures DO matter, and quite a bit
>> so.
> 
> Younger me would have definitely been in agreement; initializing these
> structs definitely tends to show up in profiles.

Older me still greatly cares :-)

> These days I'm somewhat less inclined towards that view - profiles
> naturally highlight where your cache misses are happening, and
> initializing a freshly allocated data structure is always going to be
> a cache miss. But the difference between touching 3 and 6 contiguous
> cachelines is practically nil...  assuming we aren't doing anything
> stupid like using memset (despite what Linus wants from the CPU
> vendors, rep stos _still_ sucks) and perhaps inserting prefetches
> where appropriate.
> 
> And I see work going by that makes me really wonder if it was
> justified - in particular I _really_ want to know if Christoph's bio
> initialization change was justified by actual benchmarks and what
> those numbers were, vs. looking at profiles. Wasn't anything in the
> commit log...

Not sure what Christoph change you are referring to, but all the ones
that I did to improve the init side were all backed by numbers I ran at
that time (and most/all of the commit messages will have that data). So
yes, it is indeed still very noticeable. Maybe not at 100K IOPS, but at
10M on a core it most certainly is.

I'm all for having solid and maintainable code, obviously, but frivolous
bloating of structures and more expensive setup cannot be hand waved
away with "it doesn't matter if we touch 3 or 6 cachelines" because we
obviously have a disagreement on that.

-- 
Jens Axboe


  reply	other threads:[~2022-06-29 17:16 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 14:12 [PATCH 5.20 0/4] block/dm: add bio_rewind for improving dm requeue Ming Lei
2022-06-24 14:12 ` [PATCH 5.20 1/4] block: add bio_rewind() API Ming Lei
2022-06-26 20:14   ` Kent Overstreet
2022-06-27  7:36     ` Ming Lei
2022-06-28  4:20       ` Kent Overstreet
2022-06-28  7:42         ` Ming Lei
2022-06-28 16:16           ` Kent Overstreet
2022-06-28 18:13         ` Jens Axboe
2022-06-28 18:32           ` Kent Overstreet
2022-06-29 17:16             ` Jens Axboe [this message]
2022-06-29 18:40               ` Kent Overstreet
2022-06-29 18:51                 ` Bart Van Assche
2022-06-29 19:05                   ` Kent Overstreet
2022-06-29 19:37                     ` Bart Van Assche
2022-06-29 19:50                       ` Kent Overstreet
2022-06-29 19:59                       ` Kent Overstreet
2022-06-29 19:00                 ` Jens Axboe
2022-06-29 19:26                   ` Kent Overstreet
2022-06-29 20:51                     ` Jens Axboe
2022-06-29  0:49           ` Ming Lei
2022-06-28  4:26       ` Kent Overstreet
2022-06-28  7:49         ` Ming Lei
2022-06-28 16:36           ` Kent Overstreet
2022-06-28 17:41             ` Mike Snitzer
2022-06-28 17:52               ` Kent Overstreet
2022-06-29  6:07                 ` Mike Snitzer
2022-06-29 18:11                   ` Kent Overstreet
2022-06-30  0:47                     ` Ming Lei
2022-06-30  0:58                       ` Kent Overstreet
2022-06-30  1:14                       ` Kent Overstreet
2022-07-01  3:58                         ` Ming Lei
2022-07-01 21:09                           ` Kent Overstreet
2022-06-29  1:02             ` Ming Lei
2022-06-26 21:37   ` [dm-devel] " Eric Biggers
2022-06-27  7:37     ` Ming Lei
2022-06-24 14:12 ` [PATCH 5.20 2/4] dm: add new helper for handling dm_io requeue Ming Lei
2022-06-24 14:12 ` [PATCH 5.20 3/4] dm: improve handling for DM_REQUEUE and AGAIN Ming Lei
2022-06-24 14:12 ` [PATCH 5.20 4/4] dm: add two stage requeue Ming Lei

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=6f8db146-d4b3-d17b-4e58-08adc0010cba@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=dmonakhov@openvz.org \
    --cc=ebiggers@google.com \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=snitzer@redhat.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