All of lore.kernel.org
 help / color / mirror / Atom feed
From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Benjamin Marzinski <bmarzins@redhat.com>
Cc: lixiaokeng@huawei.com, dm-devel@redhat.com,
	Martin Wilck <mwilck@suse.com>
Subject: [dm-devel] [PATCH v3 00/29] libmultipath: improve cleanup on exit
Date: Wed, 16 Dec 2020 19:16:39 +0100	[thread overview]
Message-ID: <20201216181708.22224-1-mwilck@suse.com> (raw)

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben, hi lixiaokeng,

this series was inspired by lixiaokeng's recent posting "[QUESTION] memory
leak in main (multipath)". It implements my first idea, registering
cleanup handlers with atexit(). However it turned out to be quite
complex. In particular multipathd has a lot of things to clean up.

This series is based on the previous series "multipath-tools: shutdown, 
libdevmapper races, globals" (v3).

While the bulk of the series is the cleanup handling, it also contains
some bug fixes for issues that I found while working on this.

Changes v2 -> v3:

Removed on_exit() calls, which break build on Alpine Linux (musl libc).

  02/29 "multipathd: Fix liburcu memory leak":
         use atexit() rather than on_exit()
  18/29 "libmultipath: fix log_thread startup and teardown"
         fixed rebase error pointed out by Ben
  19/29 "multipath: use atexit() for cleanup handlers"
         use atexit() rather than on_exit()
  21/29 "multipath: fix leaks in check_path_valid()"
         revert the on_exit() part of the previous version of this patch. As
	 argued in a separate mail, avoiding this corner-case "memory leak"
	 on an irregular exit isn't worth the ugliness of being non-reentrant
	 and using file-scope static variables for things that would naturally
	 be automatic variables.
  24/29 "libmultipath: use libmp_verbosity to track verbosity":
         Removed additional uses of conf->verbosity (Ben)
  25/29 "libmultipath: introduce symbolic values for logsink":
         removed blank line at end of libmultipath/debug.h (Ben)
  28/29 "multipathd: sanitize uxsock_listen()"
         unchanged, but modified commit message such that it doesn't suggest
	 memory was allocated in pages (Ben)
  29/29  "libmultipath: fix race between log_safe and log_thread_stop()":
         Ben had accepted the previous version after some discussion.
	 I moved the call to flush_logqueue() in log_thread_stop()
	 after the pthread_join() call.

Changes v1 -> v2:

Patches 24..29 have been added; they are logging cleanups, 24/29
and 29/29 are fixes to issues Ben had mentioned during his review.

01/29 "multipathd: uxlsnr: avoid deadlock on exit"
      Fix invalid array element reference (Ben)
07/29 "multipathd: move conf destruction into separate function"
      Don't reset logink when the log thread is stopped (Ben)
      Logging via logsink after shutting down the log thread is racy
      (no worse than before); the race will be fixed in 29/29.
14/29 "libmultipath: add libmp_dm_exit()": Fixed the skip_libmp_dm_init()
      case (Ben).
18/29 "libmultipath: fix log_thread startup and teardown"
      No need to wait before joining the log thread in log_thread_stop() (Ben)
      -> "libmultipath: fix race between log_safe and log_thread_stop()"
23/29: "multipath-tools: mpath-tools.supp: file with valgrind suppressions"
       Remove empty line at end of file (Ben)
24/29 "libmultipath: use libmp_verbosity to track verbosity"
       This fixes 13/21 "libmultipath: provide defaults for {get,put}_multipath_config"
       (from the previous patch series "multipath-tools: shutdown, libdevmapper races, globals";
       control of verbosity during config file loading)
29/29 "libmultipath: fix race between log_safe and log_thread_stop()"
      (see 18/29)

Regards
Martin

