All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Wilck <mwilck@suse.com>
To: Benjamin Marzinski <bmarzins@redhat.com>
Cc: lixiaokeng@huawei.com, dm-devel@redhat.com
Subject: Re: [PATCH 19/23] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit
Date: Tue, 29 Sep 2020 11:31:06 +0200	[thread overview]
Message-ID: <3ebea94ffd3be1a24c1f3a3159a337a7e878ce99.camel@suse.com> (raw)
In-Reply-To: <20200928202624.GP3384@octiron.msp.redhat.com>

On Mon, 2020-09-28 at 15:26 -0500, Benjamin Marzinski wrote:
> On Thu, Sep 24, 2020 at 03:40:50PM +0200, mwilck@suse.com wrote:
> > 
> >  /*
> >   * We don't support re-initialization after
> > @@ -65,6 +66,9 @@ int libmultipath_init(void)
> >  static void _libmultipath_exit(void)
> >  {
> >  	libmultipath_exit_called = true;
> > +	cleanup_foreign();
> 
> I don't really feel too strongly about this, but it seems to me that
> there is a difference between the checkers and prioritizers, which
> it seems like most users of libmultipath would want, and the foreign
> code, which doesn't seem that way. libmpathpersist, for instance,
> will use the checkers and prioritizers, but not the foreign code.
> On the other hand, if the caller isn't using the foreign code,
> then grabbing the lock and checking the foreign pointer shouldn't
> take much time.

It would just be a few cycles. I want callers to have to worry about
cleanup as little as possible. All else is error-prone IMO, and
although I agree that the foreign functions are less important than
checkers and prio, I thought it made sense to treat all our "plug-ins"
the same way.

Ideally I'd like to do checker/prio/foreign initialization completely
lazily too, in the sense that callers don't need to worry about calling
init_checkers() etc., either. But this series had to stop at some
point.

Either way, it's not a big issue, so please tell me if you feel
strongly enough about it to ask me to revert the change.

Regards,
Martin

  reply	other threads:[~2020-09-29  9:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-24 13:40 [PATCH 00/23] libmultipath: improve cleanup on exit mwilck
2020-09-24 13:40 ` [PATCH 01/23] multipathd: uxlsnr: avoid deadlock " mwilck
2020-09-26  1:52   ` Benjamin Marzinski
2020-09-26 13:00     ` Martin Wilck
2020-09-24 13:40 ` [PATCH 02/23] multipathd: Fix liburcu memory leak mwilck
2020-09-24 13:40 ` [PATCH 03/23] multipathd: move handling of io_err_stat_attr into libmultipath mwilck
2020-09-24 13:40 ` [PATCH 04/23] multipathd: move vecs desctruction into cleanup function mwilck
2020-09-24 13:40 ` [PATCH 05/23] multipathd: make some globals static mwilck
2020-09-24 13:40 ` [PATCH 06/23] multipathd: move threads destruction into separate function mwilck
2020-09-24 13:40 ` [PATCH 07/23] multipathd: move conf " mwilck
2020-09-28 18:03   ` Benjamin Marzinski
2020-09-29  9:12     ` Martin Wilck
2020-09-24 13:40 ` [PATCH 08/23] multipathd: move pid " mwilck
2020-09-24 13:40 ` [PATCH 09/23] multipathd: close pidfile on exit mwilck
2020-09-24 13:40 ` [PATCH 10/23] multipathd: add helper for systemd notification at exit mwilck
2020-09-24 13:40 ` [PATCH 11/23] multipathd: child(): call cleanups in failure case, too mwilck
2020-09-24 13:40 ` [PATCH 12/23] multipathd: unwatch_all_dmevents: check if waiter is initialized mwilck
2020-09-24 13:40 ` [PATCH 13/23] multipathd: print error message if config can't be loaded mwilck
2020-09-24 13:40 ` [PATCH 14/23] libmultipath: add libmp_dm_exit() mwilck
2020-09-28 18:41   ` Benjamin Marzinski
2020-09-24 13:40 ` [PATCH 15/23] multipathd: fixup libdm deinitialization mwilck
2020-09-24 13:40 ` [PATCH 16/23] libmultipath: log_thread_stop(): check if logarea is initialized mwilck
2020-09-24 13:40 ` [PATCH 17/23] multipathd: add cleanup_child() exit handler mwilck
2020-09-24 13:40 ` [PATCH 18/23] libmultipath: fix log_thread startup and teardown mwilck
2020-09-28 20:15   ` Benjamin Marzinski
2020-09-29  9:18     ` Martin Wilck
2020-09-24 13:40 ` [PATCH 19/23] multipathd: move cleanup_{prio, checkers, foreign} to libmultipath_exit mwilck
2020-09-28 20:26   ` Benjamin Marzinski
2020-09-29  9:31     ` Martin Wilck [this message]
2020-09-29 17:50       ` Benjamin Marzinski
2020-09-24 13:40 ` [PATCH 20/23] multipath: use atexit() for cleanup handlers mwilck
2020-09-24 13:40 ` [PATCH 21/23] mpathpersist: " mwilck
2020-09-24 13:40 ` [PATCH 22/23] multipath: fix leaks in check_path_valid() mwilck
2020-09-24 13:40 ` [PATCH 23/23] multipath-tools: mpath-tools.supp: file with valgrind suppressions mwilck
2020-09-24 20:58   ` Benjamin Marzinski
2020-09-25  9:53     ` Martin Wilck
2020-09-28 20:51 ` [PATCH 00/23] libmultipath: improve cleanup on exit Benjamin Marzinski

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=3ebea94ffd3be1a24c1f3a3159a337a7e878ce99.camel@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.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.