> -----Original Message----- > From: Johannes Thumshirn [mailto:jthumshirn@xxxxxxx] > Sent: Monday, August 29, 2016 3:54 AM > To: Don Brace > Cc: jejb@xxxxxxxxxxxxxxxxxx; Viswas G; Mahesh Rajashekhara; > hch@xxxxxxxxxxxxx; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh; > elliott@xxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > 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 <kevin.barnett@xxxxxxxxxxxxx> > > > > Take advantage of controller improvements. > > > > Reviewed-by: Scott Teel <scott.teel@xxxxxxxxxxxxx> > > Reviewed-by: Scott Benesh <scott.benesh@xxxxxxxxxxxxx> > > Signed-off-by: Kevin Barnett <kevin.barnett@xxxxxxxxxxxxx> > > Signed-off-by: Don Brace <don.brace@xxxxxxxxxxxxx> > > --- > > 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 majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Johannes Thumshirn Storage > jthumshirn@xxxxxxx +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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
