| Message ID | 20220208172514.3481-45-bvanassche@acm.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Remove the SCSI pointer from struct scsi_cmnd | expand |
On 08/02/2022 17:25, Bart Van Assche wrote: > Remove struct scsi_pointer from struct scsi_cmnd since the previous patches > removed all users of that member of struct scsi_cmnd. Additionally, reorder > the members of struct scsi_cmnd such that the statement that the field > below can be modified by the SCSI LLD is again correct. > > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Hannes Reinecke <hare@suse.com> > Cc: Christoph Hellwig <hch@lst.de> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> FWIW, regadless of a nitpicking comment, below: Reviewed-by: John Garry <john.garry@huawei.com> > --- > include/scsi/scsi_cmnd.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h > index 6794d7322cbd..4fd2c522e914 100644 > --- a/include/scsi/scsi_cmnd.h > +++ b/include/scsi/scsi_cmnd.h > @@ -123,11 +123,15 @@ struct scsi_cmnd { > * command (auto-sense). Length must be > * SCSI_SENSE_BUFFERSIZE bytes. */ > > + int flags; /* Command flags */ > + unsigned long state; /* Command completion state */ > + > + unsigned int extra_len; /* length of alignment and padding */ > + > /* > - * The following fields can be written to by the host specific code. > - * Everything else should be left alone. > + * The fields below can be modified by the SCSI LLD but the fields > + * above not. "but the fields above not" - this sounds a bit odd, maybe this is better: "but the fields above may not be modified" And I don't think that we need to mention SCSI in that comment .. Thanks, John > */ > - struct scsi_pointer SCp; /* Scratchpad used by some host adapters */ > > unsigned char *host_scribble; /* The host adapter is allowed to > * call scsi_malloc and get some memory > @@ -138,10 +142,6 @@ struct scsi_cmnd { > * to be at an address < 16Mb). */ > > int result; /* Status code from lower level driver */ > - int flags; /* Command flags */ > - unsigned long state; /* Command completion state */ > - > - unsigned int extra_len; /* length of alignment and padding */ > }; > > /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */ > .
> On Feb 8, 2022, at 9:25 AM, Bart Van Assche <bvanassche@acm.org> wrote: > > Remove struct scsi_pointer from struct scsi_cmnd since the previous patches > removed all users of that member of struct scsi_cmnd. Additionally, reorder > the members of struct scsi_cmnd such that the statement that the field > below can be modified by the SCSI LLD is again correct. > > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Hannes Reinecke <hare@suse.com> > Cc: Christoph Hellwig <hch@lst.de> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > include/scsi/scsi_cmnd.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h > index 6794d7322cbd..4fd2c522e914 100644 > --- a/include/scsi/scsi_cmnd.h > +++ b/include/scsi/scsi_cmnd.h > @@ -123,11 +123,15 @@ struct scsi_cmnd { > * command (auto-sense). Length must be > * SCSI_SENSE_BUFFERSIZE bytes. */ > > + int flags; /* Command flags */ > + unsigned long state; /* Command completion state */ > + > + unsigned int extra_len; /* length of alignment and padding */ > + > /* > - * The following fields can be written to by the host specific code. > - * Everything else should be left alone. > + * The fields below can be modified by the SCSI LLD but the fields > + * above not. > */ > - struct scsi_pointer SCp; /* Scratchpad used by some host adapters */ > > unsigned char *host_scribble; /* The host adapter is allowed to > * call scsi_malloc and get some memory > @@ -138,10 +142,6 @@ struct scsi_cmnd { > * to be at an address < 16Mb). */ > > int result; /* Status code from lower level driver */ > - int flags; /* Command flags */ > - unsigned long state; /* Command completion state */ > - > - unsigned int extra_len; /* length of alignment and padding */ > }; > > /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */ Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com> -- Himanshu Madhani Oracle Linux Engineering
On 2/8/22 09:54, John Garry wrote: > On 08/02/2022 17:25, Bart Van Assche wrote: >> /* >> - * The following fields can be written to by the host specific code. >> - * Everything else should be left alone. >> + * The fields below can be modified by the SCSI LLD but the fields >> + * above not. > > "but the fields above not" - this sounds a bit odd, maybe this is > better: "but the fields above may not be modified" may not -> must not? See also https://en.wiktionary.org/wiki/must_not. Anyway, I can make that change. > And I don't think that we need to mention SCSI in that comment .. I can leave the word "SCSI" out. Thanks, Bart.
On 2/8/22 18:25, Bart Van Assche wrote: > Remove struct scsi_pointer from struct scsi_cmnd since the previous patches > removed all users of that member of struct scsi_cmnd. Additionally, reorder > the members of struct scsi_cmnd such that the statement that the field > below can be modified by the SCSI LLD is again correct. > > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Hannes Reinecke <hare@suse.com> > Cc: Christoph Hellwig <hch@lst.de> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > include/scsi/scsi_cmnd.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h > index 6794d7322cbd..4fd2c522e914 100644 > --- a/include/scsi/scsi_cmnd.h > +++ b/include/scsi/scsi_cmnd.h > @@ -123,11 +123,15 @@ struct scsi_cmnd { > * command (auto-sense). Length must be > * SCSI_SENSE_BUFFERSIZE bytes. */ > > + int flags; /* Command flags */ > + unsigned long state; /* Command completion state */ > + > + unsigned int extra_len; /* length of alignment and padding */ > + > /* > - * The following fields can be written to by the host specific code. > - * Everything else should be left alone. > + * The fields below can be modified by the SCSI LLD but the fields > + * above not. > */ > - struct scsi_pointer SCp; /* Scratchpad used by some host adapters */ > > unsigned char *host_scribble; /* The host adapter is allowed to > * call scsi_malloc and get some memory > @@ -138,10 +142,6 @@ struct scsi_cmnd { > * to be at an address < 16Mb). */ > > int result; /* Status code from lower level driver */ > - int flags; /* Command flags */ > - unsigned long state; /* Command completion state */ > - > - unsigned int extra_len; /* length of alignment and padding */ > }; > > /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */ Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 6794d7322cbd..4fd2c522e914 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -123,11 +123,15 @@ struct scsi_cmnd { * command (auto-sense). Length must be * SCSI_SENSE_BUFFERSIZE bytes. */ + int flags; /* Command flags */ + unsigned long state; /* Command completion state */ + + unsigned int extra_len; /* length of alignment and padding */ + /* - * The following fields can be written to by the host specific code. - * Everything else should be left alone. + * The fields below can be modified by the SCSI LLD but the fields + * above not. */ - struct scsi_pointer SCp; /* Scratchpad used by some host adapters */ unsigned char *host_scribble; /* The host adapter is allowed to * call scsi_malloc and get some memory @@ -138,10 +142,6 @@ struct scsi_cmnd { * to be at an address < 16Mb). */ int result; /* Status code from lower level driver */ - int flags; /* Command flags */ - unsigned long state; /* Command completion state */ - - unsigned int extra_len; /* length of alignment and padding */ }; /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */