Re: [PATCH V2] scsi: put hot fields of scsi_host_template into one cacheline

John Garry
2020-04-22T10:07:19Z

On 22/04/2020 10:54, Ming Lei wrote:
> The following three fields of scsi_host_template are referenced in
> scsi IO submission path, so put them together into one cacheline:
>
> - cmd_size
> - queuecommand
> - commit_rqs
>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: John Garry <john.garry@huawei.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

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

> ---
> V2:
> - move the 3 fields at the beginning of scsi_host_template
> - comment why we do this way
>
> include/scsi/scsi_host.h | 72 ++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 33 deletions(-)
>
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 822e8cda8d9b..eae5b9f52dfe 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -30,40 +30,15 @@ struct scsi_transport_template;
> #define MODE_TARGET 0x02
>
> struct scsi_host_template {
> - struct module *module;
> - const char *name;
> -
> /*
> - * The info function will return whatever useful information the
> - * developer sees fit. If not provided, then the name field will
> - * be used instead.
> - *
> - * Status: OPTIONAL
> + * Put fields referenced in IO submission path together in
> + * same cacheline
> */
> - const char *(* info)(struct Scsi_Host *);
>
> /*
> - * Ioctl interface
> - *
> - * Status: OPTIONAL
> - */
> - int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> - void __user *arg);
> -
> -
> -#ifdef CONFIG_COMPAT
> - /*
> - * Compat handler. Handle 32bit ABI.
> - * When unknown ioctl is passed return -ENOIOCTLCMD.
> - *
> - * Status: OPTIONAL
> + * Additional per-command data allocated for the driver.
> */
> - int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> - void __user *arg);
> -#endif
> -
> - int (*init_cmd_priv)(struct Scsi_Host *shost, struct scsi_cmnd *cmd);
> - int (*exit_cmd_priv)(struct Scsi_Host *shost, struct scsi_cmnd *cmd);
> + unsigned int cmd_size;
>
> /*
> * The queuecommand function is used to queue up a scsi
> @@ -111,6 +86,41 @@ struct scsi_host_template {
> */
> void (*commit_rqs)(struct Scsi_Host *, u16);
>
> + struct module *module;
> + const char *name;
> +
> + /*
> + * The info function will return whatever useful information the
> + * developer sees fit. If not provided, then the name field will
> + * be used instead.
> + *
> + * Status: OPTIONAL
> + */
> + const char *(* info)(struct Scsi_Host *);
> +
> + /*
> + * Ioctl interface
> + *
> + * Status: OPTIONAL
> + */
> + int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> +
> +
> +#ifdef CONFIG_COMPAT
> + /*
> + * Compat handler. Handle 32bit ABI.
> + * When unknown ioctl is passed return -ENOIOCTLCMD.
> + *
> + * Status: OPTIONAL
> + */
> + int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> +#endif
> +
> + int (*init_cmd_priv)(struct Scsi_Host *shost, struct scsi_cmnd *cmd);
> + int (*exit_cmd_priv)(struct Scsi_Host *shost, struct scsi_cmnd *cmd);
> +
> /*
> * This is an error handling strategy routine. You don't need to
> * define one of these if you don't want to - there is a default
> @@ -468,10 +478,6 @@ struct scsi_host_template {
> */
> u64 vendor_id;
>
> - /*
> - * Additional per-command data allocated for the driver.
> - */
> - unsigned int cmd_size;
> struct scsi_host_cmd_pool *cmd_pool;
>
> /* Delay for runtime autosuspend */
>