| Message ID | 20210201051619.19909-1-kashyap.desai@broadcom.com |
|---|---|
| Headers | show |
| Series | io_uring iopoll in scsi layer | expand |
On 2021-02-01 12:16 a.m., Kashyap Desai wrote: > This patch series is to support io_uring iopoll feature > in scsi stack. This patch set requires shared hosttag support. > > This patch set is created on top of 5.12/scsi-staging branch. > https://kernel.googlesource.com/pub/scm/linux/kernel/git/mkp/scsi/+/refs/heads/5.12/scsi-staging Hi, I don't understand how this patchset works. My testing shows scsi_debug is broken and I will be sending a correcting patch shortly (similar to the one I sent you on 20210108). The scsi_debug driver is a simplified LLD that needs to know in advance whether a request/command issued to it will be using the .mq_poll callback. Perhaps you have found another way but one simple way to find that out is this test: if (request->cmd_flags & REQ_HIPRI) In the case of scsi_debug (after my patch) the delay associated with the command is not wired up to generate an event which leads to completion. Instead, callbacks through .mq_poll are expected and they will check if that delay has expired, if not the callback returns 0. When the delay has expired and a .mq_poll is received then completion occurs. Doug Gilbert > v3 -> > - added reviewed-by tag > - Fix comment provided by Hannes for below patch. > https://patchwork.kernel.org/project/linux-scsi/patch/20201203034100.29716-3-kashyap.desai@broadcom.com/ > - Fix Functional issue of poll_queues settings not working in v2. > > v2 -> > - updated feedback from v1. > - added reviewed-by & tested-by tag > - remove flood of prints in scsi_debug driver during iopoll > reported by Douglas Gilbert. > - added new patch to support to get shost from hctx. > added new helper function "scsi_init_hctx" > > v1 -> > Fixed warnings in scsi_debug driver. > Reported-by: kernel test robot <lkp@intel.com> > > Kashyap Desai (4): > add io_uring with IOPOLL support in scsi layer > megaraid_sas: iouring iopoll support > scsi_debug : iouring iopoll support > scsi: set shost as hctx driver_data > > drivers/scsi/megaraid/megaraid_sas.h | 3 + > drivers/scsi/megaraid/megaraid_sas_base.c | 87 +++++++++++-- > drivers/scsi/megaraid/megaraid_sas_fusion.c | 42 ++++++- > drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 + > drivers/scsi/scsi_debug.c | 130 ++++++++++++++++++++ > drivers/scsi/scsi_lib.c | 29 ++++- > include/scsi/scsi_cmnd.h | 1 + > include/scsi/scsi_host.h | 11 ++ > 8 files changed, 291 insertions(+), 14 deletions(-) > > > base-commit: a927ec3995427e9c47752900ad2df0755d02aba5 >
> Hi, > I don't understand how this patchset works. My testing shows scsi_debug is > broken and I will be sending a correcting patch shortly (similar to the > one I > sent you on 20210108). Hi Doug - scsi_debug patch from this series works on my setup. I was under impression that you want this patch to be available in tree and on top of current patchset, you want to have further incremental update. What do you suggest ? Do you want me to wait for your updated patch OR We can ask Martin to pick all the patches except scsi_debug ? You can post scsi_debug changes as another series or separate patch. I have few more megaraid_sas patches in pipeline, so I am looking for this series to be available as baseline. Kashyap > > The scsi_debug driver is a simplified LLD that needs to know in advance > whether a request/command issued to it will be using the .mq_poll > callback. > Perhaps you have found another way but one simple way to find that out is > this test: > if (request->cmd_flags & REQ_HIPRI) > Agree. I am not very much familiar with scsi_debug code so used current code change as starting point and from there things can be improved. > In the case of scsi_debug (after my patch) the delay associated with the > command is not wired up to generate an event which leads to completion. > Instead, callbacks through .mq_poll are expected and they will check if > that > delay has expired, if not the callback returns 0. When the delay has > expired > and a .mq_poll is received then completion occurs. > > Doug Gilbert > > > v3 -> > > - added reviewed-by tag > > - Fix comment provided by Hannes for below patch. > > https://patchwork.kernel.org/project/linux-scsi/patch/20201203034100.2 > > 9716-3-kashyap.desai@broadcom.com/ > > - Fix Functional issue of poll_queues settings not working in v2. > > > > v2 -> > > - updated feedback from v1. > > - added reviewed-by & tested-by tag > > - remove flood of prints in scsi_debug driver during iopoll > > reported by Douglas Gilbert. > > - added new patch to support to get shost from hctx. > > added new helper function "scsi_init_hctx" > > > > v1 -> > > Fixed warnings in scsi_debug driver. > > Reported-by: kernel test robot <lkp@intel.com> > > > > Kashyap Desai (4): > > add io_uring with IOPOLL support in scsi layer > > megaraid_sas: iouring iopoll support > > scsi_debug : iouring iopoll support > > scsi: set shost as hctx driver_data > > > > drivers/scsi/megaraid/megaraid_sas.h | 3 + > > drivers/scsi/megaraid/megaraid_sas_base.c | 87 +++++++++++-- > > drivers/scsi/megaraid/megaraid_sas_fusion.c | 42 ++++++- > > drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 + > > drivers/scsi/scsi_debug.c | 130 ++++++++++++++++++++ > > drivers/scsi/scsi_lib.c | 29 ++++- > > include/scsi/scsi_cmnd.h | 1 + > > include/scsi/scsi_host.h | 11 ++ > > 8 files changed, 291 insertions(+), 14 deletions(-) > > > > > > base-commit: a927ec3995427e9c47752900ad2df0755d02aba5 > >
On 2021-02-02 5:32 a.m., Kashyap Desai wrote: >> Hi, >> I don't understand how this patchset works. My testing shows scsi_debug is >> broken and I will be sending a correcting patch shortly (similar to the >> one I >> sent you on 20210108). > > Hi Doug - > > scsi_debug patch from this series works on my setup. I was under impression > that you want this patch to be available in tree and on top of current > patchset, you want to have further incremental update. > What do you suggest ? Do you want me to wait for your updated patch OR We > can ask Martin to pick all the patches except scsi_debug ? You can post > scsi_debug changes as another series or separate patch. Kashyap, I did post a fixing patch titled: "[PATCH] scsi_debug: add new defer_type for mq_poll" and cc-ed it to you. At first I also thought your scsi_debug patch worked but on closer examination it was falling through to the existing delay_expired-event_generated model. It was only when I checked for requests with REQ_HIPRI set and in those cases turned off the event generation, that I saw it was not working. Another way to tell if iopoll/blk_poll is really working is to turn off the code that drives blk_poll(). What should happen in that case (due to the mid layer) is that the command timeout (after 60 seconds?) should cancel the command. If the command still manages to succeed normally then iopoll is not really working. > I have few more megaraid_sas patches in pipeline, so I am looking for this > series to be available as baseline. And on my side the most recent "[PATCH v15 00/45] sg: add v4 interface" patchset [20210225] included "[PATCH v15 44/45] sg: add blk_poll support". Jens accepted a patch to the fio sg engine to use the "hipri=1" option to drive that new capability. This is an addition to the fio script example that you sent using io_uring: [global] ioengine=io_uring hipri=1 direct=1 runtime=3s rw=randread norandommap bs=512 [seqprecon] filename=/dev/sdb Doug Gilbert >> The scsi_debug driver is a simplified LLD that needs to know in advance >> whether a request/command issued to it will be using the .mq_poll >> callback. >> Perhaps you have found another way but one simple way to find that out is >> this test: >> if (request->cmd_flags & REQ_HIPRI) >> > > Agree. I am not very much familiar with scsi_debug code so used current code > change as starting point and from there things can be improved. > >> In the case of scsi_debug (after my patch) the delay associated with the >> command is not wired up to generate an event which leads to completion. >> Instead, callbacks through .mq_poll are expected and they will check if >> that >> delay has expired, if not the callback returns 0. When the delay has >> expired >> and a .mq_poll is received then completion occurs. >> >> Doug Gilbert >> >>> v3 -> >>> - added reviewed-by tag >>> - Fix comment provided by Hannes for below patch. >>> https://patchwork.kernel.org/project/linux-scsi/patch/20201203034100.2 >>> 9716-3-kashyap.desai@broadcom.com/ >>> - Fix Functional issue of poll_queues settings not working in v2. >>> >>> v2 -> >>> - updated feedback from v1. >>> - added reviewed-by & tested-by tag >>> - remove flood of prints in scsi_debug driver during iopoll >>> reported by Douglas Gilbert. >>> - added new patch to support to get shost from hctx. >>> added new helper function "scsi_init_hctx" >>> >>> v1 -> >>> Fixed warnings in scsi_debug driver. >>> Reported-by: kernel test robot <lkp@intel.com> >>> >>> Kashyap Desai (4): >>> add io_uring with IOPOLL support in scsi layer >>> megaraid_sas: iouring iopoll support >>> scsi_debug : iouring iopoll support >>> scsi: set shost as hctx driver_data >>> >>> drivers/scsi/megaraid/megaraid_sas.h | 3 + >>> drivers/scsi/megaraid/megaraid_sas_base.c | 87 +++++++++++-- >>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 42 ++++++- >>> drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 + >>> drivers/scsi/scsi_debug.c | 130 ++++++++++++++++++++ >>> drivers/scsi/scsi_lib.c | 29 ++++- >>> include/scsi/scsi_cmnd.h | 1 + >>> include/scsi/scsi_host.h | 11 ++ >>> 8 files changed, 291 insertions(+), 14 deletions(-) >>> >>> >>> base-commit: a927ec3995427e9c47752900ad2df0755d02aba5 >>>