On 2023/5/15 13:00, Ming Lei wrote:
On Mon, May 15, 2023 at 11:47:31AM +0800, haowenchao (C) wrote:
On 2023/5/15 11:24, Ming Lei wrote:
On Mon, May 15, 2023 at 10:55:01AM +0800, haowenchao (C) wrote:
On 2023/5/13 23:55, Ming Lei wrote:
Hello Guys,
scsi_device->iorequest_cnt/iodone_cnt/ioerr_cnt are only inc/dec,
and exported to userspace via sysfs in kernel, and not see any kernel
consumers, so the exported counters are only for userspace, just be curious
which/how applications consume them?
These counters are used to checking the disk health status in a certain scenario
for us.
All the three counters are increased only, and you could use some simple bpf code
to retrieve such info easily. It adds such overhead for everyone, who may not use
this counters at all, maybe 99% of people don't use it.
These counters not only adds cost in fast path, but also causes kernel panic,
especially the "atomic_inc(&cmd->device->iorequest_cnt)" in
scsi_queue_rq(), because cmd->device may be freed after returning
from scsi_dispatch_cmd(), which is introduced by:
cfee29ffb45b ("scsi: core: Do not increase scsi_device's iorequest_cnt if dispatch failed")
If there aren't explicit applications which depend on these counters,
I'd suggest to kill the three since all are not in stable ABI. Otherwise
I think we need to revert cfee29ffb45b.
Sorry for introduce this bug.
We would check these counters after iodone_cnt equal to iorequest_cnt. So
cfee29ffb45b is aimed to fix the issue of iorequest_cnt is increased for
multiple times if scsi_dispatch_cmd() failed.
Maybe we should revert cfee29ffb45b and fix the original issue with following
changes:
Yeah, it can be fixed in this way, can you cook one such fix?
OK, I would post patches.
Thanks!
Longterm, maybe we can mark these sysfs interface as obsolete and let
existed users switch to ebpf, and finally remove them.
Where can we mark these sysfs interface as obsolete?
It could be one comment on the sysfs interface or extra logging, but not
sure if the latter can work since user expects these sysfs interfaces to
dump counter only.
Thanks,
Ming
I just send the patch but did not comment the sysfs interface, because I am not
sure if others would feedback about these sysfs interface. Would you help to
review the patch?
Thanks.
