| Message ID | 20230304003103.2572793-3-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Constify most SCSI host templates | expand |
On Fri, Mar 03, 2023 at 04:29:44PM -0800, Bart Van Assche wrote: > Prepare for constifying most SCSI host template pointers by constifying > the SCSI host template pointer arguments and variables in the SCSI core. > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Hannes Reinecke <hare@suse.de> > Cc: John Garry <john.garry@huawei.com> > Cc: Mike Christie <michael.christie@oracle.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/scsi/scsi_error.c | 16 ++++++++-------- > drivers/scsi/scsi_sysfs.c | 6 +++--- > 2 files changed, 11 insertions(+), 11 deletions(-) > Looks good to me. Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
On 04/03/2023 00:29, Bart Van Assche wrote: > Prepare for constifying most SCSI host template pointers by constifying > the SCSI host template pointer arguments and variables in the SCSI core. > > Cc: Christoph Hellwig<hch@lst.de> > Cc: Ming Lei<ming.lei@redhat.com> > Cc: Hannes Reinecke<hare@suse.de> > Cc: John Garry<john.garry@huawei.com> > Cc: Mike Christie<michael.christie@oracle.com> > Signed-off-by: Bart Van Assche<bvanassche@acm.org> Hi Bart, You wrote that most pointers were now cast as const - which ones where not? From a quick scan they all seem to be const Apart from that: Reviewed-by: John Garry <john.g.garry@oracle.com> Thanks, John
On 3/6/23 06:29, John Garry wrote: > You wrote that most pointers were now cast as const - which ones where > not? From a quick scan they all seem to be const Hi Garry, Some SCSI drivers modify one of more members of the SCSI host template. An example can be found in drivers/scsi/pcmcia/nsp_cs.c: sht->name = data->nspinfo; Another example from drivers/scsi/bnx2fc/bnx2fc_fcoe.c: bnx2fc_shost_template.can_queue = hba->max_outstanding_cmds; Thanks, Bart.
On 06/03/2023 16:07, Bart Van Assche wrote: > On 3/6/23 06:29, John Garry wrote: >> You wrote that most pointers were now cast as const - which ones were >> not? From a quick scan they all seem to be const > > Hi Garry, > > Some SCSI drivers modify one of more members of the SCSI host template. > An example can be found in drivers/scsi/pcmcia/nsp_cs.c: I seemed to get the wrong idea of what you meant in the commit message. When you wrote "Prepare for constifying most SCSI host template pointers", I got the impression that most of the pointers to SCSI host template in the core code were going to be pointers to const. However you really meant that most of the per-driver SCSI host template instances would be const. Anyway, Reviewed-by: John Garry <john.g.garry@oracle.com> > > sht->name = data->nspinfo; > > Another example from drivers/scsi/bnx2fc/bnx2fc_fcoe.c: > > bnx2fc_shost_template.can_queue = hba->max_outstanding_cmds; BTW, surely we should be setting shost->can_queue = hba->max_outstanding_cmds after scsi_host_alloc() and not modifying bnx2fc_shost_template, right? The series is already huge, so this stuff would be done separately, I suppose. Thanks, John
On 3/6/23 10:55, John Garry wrote: > On 06/03/2023 16:07, Bart Van Assche wrote: >> Another example from drivers/scsi/bnx2fc/bnx2fc_fcoe.c: >> >> bnx2fc_shost_template.can_queue = hba->max_outstanding_cmds; > > BTW, surely we should be setting shost->can_queue = > hba->max_outstanding_cmds after scsi_host_alloc() and not modifying > bnx2fc_shost_template, right? The series is already huge, so this stuff > would be done separately, I suppose. Hi John, If anyone else wants to work on this that's fine with me. My view is that the SCSI core should support declaring host templates const but I'm not sure it's worth it to make changes in old drivers such that their SCSI host template can be declared const. One class of SCSI LLDs that does not have a const SCSI host template are the NCR drivers. The NCR SCSI host controller was popular 40 years ago. There are probably not many working SCSI devices left that are based on this SCSI controller. Bart.
On Mon, 6 Mar 2023, Bart Van Assche wrote: > On 3/6/23 10:55, John Garry wrote: > > On 06/03/2023 16:07, Bart Van Assche wrote: > >> Another example from drivers/scsi/bnx2fc/bnx2fc_fcoe.c: > >> > >> bnx2fc_shost_template.can_queue = hba->max_outstanding_cmds; > > > > BTW, surely we should be setting shost->can_queue = > > hba->max_outstanding_cmds after scsi_host_alloc() and not modifying > > bnx2fc_shost_template, right? The series is already huge, so this stuff > > would be done separately, I suppose. > > Hi John, > > If anyone else wants to work on this that's fine with me. My view is > that the SCSI core should support declaring host templates const but I'm > not sure it's worth it to make changes in old drivers such that their > SCSI host template can be declared const. Would it alter the driver .o files? If not, the changes won't require actual testing. > One class of SCSI LLDs that does not have a const SCSI host template are > the NCR drivers. The NCR SCSI host controller was popular 40 years ago. > There are probably not many working SCSI devices left that are based on > this SCSI controller. > True, the NCR 5380 controller was popular 40 years ago among early adopters like Sun and Apple. Is this relevant?
On 3/6/23 15:34, Finn Thain wrote: > Would it alter the driver .o files? If not, the changes won't require > actual testing. Hi Finn, My understanding is that declaring a static data structure const may move it to another section in the .o file but otherwise that the .o file should not be affected. I agree that retesting shouldn't be necessary for patches that only involve making data structures const. Thanks, Bart.
On Sat, Mar 4, 2023 at 8:31 AM Bart Van Assche <bvanassche@acm.org> wrote: > > Prepare for constifying most SCSI host template pointers by constifying > the SCSI host template pointer arguments and variables in the SCSI core. > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Hannes Reinecke <hare@suse.de> > Cc: John Garry <john.garry@huawei.com> > Cc: Mike Christie <michael.christie@oracle.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Ming Lei <ming.lei@redhat.com>
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 2aa2c2aee6e7..3ec8bfd4090f 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -58,7 +58,7 @@ #define HOST_RESET_SETTLE_TIME (10) static int scsi_eh_try_stu(struct scsi_cmnd *scmd); -static enum scsi_disposition scsi_try_to_abort_cmd(struct scsi_host_template *, +static enum scsi_disposition scsi_try_to_abort_cmd(const struct scsi_host_template *, struct scsi_cmnd *); void scsi_eh_wakeup(struct Scsi_Host *shost) @@ -699,7 +699,7 @@ EXPORT_SYMBOL_GPL(scsi_check_sense); static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) { - struct scsi_host_template *sht = sdev->host->hostt; + const struct scsi_host_template *sht = sdev->host->hostt; struct scsi_device *tmp_sdev; if (!sht->track_queue_depth || @@ -731,7 +731,7 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) static void scsi_handle_queue_full(struct scsi_device *sdev) { - struct scsi_host_template *sht = sdev->host->hostt; + const struct scsi_host_template *sht = sdev->host->hostt; struct scsi_device *tmp_sdev; if (!sht->track_queue_depth) @@ -840,7 +840,7 @@ static enum scsi_disposition scsi_try_host_reset(struct scsi_cmnd *scmd) unsigned long flags; enum scsi_disposition rtn; struct Scsi_Host *host = scmd->device->host; - struct scsi_host_template *hostt = host->hostt; + const struct scsi_host_template *hostt = host->hostt; SCSI_LOG_ERROR_RECOVERY(3, shost_printk(KERN_INFO, host, "Snd Host RST\n")); @@ -870,7 +870,7 @@ static enum scsi_disposition scsi_try_bus_reset(struct scsi_cmnd *scmd) unsigned long flags; enum scsi_disposition rtn; struct Scsi_Host *host = scmd->device->host; - struct scsi_host_template *hostt = host->hostt; + const struct scsi_host_template *hostt = host->hostt; SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, "%s: Snd Bus RST\n", __func__)); @@ -912,7 +912,7 @@ static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd) unsigned long flags; enum scsi_disposition rtn; struct Scsi_Host *host = scmd->device->host; - struct scsi_host_template *hostt = host->hostt; + const struct scsi_host_template *hostt = host->hostt; if (!hostt->eh_target_reset_handler) return FAILED; @@ -941,7 +941,7 @@ static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd) static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd) { enum scsi_disposition rtn; - struct scsi_host_template *hostt = scmd->device->host->hostt; + const struct scsi_host_template *hostt = scmd->device->host->hostt; if (!hostt->eh_device_reset_handler) return FAILED; @@ -970,7 +970,7 @@ static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd) * link down on FibreChannel) */ static enum scsi_disposition -scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct scsi_cmnd *scmd) +scsi_try_to_abort_cmd(const struct scsi_host_template *hostt, struct scsi_cmnd *scmd) { if (!hostt->eh_abort_handler) return FAILED; diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index ee28f73af4d4..603e8fcfcb8a 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -296,7 +296,7 @@ store_host_reset(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct Scsi_Host *shost = class_to_shost(dev); - struct scsi_host_template *sht = shost->hostt; + const struct scsi_host_template *sht = shost->hostt; int ret = -EINVAL; int type; @@ -1025,7 +1025,7 @@ sdev_store_queue_depth(struct device *dev, struct device_attribute *attr, { int depth, retval; struct scsi_device *sdev = to_scsi_device(dev); - struct scsi_host_template *sht = sdev->host->hostt; + const struct scsi_host_template *sht = sdev->host->hostt; if (!sht->change_queue_depth) return -EINVAL; @@ -1606,7 +1606,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev) { unsigned long flags; struct Scsi_Host *shost = sdev->host; - struct scsi_host_template *hostt = shost->hostt; + const struct scsi_host_template *hostt = shost->hostt; struct scsi_target *starget = sdev->sdev_target; device_initialize(&sdev->sdev_gendev);
Prepare for constifying most SCSI host template pointers by constifying the SCSI host template pointer arguments and variables in the SCSI core. Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: John Garry <john.garry@huawei.com> Cc: Mike Christie <michael.christie@oracle.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/scsi/scsi_error.c | 16 ++++++++-------- drivers/scsi/scsi_sysfs.c | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-)