On Sat, 2020-09-19 at 17:14 -0500, Benjamin Marzinski wrote:
> On Wed, Sep 16, 2020 at 05:37:09PM +0200, mwilck@xxxxxxxx wrote:
> > From: Martin Wilck <mwilck@xxxxxxxx>
> >
> > As one step towards bundling all possibly racy libdm init calls
> > into a single
> > function, split the code for determining and checking versions of
> > libdm and kernel components. Provide a generic helper
> > libmp_get_version() that makes sure the versions are "lazily"
> > initialized.
> > Note that retrieving the versions requires libdm calls, thus the
> > version initialization calls libdm initialization, which might call
> > version initialization recursively. But that's not an issue, as
> > the initialization is protected by pthread_once().
>
> This is confusing me. dm_tgt_version will call
> DM_DEVICE_LIST_VERSIONS,
> but it doesn't call libmp_dm_task_create(), so I don't see the
> recursion
> possiblity.
You are right, I was confused.
The callstack for "lazy" initialization looks like this:
libmp_dm_task_create
pthread_once(&dm_initialized, libmp_dm_init);
libmp_dm_init()
=> dm_prereq()
init_versions
pthread_once(&versions_initialized, _init_versions);
_init_versions()
init_dm_library_version()
init_dm_drv_version()
init_dm_mpath_version()
dm_tgt_version
** dm_task_create
libmp_task_run()
pthread_mutex_lock(&libmp_dm_lock);
dm_task_run()
(If an application called dm_prereq() explicitly, it would enter
the stack at "=>", which would be less of a problem).
I suppose I should add a comment at "**" saying that we must not call
libmp_dm_task_create() there to avoid recursion. I also notice that my
function naming I used is inconsistent; it should be
libmp_dm_task_run() rather than just libmp_task_run().
> That's good because according to the man page:
>
> "If you specify a routine that directly or indirectly results in a
> recursive call to pthread_once(3) and that specifies the same routine
> argument, the recursive call can result in a deadlock."
Thanks for pointing this out. I had missed that indeed.
I see this paragraph only in some old DECThreads man pages on the web.
There's a slighly different "APPLICATION USAGE" statement under
https://pubs.opengroup.org/onlinepubs/9699919799/. The pragraph is
missing in https://man7.org/linux/man-pages/man3/pthread_once.3p.html,
which is still based on POSIX.1-2008, 2013 edition - that's why I
missed it.
Anyway, it makes sense, because pthread_once() guarantees that the init
routine has _finished_ when it returns, which can't work with recursive
calls.
Anyway, I'll send a v2.
Regards
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
