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

[Ming Lei <ming.lei@redhat.com>]

On Tue, Apr 21, 2020 at 04:16:52PM +0100, John Garry wrote:
> On 21/04/2020 13:49, 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>
> > ---
> >   include/scsi/scsi_host.h | 67 +++++++++++++++++++++-------------------
> >   1 file changed, 35 insertions(+), 32 deletions(-)
> > 
> > diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> > index 822e8cda8d9b..959dc5160f72 100644
> > --- a/include/scsi/scsi_host.h
> > +++ b/include/scsi/scsi_host.h
> > @@ -33,37 +33,12 @@ 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
> > -	 */
> > -	const char *(* info)(struct Scsi_Host *);
> > +	/* Put hot fields together in same cacheline */
> >   	/*
> > -	 * 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);
> 
> Should new member .init_cmd_priv be included also in the "hot" group? Even
> if NULL generally, we still have to load that from memory to know it (is
> NULL) in scsi_mq_init_request() and scsi_init_command() [which are fastpath,
> right?]

scsi_mq_init_request() is called via ->init_request(), which is oneshot
initialization.

scsi_init_command() is in fast path, but it is called from
scsi_mq_prep_fn() directly, so it isn't related with ->init_request().


Thanks, 
Ming