mbox series

[v2,00/50] Remove the request pointer from struct scsi_cmnd

Message ID 20210518174450.20664-1-bvanassche@acm.org
Headers show
Series Remove the request pointer from struct scsi_cmnd | expand

Message

Bart Van Assche May 18, 2021, 5:44 p.m. UTC 
Hi Martin,

This patch series implements the following two changes for all SCSI drivers:
- Use blk_mq_rq_from_pdu() instead of the request member of struct scsi_cmnd
  since adding an offset to a pointer is faster than pointer indirection.
- Remove the request pointer from struct scsi_cmnd.

Please consider this patch series for kernel v5.14.

Thanks,

Bart.

Changes compared to v1:
- Renamed blk_req() into scsi_cmd_to_rq().
- Added a few Acked-by tags.

Bart Van Assche (50):
  core: Introduce the scsi_cmd_to_rq() function
  core: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  sd: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  sr: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  scsi_transport_fc: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  scsi_transport_spi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ata: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  RDMA/iser: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  RDMA/srp: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  zfcp: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  53c700: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  NCR5380: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  aacraid: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  advansys: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  bnx2i: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  csiostor: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  cxlflash: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  dpt_i2o: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  fnic: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  hisi_sas: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  hpsa: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ibmvfc: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ibmvscsi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ips: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  libsas: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  lpfc: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  megaraid: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  mpt3sas: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  mvumi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  myrb: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  myrs: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ncr53c8xx: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qedf: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qedi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qla1280: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qla2xxx: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qla4xxx: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  qlogicpti: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  scsi_debug: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  smartpqi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  snic: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  stex: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  sun3_scsi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  sym53c8xx: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  ufs: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  virtio_scsi: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  xen-scsifront: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  tcm_loop: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  usb-storage: Use scsi_cmd_to_rq() instead of scsi_cmnd.request
  core: Remove the request member from struct scsi_cmnd

 drivers/ata/libata-eh.c                     |  5 +-
 drivers/ata/libata-scsi.c                   | 10 ++--
 drivers/ata/pata_falcon.c                   |  4 +-
 drivers/infiniband/ulp/iser/iser_memory.c   |  2 +-
 drivers/infiniband/ulp/srp/ib_srp.c         |  6 +-
 drivers/s390/scsi/zfcp_fsf.c                |  2 +-
 drivers/scsi/53c700.c                       |  2 +-
 drivers/scsi/NCR5380.c                      |  6 +-
 drivers/scsi/aacraid/aachba.c               |  2 +-
 drivers/scsi/aacraid/commsup.c              |  2 +-
 drivers/scsi/advansys.c                     |  4 +-
 drivers/scsi/bnx2i/bnx2i_hwi.c              |  2 +-
 drivers/scsi/csiostor/csio_scsi.c           |  6 +-
 drivers/scsi/cxlflash/main.c                |  2 +-
 drivers/scsi/dpt_i2o.c                      |  4 +-
 drivers/scsi/fnic/fnic_scsi.c               | 49 ++++++++--------
 drivers/scsi/hisi_sas/hisi_sas_main.c       |  4 +-
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c      |  2 +-
 drivers/scsi/hpsa.c                         |  6 +-
 drivers/scsi/ibmvscsi/ibmvfc.c              |  2 +-
 drivers/scsi/ibmvscsi/ibmvscsi.c            |  2 +-
 drivers/scsi/ips.c                          |  2 +-
 drivers/scsi/libsas/sas_ata.c               |  2 +-
 drivers/scsi/libsas/sas_scsi_host.c         |  2 +-
 drivers/scsi/lpfc/lpfc_scsi.c               | 63 ++++++++++-----------
 drivers/scsi/megaraid/megaraid_sas_base.c   |  4 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 10 ++--
 drivers/scsi/mpt3sas/mpt3sas_base.c         |  4 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c        |  6 +-
 drivers/scsi/mvumi.c                        |  2 +-
 drivers/scsi/myrb.c                         | 11 ++--
 drivers/scsi/myrs.c                         | 11 ++--
 drivers/scsi/ncr53c8xx.c                    |  4 +-
 drivers/scsi/qedf/qedf_io.c                 |  8 +--
 drivers/scsi/qedi/qedi_fw.c                 |  9 +--
 drivers/scsi/qla1280.c                      |  6 +-
 drivers/scsi/qla2xxx/qla_os.c               |  4 +-
 drivers/scsi/qla4xxx/ql4_iocb.c             |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c               |  4 +-
 drivers/scsi/qlogicpti.c                    |  2 +-
 drivers/scsi/scsi.c                         |  2 +-
 drivers/scsi/scsi_debug.c                   | 13 +++--
 drivers/scsi/scsi_error.c                   | 15 +++--
 drivers/scsi/scsi_lib.c                     | 29 +++++-----
 drivers/scsi/scsi_logging.c                 | 18 +++---
 drivers/scsi/scsi_transport_fc.c            |  2 +-
 drivers/scsi/scsi_transport_spi.c           |  2 +-
 drivers/scsi/sd.c                           | 33 +++++------
 drivers/scsi/sd_zbc.c                       | 10 ++--
 drivers/scsi/smartpqi/smartpqi_init.c       |  4 +-
 drivers/scsi/snic/snic_scsi.c               | 10 ++--
 drivers/scsi/sr.c                           | 13 ++---
 drivers/scsi/stex.c                         |  6 +-
 drivers/scsi/sun3_scsi.c                    |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c         |  4 +-
 drivers/scsi/ufs/ufshcd.c                   | 28 ++++-----
 drivers/scsi/virtio_scsi.c                  |  4 +-
 drivers/scsi/xen-scsifront.c                |  2 +-
 drivers/target/loopback/tcm_loop.c          |  4 +-
 drivers/usb/storage/transport.c             |  2 +-
 include/scsi/scsi_cmnd.h                    | 15 +++--
 include/scsi/scsi_device.h                  | 16 +++---
 62 files changed, 255 insertions(+), 259 deletions(-)

