> -----Original Message-----
> From: Johannes Thumshirn [mailto:[email protected]]
> Sent: Monday, August 29, 2016 3:54 AM
> To: Don Brace
> Cc: [email protected]; Viswas G; Mahesh Rajashekhara;
> [email protected]; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> [email protected]; [email protected]
> Subject: Re: [PATCH 01/17] smartpqi: change aio sg processing
>
> EXTERNAL EMAIL
>
>
> On Fri, Aug 26, 2016 at 11:44:11AM -0500, Don Brace wrote:
> > From: Kevin Barnett <[email protected]>
> >
> > Take advantage of controller improvements.
> >
> > Reviewed-by: Scott Teel <[email protected]>
> > Reviewed-by: Scott Benesh <[email protected]>
> > Signed-off-by: Kevin Barnett <[email protected]>
> > Signed-off-by: Don Brace <[email protected]>
> > ---
> > drivers/scsi/smartpqi/smartpqi_init.c | 68 +++++++++++++++++++--------
> ------
> > 1 file changed, 39 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> > index 906f1aa..418f636 100644
> > --- a/drivers/scsi/smartpqi/smartpqi_init.c
> > +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> > @@ -4263,48 +4263,58 @@ static int pqi_build_aio_sg_list(struct
> pqi_ctrl_info *ctrl_info,
> > int i;
> > u16 iu_length;
> > int sg_count;
> > - unsigned int num_sg_in_iu = 0;
> > + bool chained;
> > + unsigned int num_sg_in_iu;
> > + unsigned int max_sg_per_iu;
> > struct scatterlist *sg;
> > struct pqi_sg_descriptor *sg_descriptor;
> >
> > sg_count = scsi_dma_map(scmd);
> > if (sg_count < 0)
> > return sg_count;
> > +
> > + iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> > + PQI_REQUEST_HEADER_LENGTH;
> > + num_sg_in_iu = 0;
> > +
> > if (sg_count == 0)
> > goto out;
> >
> > - if (sg_count <= ctrl_info->max_sg_per_iu) {
> > - sg_descriptor = &request->sg_descriptors[0];
> > - scsi_for_each_sg(scmd, sg, sg_count, i) {
> > - pqi_set_sg_descriptor(sg_descriptor, sg);
> > - sg_descriptor++;
> > - }
> > - put_unaligned_le32(CISS_SG_LAST,
> > - &request->sg_descriptors[sg_count - 1].flags);
> > - num_sg_in_iu = sg_count;
> > - } else {
> > - sg_descriptor = &request->sg_descriptors[0];
> > - put_unaligned_le64((u64)io_request-
> >sg_chain_buffer_dma_handle,
> > - &sg_descriptor->address);
> > - put_unaligned_le32(sg_count * sizeof(*sg_descriptor),
> > - &sg_descriptor->length);
> > - put_unaligned_le32(CISS_SG_CHAIN, &sg_descriptor->flags);
> > -
> > - sg_descriptor = io_request->sg_chain_buffer;
> > - scsi_for_each_sg(scmd, sg, sg_count, i) {
> > - pqi_set_sg_descriptor(sg_descriptor, sg);
> > - sg_descriptor++;
> > + sg = scsi_sglist(scmd);
> > + sg_descriptor = request->sg_descriptors;
> > + max_sg_per_iu = ctrl_info->max_sg_per_iu - 1;
> > + chained = false;
> > + i = 0;
> > +
> > + while (1) {
>
> Is there any compelling reason why you didn't use scsi_for_each_sg()
> here? I don't see a reason for the while (1) construct.
>
The PQI chaining makes using scsi_for_each_sg a little more difficult to
maintain. We would prefer to leave the code as is.
>
> > + pqi_set_sg_descriptor(sg_descriptor, sg);
> > + if (!chained)
> > + num_sg_in_iu++;
> > + i++;
> > + if (i == sg_count)
> > + break;
> > + sg_descriptor++;
> > + if (i == max_sg_per_iu) {
> > + put_unaligned_le64(
> > + (u64)io_request->sg_chain_buffer_dma_handle,
> > + &sg_descriptor->address);
> > + put_unaligned_le32((sg_count - num_sg_in_iu)
> > + * sizeof(*sg_descriptor),
> > + &sg_descriptor->length);
> > + put_unaligned_le32(CISS_SG_CHAIN,
> > + &sg_descriptor->flags);
> > + chained = true;
> > + num_sg_in_iu++;
> > + sg_descriptor = io_request->sg_chain_buffer;
> > }
> > - put_unaligned_le32(CISS_SG_LAST,
> > - &io_request->sg_chain_buffer[sg_count - 1].flags);
> > - num_sg_in_iu = 1;
> > - request->partial = 1;
> > + sg = sg_next(sg);
> > }
> >
> > -out:
> > - iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> > - PQI_REQUEST_HEADER_LENGTH;
> > + put_unaligned_le32(CISS_SG_LAST, &sg_descriptor->flags);
> > + request->partial = chained;
> > iu_length += num_sg_in_iu * sizeof(*sg_descriptor);
> > +
> > +out:
> > put_unaligned_le16(iu_length, &request->header.iu_length);
> > request->num_sg_descriptors = num_sg_in_iu;
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Johannes Thumshirn Storage
> [email protected] +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
