dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Mike Snitzer <snitzer@redhat.com>, Christoph Hellwig <hch@infradead.org>
Cc: linux-block@vger.kernel.org, dm-devel@redhat.com,
	Mikulas Patocka <mpatocka@redhat.com>,
	"Alasdair G. Kergon" <agk@redhat.com>
Subject: Re: [PATCH 3/3] block: use a driver-specific handler for the "inflight" value
Date: Fri, 16 Nov 2018 08:25:42 -0700	[thread overview]
Message-ID: <c84a1ed6-cc90-472f-446f-d83a27d3ac20@kernel.dk> (raw)
In-Reply-To: <20181116135555.GA28870@redhat.com>

On 11/16/18 6:55 AM, Mike Snitzer wrote:
> On Fri, Nov 16 2018 at  4:11am -0500,
> Christoph Hellwig <hch@infradead.org> wrote:
> 
>> On Fri, Nov 16, 2018 at 01:04:19AM +0100, Mikulas Patocka wrote:
>>> Device mapper was converted to percpu inflight counters. In order to
>>> display the correct values in the "inflight" sysfs file and in
>>> /proc/diskstats, we need a custom callback that sums the percpu counters.
>>>
>>> The function part_round_stats calculates the number of in-flight I/Os
>>> every jiffy and uses this to calculate the counters time_in_queue and
>>> io_ticks. In order to avoid excessive memory traffic on systems with high
>>> number of CPUs, this functionality is disabled when percpu inflight values
>>> are used and the values time_in_queue and io_ticks are calculated
>>> differently - the result is less precise.
>>
>> And none of that is device mapper specific.  Please submit this code
>> to the block layer for use by all make_request based drivers.  Depending
>> on what Jens as the maintainers thinkgs of the tradeoffs we can discuss
>> if the summing should be on or off by default, or if we maybe even
>> need the current code as a fallback.
> 
> I agree.
> 
> Mikulas, we discussed that the changes would be made to block core.  I
> know that makes you less comfortable (I assume because you need to
> consider more than DM) but it is the right way forward.
> 
> Now that the legacy IO path is gone we have less to consider; these
> counters are only impacting bio-based.

Agree, either the new code is good enough to be used in general, and
then it should be generally used, or it should not exist in the first
place. We've always worked very hard to provide the most efficient
helpers and infrastructure we can in the block layer itself, so that
drivers don't have to reinvent the wheel.

-- 
Jens Axboe

  reply	other threads:[~2018-11-16 15:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-16  0:04 [PATCH 3/3] block: use a driver-specific handler for the "inflight" value Mikulas Patocka
2018-11-16  9:11 ` Christoph Hellwig
2018-11-16 13:55   ` Mike Snitzer
2018-11-16 15:25     ` Jens Axboe [this message]
2018-11-28  0:41       ` Mikulas Patocka

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=c84a1ed6-cc90-472f-446f-d83a27d3ac20@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=mpatocka@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;
as well as URLs for NNTP newsgroup(s).