Hi Ming,

On 6/18/19 3:37 AM, Ming Lei wrote:
Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: Steffen Maier <ma...@linux.ibm.com>
Cc: Benjamin Block <bbl...@linux.ibm.com>
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Cc: Heiko Carstens <heiko.carst...@de.ibm.com>
Cc: linux-s...@vger.kernel.org
Acked-by: Benjamin Block <bbl...@linux.ibm.com>
Reviewed-by: Christoph Hellwig <h...@lst.de>
Reviewed-by: Bart Van Assche <bvanass...@acm.org>
Signed-off-by: Ming Lei <ming....@redhat.com>
---
  drivers/s390/scsi/zfcp_fc.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 33eddb02ee30..b018b61bd168 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -620,7 +620,7 @@ static void zfcp_fc_sg_free_table(struct scatterlist *sg, 
int count)
  {
        int i;
- for (i = 0; i < count; i++, sg++)
+       for (i = 0; i < count; i++, sg = sg_next(sg))
                if (sg)
                        free_page((unsigned long) sg_virt(sg));
                else
@@ -641,7 +641,7 @@ static int zfcp_fc_sg_setup_table(struct scatterlist *sg, 
int count)
        int i;
sg_init_table(sg, count);
-       for (i = 0; i < count; i++, sg++) {
+       for (i = 0; i < count; i++, sg = sg_next(sg)) {
                addr = (void *) get_zeroed_page(GFP_KERNEL);
                if (!addr) {
                        zfcp_fc_sg_free_table(sg, i);


I'm still catching up with emails that came during my vacation, so I'm not fully up-to-date on the current state of this and how to bring in potential fixups on top.

I think, we also have two more (not so obvious) places in the corresponding response/completion code path, where we might need to introduce the proper iterator helper:

zfcp_fsf.c:

static int zfcp_fc_eval_gpn_ft(struct zfcp_fc_req *fc_req,
                               struct zfcp_adapter *adapter, int max_entries)
{
        struct scatterlist *sg = &fc_req->sg_rsp;
...
        /* first entry is the header */
        for (x = 1; x < max_entries && !last; x++) {
...
                if (x % (ZFCP_FC_GPN_FT_ENT_PAGE + 1))
...
                else
                        acc = sg_virt(++sg);
                                      ^^^^

zfcp_dbf.c:

static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
                                              struct zfcp_fsf_req *fsf,
                                              u16 len)
{
        struct scatterlist *resp_entry = ct_els->resp;
...
        /* the basic CT_IU preamble is the same size as one entry in the GPN_FT
         * response, allowing us to skip special handling for it - just skip it
         */
        for (x = 1; x < max_entries && !last; x++) {
                if (x % (ZFCP_FC_GPN_FT_ENT_PAGE + 1))
...
                else
                        acc = sg_virt(++resp_entry);
                                      ^^^^^^^^^^^^


What do you think?

--
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Reply via email to