All of lore.kernel.org
 help / color / mirror / Atom feed
From: Enzo Matsumiya <ematsumiya@suse.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@lst.de>,
	linux-nvme@lists.infradead.org, Keith Busch <kbusch@kernel.org>,
	Jens Axboe <axboe@fb.com>, Sagi Grimberg <sagi@grimberg.me>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nvme-pci: trigger disk activity LED
Date: Tue, 1 Mar 2022 00:30:01 -0300	[thread overview]
Message-ID: <20220301033001.tozk6cakdznww6wi@cyberdelia> (raw)
In-Reply-To: <36cfd242-6bb0-0af6-0faf-946c79baa378@kernel.dk>

On 02/28, Jens Axboe wrote:
>On 2/28/22 2:22 AM, Christoph Hellwig wrote:
>> I don't think we should add code to the absolutel fast path for
>> blinkenlights.
>
>Agree. It'd be a lot better to put the cost on the led trigger
>side, and not need anything in the fast path for block devices.
>Monitor disk stats, or something like that.

There's been at least 4 attempts to do so, as far as I'm aware (one of
them being mine). All got rejected due to the complexity it introduced,
that's how I ended up with this one-liner.

Performance-wise, I'm understand the problems, but according to ftrace,
ledtrig_disk_activity() adds an average of 0.2us overhead, whether an
LED is assigned or not. Is that really unacceptable?

If so, would introducing a CONFIG_NVME_LED (default =n) and wrap that
call around it make it better? Then at least there's a chance to inform
users that desires this feature about performance costs.


Cheers,

Enzo

  reply	other threads:[~2022-03-01  3:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-27 23:42 [PATCH] nvme-pci: trigger disk activity LED Enzo Matsumiya
2022-02-28  9:22 ` Christoph Hellwig
2022-02-28 13:58   ` Jens Axboe
2022-03-01  3:30     ` Enzo Matsumiya [this message]
2022-03-01  3:36       ` Jens Axboe

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=20220301033001.tozk6cakdznww6wi@cyberdelia \
    --to=ematsumiya@suse.de \
    --cc=axboe@fb.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.