Comments

Bart Van Assche May 18, 2021, 9:21 p.m. UTC | #1 
On 5/18/21 10:55 AM, James Bottomley wrote:
> On Tue, 2021-05-18 at 10:44 -0700, Bart Van Assche wrote:
>> Hi Martin,
>>
>> This patch series implements the following two changes for all SCSI
>> drivers:
>> - Use blk_mq_rq_from_pdu() instead of the request member of struct
>> scsi_cmnd
>>   since adding an offset to a pointer is faster than pointer
>> indirection.
> 
> Are there any performance results to back up this assertion?  It's
> quite a lot of churn so it would be nice to know it's worth it.

I have not yet run any performance measurements because I expect that it
will be challenging to measure the performance impact of a change like
this one accurately. The performance measurement tool itself (e.g. fio)
might introduce more variation between runs than the performance
improvement of this patch series. Another reason I have not yet run any
performance measurements is because I was assuming that everyone would
be happy with a patch series that makes code faster and that reduces the
size of a key SCSI data structure.

Anyway, I have run 'make drivers/scsi/scsi_lib.lst' with and without
this patch series applied. What I see is that without this patch series
the assembly code for converting a SCSI command pointer into a request
pointer looks like this:

48 8b bb 10 01 00 00    mov    0x110(%rbx),%rdi

With this patch series applied that conversion code changes into the
following:

48 8d bb f0 fe ff ff    lea    -0x110(%rbx),%rdi

The above shows that struct request has a size of 0x110 = 272 bytes with
my kernel configuration.

This illustrates that this patch series realizes an improvement since
"mov" instructions used for converting SCSI command pointers into struct
request pointers are converted into "lea" instructions. "mov" fetches
data from memory while "lea" does not.

Bart.
Juergen Gross May 19, 2021, 9:20 a.m. UTC | #2 
On 18.05.21 19:44, Bart Van Assche wrote:
> Prepare for removal of the request pointer by using scsi_cmd_to_rq()

> instead. This patch does not change any functionality.

> 

> Signed-off-by: Bart Van Assche <bvanassche@acm.org>


Acked-by: Juergen Gross <jgross@suse.com>



Juergen
John Garry May 19, 2021, 10:12 a.m. UTC | #3 
On 18/05/2021 18:44, Bart Van Assche wrote:
> Prepare for removal of the request pointer by using scsi_cmd_to_rq()

> instead. This patch does not change any functionality.

> 

> Signed-off-by: Bart Van Assche<bvanassche@acm.org>


Tested-by: John Garry <john.garry@huawei.com>

Reviewed-by: John Garry <john.garry@huawei.com>