[v2,0/6] Multipath io_err_stat fixes

Message

Benjamin Marzinski Jan. 15, 2021, 2:20 a.m. UTC

I found an ABBA deadlock in the io_err_stat marginal path code, and in
the process of fixing it, noticed a potential crash on shutdown. This
patchset addresses both of the issues.

Changes from v1:

0002: use cleanup_mutex instead of cleanup_unlock as suggested by
Martin

0003: add pthread_testcancel and use cleanup_mutex instead of
cleanup_unlock as suggested by Martin. Also, make tmp_pathvec a constant
pointer, since it should always equal _pathvec.

0004-0006 are new patches to deal with io_err_stat issues from Martin's
review

Benjamin Marzinski (6):
  libmultipath: make find_err_path_by_dev() static
  multipathd: avoid io_err_stat crash during shutdown
  multipathd: avoid io_err_stat ABBA deadlock
  multipathd: use get_monotonic_time() in io_err_stat code
  multipathd: combine free_io_err_stat_path and destroy_directio_ctx
  multipathd: cleanup logging for marginal paths

 libmultipath/io_err_stat.c | 216 ++++++++++++++++---------------------
 multipathd/main.c          |  25 +++--
 2 files changed, 105 insertions(+), 136 deletions(-)

Comments

Martin Wilck Jan. 15, 2021, 1:56 p.m. UTC | #1

On Thu, 2021-01-14 at 20:20 -0600, Benjamin Marzinski wrote:
> I found an ABBA deadlock in the io_err_stat marginal path code, and
> in
> the process of fixing it, noticed a potential crash on shutdown. This
> patchset addresses both of the issues.
> 
> Changes from v1:
> 
> 0002: use cleanup_mutex instead of cleanup_unlock as suggested by
> Martin
> 
> 0003: add pthread_testcancel and use cleanup_mutex instead of
> cleanup_unlock as suggested by Martin. Also, make tmp_pathvec a
> constant
> pointer, since it should always equal _pathvec.
> 
> 0004-0006 are new patches to deal with io_err_stat issues from
> Martin's
> review
> 
> Benjamin Marzinski (6):
>   libmultipath: make find_err_path_by_dev() static
>   multipathd: avoid io_err_stat crash during shutdown
>   multipathd: avoid io_err_stat ABBA deadlock
>   multipathd: use get_monotonic_time() in io_err_stat code
>   multipathd: combine free_io_err_stat_path and destroy_directio_ctx
>   multipathd: cleanup logging for marginal paths
> 
>  libmultipath/io_err_stat.c | 216 ++++++++++++++++-------------------
> --
>  multipathd/main.c          |  25 +++--
>  2 files changed, 105 insertions(+), 136 deletions(-)

For the series, except 6/6 where I had a nitpick:

Reviewed-by: Martin Wilck <[e-mail]>