Re: SCSI: Consumer of scsi_devices->iorequest_cnt/iodone_cnt/ioerr_cnt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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:

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 03964b26f3f2..0226c9279cef 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1485,6 +1485,7 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
                 */
                SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
                        "queuecommand : device blocked\n"));
+               atomic_dec(&cmd->device->iorequest_cnt);
                return SCSI_MLQUEUE_DEVICE_BUSY;
        }
@@ -1517,6 +1518,7 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
        trace_scsi_dispatch_cmd_start(cmd);
        rtn = host->hostt->queuecommand(host, cmd);
        if (rtn) {
+               atomic_dec(&cmd->device->iorequest_cnt);
                trace_scsi_dispatch_cmd_error(cmd, rtn);
                if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
                    rtn != SCSI_MLQUEUE_TARGET_BUSY)


[1] [linux]$ git grep -n -E "iorequest_cnt|iodone_cnt|ioerr_cnt" ./
Documentation/scsi/st.rst:222:value as iodone_cnt at the device level. The tape statistics only count
drivers/scsi/scsi_error.c:362:  atomic_inc(&scmd->device->iodone_cnt);
drivers/scsi/scsi_lib.c:1428:   atomic_inc(&cmd->device->iodone_cnt);
drivers/scsi/scsi_lib.c:1430:           atomic_inc(&cmd->device->ioerr_cnt);
drivers/scsi/scsi_lib.c:1764:   atomic_inc(&cmd->device->iorequest_cnt);
drivers/scsi/scsi_sysfs.c:968:show_sdev_iostat(iorequest_cnt);
drivers/scsi/scsi_sysfs.c:969:show_sdev_iostat(iodone_cnt);
drivers/scsi/scsi_sysfs.c:970:show_sdev_iostat(ioerr_cnt);
drivers/scsi/scsi_sysfs.c:1288: &dev_attr_iorequest_cnt.attr,
drivers/scsi/scsi_sysfs.c:1289: &dev_attr_iodone_cnt.attr,
drivers/scsi/scsi_sysfs.c:1290: &dev_attr_ioerr_cnt.attr,
drivers/scsi/sd.c:3531: atomic_set(&sdkp->device->ioerr_cnt, 0);
include/scsi/scsi_device.h:234: atomic_t iorequest_cnt;
include/scsi/scsi_device.h:235: atomic_t iodone_cnt;
include/scsi/scsi_device.h:236: atomic_t ioerr_cnt;




Thanks,
Ming





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux