Re: [PATCH V5 10/16] s390: zfcp_fc: use sg helper to operate scatterlist

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ming,

On 6/26/19 5:07 AM, Ming Lei wrote:
On Tue, Jun 25, 2019 at 12:51:04PM +0200, Steffen Maier wrote:
I don't mind doing this change for zfcp. However, I'm having doubts
regarding the rationale in the commit description.

However, I still suggest to do it because it will make us to audit SCSI chained
sg uses much easier. And the change shouldn't have performance effect.

If I was not mistaken above, the following could be more descriptive parts
of a patch/commit description, with hopefully less confusion for anyone
having to look at zfcp git history a few weeks/months/years from now:

"While not required for this SCSI MQ change regarding scatterlist
allocation, change all other scatterlist iterators in zfcp to the safe
sg_next() even if not necessary as these changed zfcp-internal scatterlists
are linear and unchained. This may avoid confusion about a potential need
for conversions in the future."

Sure, let's change the code, but could you please update the description of your below new patch to something like I drafted above regarding rationale?

If you would like to send a V2, I'll be happy to give a Reviewed-by.

  From c9c368308fefbf034d670984fe9746a4181fe514 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@xxxxxxxxxx>
Date: Tue, 25 Jun 2019 09:15:34 +0800
Subject: [PATCH] s390: scsi: use sg helper to iterate over scatterlist

Unlike the legacy I/O path, scsi-mq preallocates a large array to hold
the scatterlist for each request. This static allocation can consume
substantial amounts of memory on modern controllers which support a
large number of concurrently outstanding requests.

To facilitate a switch to a smaller static allocation combined with a
dynamic allocation for requests that need it, we need to make sure all
SCSI drivers handle chained scatterlists correctly.

Convert remaining drivers that directly dereference the scatterlist
array to using the iterator functions.

OK, I still suggest to apply the patch for the mentioned reason if you
are fine.



--
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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux