Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Mar 2022 00:30:01 -0300
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
Message-ID: <20220301033001.tozk6cakdznww6wi@cyberdelia>
References: <20220227234258.24619-1-ematsumiya@suse.de>
 <20220228092215.GA8549@lst.de>
 <36cfd242-6bb0-0af6-0faf-946c79baa378@kernel.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <36cfd242-6bb0-0af6-0faf-946c79baa378@kernel.dk>
Precedence: bulk

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