[dm-devel] [PATCH v3 00/18] Multipath patch dump

Martin Wilck Martin.Wilck at suse.com
Wed Feb 19 08:55:08 UTC 2020


On Wed, 2020-02-19 at 00:48 -0600, Benjamin Marzinski wrote:
> This patch set is has multiple parts.
> 
> patch 01 is just a resubmit of my previous patch, with Martin's
> corrections added.
> 
> Patches 02 - 06 are miscellaneous fixes and cleanups
> 
> Patches 07 - 09 are related to adding a new format wildcard, at the
> 	request of HPE, that allows multipath to get and display
> 	information from the vendor specific VPD pages
> 
> Patches 10 - 18 are the part that needs the most review, patch 14
> 	especially. It turns out that on machines with a large number
> 	of cores, io_destroy(), which is used by the directio checker,
> 	can take a long time to complete. Talking to some kernel people
> 	at Red Hat, I was told that this isn't a bug. It's working as
> 	expected, and multipath shouldn't be issuing so many
> 	io_destroy() calls (1 per path, when multipath or multipathd
> 	stops). Cutting out the io_destroy calls involved a major
> change
> 	to the directio checker. It's pretty hard to test a lot of the
> 	corner cases on actual hardware, so I've written a bunch of
> 	unit tests for this (patches 16 & 17).
> 
> Changes in v3:
> This removes the conflicts with Martin's earlier patches,
> specifically
> 
> 0003-libmultipath-fix-leak-in-foreign-code.patch
> 	removed in favor of Martin's
> 	"libmultipath: _init_foreign(): fix possible memory leak"
> 	patch
> 
> 0016-fixup-libmultipath-add-code-to-get-vendor-specific-v.patch
> 0017-fixup-libmultipath-make-directio-checker-share-io-co.patch
> 	New patches to fix compile-time errors resulting from the
> 	rebase
> 
> 0018-tests-make-directio-tests-able-to-work-on-a-real-dev.patch
> 	Fix conflicts between with Martin's earlier patches
> 
> Changes in v2:
> 0001-multipathd-warn-when-configuration-has-been-changed.patch
> 	Changed to reflect Martin Wilck's comments
> 
> 0002-multipathd-staticify-uxlsnr-variables-functions.patch
> 	New patch
> 
> 0008-libmultipath-fix-sgio_get_vpd-looping.patch
> 	Test has been changed to keep create_vpd83 the same, and
> 	overwrite the length in the actual test, as suggested by
> 	Martin Wilck
> 
> 0010-libmultipath-add-code-to-get-vendor-specific-vpd-dat.patch
> 	The information to find the vpd page and how to decode it is
> 	now simply the index in a table of name -> page_nr mappings
> 
> 0012-libmultipath-change-failed-path-prio-timeout.patch
> 	The timeout argument has been renamed to avoid confusion,
> 	as suggested by Martin Wilck
> 
> 0015-libmultipath-make-directio-checker-share-io-contexts.patch
> 	Minor changes to print more useful messages, and avoid
> 	printing anything when we get a non-zero io_cancel()
> 	result, as suggested by Martin Wilck
> 
> 0016-tests-add-directio-unit-tests.patch
> 	Minor change to improve readability, as suggested by
> 	Martin Wilck
> 
> 0017-tests-make-directio-tests-able-to-work-on-a-real-dev.patch
> 	New patch. This adds the ability to set a testing device, so
> 	you can run the directio tests on an actual device
> 
> 
> 
> Benjamin Marzinski (16):
>   multipathd: warn when configuration has been changed.
>   multipathd: staticify uxlsnr variables/functions
>   Fix leak in mpathpersist
>   libmultipath: remove unused path->prio_args
>   libmultipath: constify get_unaligned_be*
>   libmultipath: add missing hwe mpe variable merges
>   libmultipath: fix sgio_get_vpd looping
>   libmultipath: add vend_id to get_vpd_sgio
>   libmultipath: add code to get vendor specific vpd data
>   libmultipath: change how the checker async is set
>   libmultipath: change failed path prio timeout
>   multipathd: add new paths under vecs lock
>   libmultipath: add new checker class functions
>   libmultipath: make directio checker share io contexts
>   tests: add directio unit tests
>   tests: make directio tests able to work on a real device
> 
> Martin Wilck (2):
>   fixup! libmultipath: add code to get vendor specific vpd data
>   fixup! libmultipath: make directio checker share io contexts
> 
>  libmpathpersist/mpath_persist.c  |   2 +-
>  libmultipath/checkers.c          |  29 +-
>  libmultipath/checkers.h          |   1 +
>  libmultipath/checkers/directio.c | 336 ++++++++++---
>  libmultipath/config.c            |  10 +
>  libmultipath/config.h            |   2 +
>  libmultipath/dict.c              |  38 ++
>  libmultipath/discovery.c         |  80 ++-
>  libmultipath/discovery.h         |   2 +-
>  libmultipath/hwtable.c           |   1 +
>  libmultipath/print.c             |  25 +
>  libmultipath/prio.c              |   6 +-
>  libmultipath/propsel.c           |  20 +-
>  libmultipath/propsel.h           |   1 +
>  libmultipath/structs.h           |  16 +-
>  libmultipath/unaligned.h         |   4 +-
>  mpathpersist/main.c              |   1 +
>  multipath/main.c                 |   1 +
>  multipath/multipath.conf.5       |  15 +-
>  multipathd/main.c                |  18 +-
>  multipathd/uxlsnr.c              | 150 +++++-
>  tests/Makefile                   |  19 +-
>  tests/directio.c                 | 812
> +++++++++++++++++++++++++++++++
>  tests/directio_test_dev          |   4 +
>  tests/vpd.c                      |  87 ++--
>  25 files changed, 1527 insertions(+), 153 deletions(-)
>  create mode 100644 tests/directio.c
>  create mode 100644 tests/directio_test_dev
> 

ACK for the series.

... one small nit: I'd rather not add the new file
"tests/directio_test_dev" from patch 18/18 to version control (git),
because the file is likely to be different in every environment, and
changes to the file will need to be stashed away before every branch
export, which is inconvenient. "make test" will work just fine without
the file. I'd prefer a tests/README file explaining how to
activate directio-on-real-device testing.

But this is not a reject, Christophe could fix this when merging, or
I'll post a fix.

Martin

-- 
Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer






More information about the dm-devel mailing list