Martin Wilck (29):
  multipathd: uxlsnr: avoid deadlock on exit
  multipathd: Fix liburcu memory leak
  multipathd: move handling of io_err_stat_attr into libmultipath
  multipathd: move vecs desctruction into cleanup function
  multipathd: make some globals static
  multipathd: move threads destruction into separate function
  multipathd: move conf destruction into separate function
  multipathd: move pid destruction into separate function
  multipathd: close pidfile on exit
  multipathd: add helper for systemd notification at exit
  multipathd: child(): call cleanups in failure case, too
  multipathd: unwatch_all_dmevents: check if waiter is initialized
  multipathd: print error message if config can't be loaded
  libmultipath: add libmp_dm_exit()
  multipathd: fixup libdm deinitialization
  libmultipath: log_thread_stop(): check if logarea is initialized
  multipathd: add cleanup_child() exit handler
  libmultipath: fix log_thread startup and teardown
  multipathd: move cleanup_{prio,checkers,foreign} to libmultipath_exit
  multipath: use atexit() for cleanup handlers
  mpathpersist: use atexit() for cleanup handlers
  multipath: fix leaks in check_path_valid()
  multipath-tools: mpath-tools.supp: file with valgrind suppressions
  libmultipath: use libmp_verbosity to track verbosity
  libmultipath: introduce symbolic values for logsink
  libmultipath: simplify dlog()
  multipathd: common code for "-k" and command args
  multipathd: sanitize uxsock_listen()
  libmultipath: fix race between log_safe and log_thread_stop()

 libmpathpersist/mpath_persist.c       |   7 +-
 libmultipath/config.c                 |  14 +-
 libmultipath/config.h                 |   2 +
 libmultipath/configure.c              |  16 +-
 libmultipath/debug.c                  |  38 +--
 libmultipath/debug.h                  |  27 +-
 libmultipath/devmapper.c              |  30 +-
 libmultipath/devmapper.h              |   1 +
 libmultipath/io_err_stat.c            |   7 +-
 libmultipath/libmultipath.version     |  31 +-
 libmultipath/log_pthread.c            | 111 +++++---
 mpathpersist/main.c                   |   5 +-
 multipath/main.c                      |  91 +++---
 multipathd/dmevents.c                 |   2 +
 multipathd/main.c                     | 394 ++++++++++++++++----------
 multipathd/uxlsnr.c                   |  80 ++++--
 tests/alias.c                         |   1 +
 tests/blacklist.c                     |   2 +
 tests/globals.c                       |   3 +-
 tests/hwtable.c                       |   2 +-
 tests/test-log.c                      |   4 +-
 tests/test-log.h                      |   3 +-
 third-party/valgrind/mpath-tools.supp |  32 +++
 23 files changed, 557 insertions(+), 346 deletions(-)
 create mode 100644 third-party/valgrind/mpath-tools.supp

-- 
2.29.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


             reply	other threads:[~2020-12-16 18:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 18:16 mwilck [this message]
2020-12-16 18:16 ` [dm-devel] [PATCH v3 01/29] multipathd: uxlsnr: avoid deadlock on exit mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 02/29] multipathd: Fix liburcu memory leak mwilck
2020-12-17  1:19   ` Benjamin Marzinski
2020-12-16 18:16 ` [dm-devel] [PATCH v3 03/29] multipathd: move handling of io_err_stat_attr into libmultipath mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 04/29] multipathd: move vecs desctruction into cleanup function mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 05/29] multipathd: make some globals static mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 06/29] multipathd: move threads destruction into separate function mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 07/29] multipathd: move conf " mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 08/29] multipathd: move pid " mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 09/29] multipathd: close pidfile on exit mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 11/29] multipathd: child(): call cleanups in failure case, too mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 12/29] multipathd: unwatch_all_dmevents: check if waiter is initialized mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 13/29] multipathd: print error message if config can't be loaded mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 14/29] libmultipath: add libmp_dm_exit() mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 15/29] multipathd: fixup libdm deinitialization mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 16/29] libmultipath: log_thread_stop(): check if logarea is initialized mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 17/29] multipathd: add cleanup_child() exit handler mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 18/29] libmultipath: fix log_thread startup and teardown mwilck
2020-12-17  2:23   ` Benjamin Marzinski
2020-12-16 18:16 ` [dm-devel] [PATCH v3 19/29] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit mwilck
2020-12-16 18:16 ` [dm-devel] [PATCH v3 20/29] multipath: use atexit() for cleanup handlers mwilck
2020-12-17  2:40   ` Benjamin Marzinski
2020-12-16 18:17 ` [dm-devel] [PATCH v3 21/29] mpathpersist: " mwilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 22/29] multipath: fix leaks in check_path_valid() mwilck
2020-12-17  3:34   ` Benjamin Marzinski
2020-12-17  9:54     ` Martin Wilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 23/29] multipath-tools: mpath-tools.supp: file with valgrind suppressions mwilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 24/29] libmultipath: use libmp_verbosity to track verbosity mwilck
2020-12-17  3:39   ` Benjamin Marzinski
2020-12-16 18:17 ` [dm-devel] [PATCH v3 25/29] libmultipath: introduce symbolic values for logsink mwilck
2020-12-17  3:42   ` Benjamin Marzinski
2020-12-16 18:17 ` [dm-devel] [PATCH v3 26/29] libmultipath: simplify dlog() mwilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 27/29] multipathd: common code for "-k" and command args mwilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 28/29] multipathd: sanitize uxsock_listen() mwilck
2020-12-16 18:17 ` [dm-devel] [PATCH v3 29/29] libmultipath: fix race between log_safe and log_thread_stop() mwilck
2020-12-17  5:56   ` Benjamin Marzinski
2020-12-16 18:24 ` [dm-devel] [PATCH v3 00/29] libmultipath: improve cleanup on exit Martin Wilck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201216181708.22224-1-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    --cc=lixiaokeng@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.