Re: [PATCH v5] nvme: multipath: Implemented new iopolicy "queue-depth"

[John Meneghini]

On 5/23/24 04:14, Christoph Hellwig wrote:
> Oh, and there is really something of with the patch format.
>
> First the subject line is completely wrong, this should be:
>
> nvme-multipath: implement "queue-depth" iopolicy.

Will fix.

> On Wed, May 22, 2024 at 12:54:06PM -0400, John Meneghini wrote:
> > From: "Ewan D. Milne" <emilne@xxxxxxxxxx>
>
> > Signed-off-by: Thomas Song <tsong@xxxxxxxxxxxxxxx>
> > [emilne: patch developed by Thomas Song @ Pure Storage, fixed whitespace
> >        and compilation warnings, updated MODULE_PARM description, and
> >        fixed potential issue with ->current_path[] being used]
> > Signed-off-by: Ewan D. Milne <emilne@xxxxxxxxxx>
>
> Second the patch author needs to be the first signoff.  I'm not entirely
> sure who is supposed to be the author, but either it should be Thomas,
> or if the patch has changed so much that it's someone else the original
> signoff should turn into a Co-Developed-by.

This patch was originally authored by Thomas Song, an intern who worked at Purestorage. The original patch was almost a year 
ago. Thomas no longer works for Purestorage, and he is no where to be found. Ewan and I have both made many changes since then, 
but the basic concept, which adds a controller scoped atomic counter in nvme_find_path(), remains the same. Ewan worked on the 
patch for several months, trying different counter implementations, all in an effort to avoid the atomic counter.  However, 
nothing else worked and in the end this patch retains the essential features of Thomas Songs original patch.

I'd like Thomas to get credit for this patch, so I've changed the author from Ewan Milne to Thomas Song.

As for the Co-Developed-By: checkpatch.pl is insisting that that each "Co-developed-by:" be followed by a signoff.

  jmeneghi:linux > scripts/checkpatch.pl --git HEAD
  WARNING: Co-developed-by: must be immediately followed by Signed-off-by:
  #26:
  Co-developed-by: Ewan D. Milne <emilne@xxxxxxxxxx>
  [jmeneghi: vairious changes and improvements, addressed review comments]

  WARNING: Co-developed-by: must be immediately followed by Signed-off-by:
  #28:
  Co-developed-by: John Meneghini <jmeneghi@xxxxxxxxxx>
  Link: https://lore.kernel.org/linux-nvme/20240509202929.831680-1-jmeneghi@xxxxxxxxxx/

  total: 0 errors, 2 warnings, 197 lines checked


So this is what I currently have for a message log.

Author: Thomas Song <tsong@xxxxxxxxxxxxxxx>
Date:   Tue Nov 7 16:23:29 2023 -0500

    nvme-multipath: implemented new iopolicy "queue-depth"

    The round-robin path selector is inefficient in cases where there is a
    difference in latency between paths.  In the presence of one or more
    high latency paths the round-robin selector continues to use the high
    latency path equally. This results in a bias towards the highest latency
    path and can cause a significant decrease in overall performance as IOs
    pile on the highest latency path. This problem is acute with NVMe-oF
    controllers.

    The queue-depth policy instead sends I/O requests down the path with the
    least amount of requests in its request queue. Paths with lower latency
    will clear requests more quickly and have less requests in their queues
    compared to higher latency paths. The goal of this path selector is to
    make more use of lower latency paths which will bring down overall IO
    latency and increase throughput and performance.

    Signed-off-by: Thomas Song <tsong@xxxxxxxxxxxxxxx>
    [emilne: patch developed by Thomas Song @ Pure Storage, fixed whitespace
          and compilation warnings, updated MODULE_PARM description, and
          fixed potential issue with ->current_path[] being used]
    Co-developed-by: Ewan D. Milne <emilne@xxxxxxxxxx>
    Signed-off-by: Ewan D. Milne <emilne@xxxxxxxxxx>
    [jmeneghi: vairious changes and improvements, addressed review comments]
    Co-developed-by: John Meneghini <jmeneghi@xxxxxxxxxx>
    Signed-off-by: John Meneghini <jmeneghi@xxxxxxxxxx>
    Link: https://lore.kernel.org/linux-nvme/20240509202929.831680-1-jmeneghi@xxxxxxxxxx/
    Tested-by: Marco Patalano <mpatalan@xxxxxxxxxx>
    Reviewed-by: Randy Jennings <randyj@xxxxxxxxxxxxxxx>
    Tested-by: Jyoti Rani <jrani@xxxxxxxxxxxxxxx>