fix coredump in io_err_stat
Signed-off-by: wangzhiqiang <wangzhiqiang95@huawei.com>
This commit is contained in:
parent
4cf54e7303
commit
e436876ec2
@ -0,0 +1,98 @@
|
|||||||
|
From eff53df4b1cdd74643f54568a327ae7b824337bb Mon Sep 17 00:00:00 2001
|
||||||
|
From: Martin Wilck <mwilck@suse.com>
|
||||||
|
Date: Wed, 27 Sep 2023 12:34:08 +0200
|
||||||
|
Subject: [PATCH] libmultipath: io_err_stat: don't free aio memory before
|
||||||
|
completion
|
||||||
|
|
||||||
|
It is wrong to assume that aio data structures can be reused or freed
|
||||||
|
after io_cancel(). io_cancel() will almost always return -EINPROGRESS,
|
||||||
|
anyway. Use the io_starttime field to indicate whether an io event
|
||||||
|
has been completed by the kernel. Make sure no in-flight buffers are freed.
|
||||||
|
|
||||||
|
Fixes https://github.com/opensvc/multipath-tools/issues/73.
|
||||||
|
|
||||||
|
Signed-off-by: Martin Wilck <mwilck@suse.com>
|
||||||
|
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
|
||||||
|
Cc: Li Xiao Keng <lixiaokeng@huawei.com>
|
||||||
|
Cc: Miao Guanqin <miaoguanqin@huawei.com>
|
||||||
|
Cc: Guan Junxiong <guanjunxiong@huawei.com>
|
||||||
|
---
|
||||||
|
libmultipath/io_err_stat.c | 26 ++++++++++++++++----------
|
||||||
|
1 file changed, 16 insertions(+), 10 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
|
||||||
|
index d8d91f6..480034b 100644
|
||||||
|
--- a/libmultipath/io_err_stat.c
|
||||||
|
+++ b/libmultipath/io_err_stat.c
|
||||||
|
@@ -112,10 +112,14 @@ static int init_each_dio_ctx(struct dio_ctx *ct, int blksize,
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
-static void deinit_each_dio_ctx(struct dio_ctx *ct)
|
||||||
|
+static int deinit_each_dio_ctx(struct dio_ctx *ct)
|
||||||
|
{
|
||||||
|
- if (ct->buf)
|
||||||
|
- free(ct->buf);
|
||||||
|
+ if (!ct->buf)
|
||||||
|
+ return 0;
|
||||||
|
+ if (ct->io_starttime.tv_sec != 0 || ct->io_starttime.tv_nsec != 0)
|
||||||
|
+ return 1;
|
||||||
|
+ free(ct->buf);
|
||||||
|
+ return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
static int setup_directio_ctx(struct io_err_stat_path *p)
|
||||||
|
@@ -164,6 +168,7 @@ fail_close:
|
||||||
|
static void free_io_err_stat_path(struct io_err_stat_path *p)
|
||||||
|
{
|
||||||
|
int i;
|
||||||
|
+ int inflight = 0;
|
||||||
|
|
||||||
|
if (!p)
|
||||||
|
return;
|
||||||
|
@@ -173,8 +178,13 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
|
||||||
|
cancel_inflight_io(p);
|
||||||
|
|
||||||
|
for (i = 0; i < CONCUR_NR_EVENT; i++)
|
||||||
|
- deinit_each_dio_ctx(p->dio_ctx_array + i);
|
||||||
|
- FREE(p->dio_ctx_array);
|
||||||
|
+ inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
|
||||||
|
+
|
||||||
|
+ if (!inflight)
|
||||||
|
+ FREE(p->dio_ctx_array);
|
||||||
|
+ else
|
||||||
|
+ io_err_stat_log(2, "%s: can't free aio space of %s, %d IOs in flight",
|
||||||
|
+ __func__, p->devname, inflight);
|
||||||
|
|
||||||
|
if (p->fd > 0)
|
||||||
|
close(p->fd);
|
||||||
|
@@ -503,7 +513,7 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
|
||||||
|
int rc = PATH_UNCHECKED;
|
||||||
|
int r;
|
||||||
|
|
||||||
|
- if (ct->io_starttime.tv_sec == 0)
|
||||||
|
+ if (ct->io_starttime.tv_sec == 0 && ct->io_starttime.tv_nsec == 0)
|
||||||
|
return rc;
|
||||||
|
timespecsub(t, &ct->io_starttime, &difftime);
|
||||||
|
if (difftime.tv_sec > IOTIMEOUT_SEC) {
|
||||||
|
@@ -514,8 +524,6 @@ static int try_to_cancel_timeout_io(struct dio_ctx *ct, struct timespec *t,
|
||||||
|
if (r)
|
||||||
|
io_err_stat_log(5, "%s: io_cancel error %i",
|
||||||
|
dev, errno);
|
||||||
|
- ct->io_starttime.tv_sec = 0;
|
||||||
|
- ct->io_starttime.tv_nsec = 0;
|
||||||
|
rc = PATH_TIMEOUT;
|
||||||
|
} else {
|
||||||
|
rc = PATH_PENDING;
|
||||||
|
@@ -559,8 +567,6 @@ static void cancel_inflight_io(struct io_err_stat_path *pp)
|
||||||
|
if (r)
|
||||||
|
io_err_stat_log(5, "%s: io_cancel error %d, %i",
|
||||||
|
pp->devname, r, errno);
|
||||||
|
- ct->io_starttime.tv_sec = 0;
|
||||||
|
- ct->io_starttime.tv_nsec = 0;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
--
|
||||||
|
2.33.0
|
||||||
|
|
||||||
@ -0,0 +1,70 @@
|
|||||||
|
From 26dab1986820b1d84c27ea02c5343edd88383842 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Martin Wilck <mwilck@suse.com>
|
||||||
|
Date: Thu, 19 Oct 2023 18:53:33 +0200
|
||||||
|
Subject: [PATCH] libmultipath: io_err_stat: call io_destroy() inside
|
||||||
|
free_io_err_pathvec()
|
||||||
|
|
||||||
|
Memory used by aio iocbs must not be freed before either all iocbs
|
||||||
|
have finished, or io_destroy() has been called (which will wait until
|
||||||
|
all iocbs have finished).
|
||||||
|
|
||||||
|
Don't call cancel_inflight_io() from free_io_err_stat_path(). Rather,
|
||||||
|
cancel directly from free_io_err_pathvec(). This way we make sure that
|
||||||
|
we can't call io_cancel() with an already destroyed ioctx, and that
|
||||||
|
we don't free memory of in-flight requests.
|
||||||
|
|
||||||
|
The other callers of free_io_err_stat_path() also don't need to call
|
||||||
|
cancel_inflight_io(). In service_paths(), where free_io_err_stat_path()
|
||||||
|
it is called for paths on which all IO has either finished or timed out,
|
||||||
|
hanging requests will already have been cancelled in the
|
||||||
|
try_to_cancel_timeout_io() code path (note that total_time is at least
|
||||||
|
2 * IO_TIMEOUT_SEC). In the failure case of enqueue_io_err_stat_by_path(), no
|
||||||
|
IO has been submitted yet.
|
||||||
|
|
||||||
|
Signed-off-by: Martin Wilck <mwilck@suse.com>
|
||||||
|
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
|
||||||
|
Cc: Li Xiao Keng <lixiaokeng@huawei.com>
|
||||||
|
Cc: Miao Guanqin <miaoguanqin@huawei.com>
|
||||||
|
Cc: Guan Junxiong <guanjunxiong@huawei.com>
|
||||||
|
---
|
||||||
|
libmultipath/io_err_stat.c | 12 +++++++++---
|
||||||
|
1 file changed, 9 insertions(+), 3 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
|
||||||
|
index c474c340..3f32e32e 100644
|
||||||
|
--- a/libmultipath/io_err_stat.c
|
||||||
|
+++ b/libmultipath/io_err_stat.c
|
||||||
|
@@ -175,8 +175,6 @@ static void free_io_err_stat_path(struct io_err_stat_path *p)
|
||||||
|
if (!p->dio_ctx_array)
|
||||||
|
goto free_path;
|
||||||
|
|
||||||
|
- cancel_inflight_io(p);
|
||||||
|
-
|
||||||
|
for (i = 0; i < CONCUR_NR_EVENT; i++)
|
||||||
|
inflight += deinit_each_dio_ctx(p->dio_ctx_array + i);
|
||||||
|
|
||||||
|
@@ -221,6 +219,15 @@ static void free_io_err_pathvec(void)
|
||||||
|
pthread_cleanup_push(cleanup_mutex, &io_err_pathvec_lock);
|
||||||
|
if (!io_err_pathvec)
|
||||||
|
goto out;
|
||||||
|
+
|
||||||
|
+ /* io_cancel() is a noop, but maybe in the future it won't be */
|
||||||
|
+ vector_foreach_slot(io_err_pathvec, path, i) {
|
||||||
|
+ if (path && path->dio_ctx_array)
|
||||||
|
+ cancel_inflight_io(path);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ /* This blocks until all I/O is finished */
|
||||||
|
+ io_destroy(ioctx);
|
||||||
|
vector_foreach_slot(io_err_pathvec, path, i)
|
||||||
|
free_io_err_stat_path(path);
|
||||||
|
vector_free(io_err_pathvec);
|
||||||
|
@@ -752,5 +759,4 @@ void stop_io_err_stat_thread(void)
|
||||||
|
|
||||||
|
pthread_join(io_err_stat_thr, NULL);
|
||||||
|
free_io_err_pathvec();
|
||||||
|
- io_destroy(ioctx);
|
||||||
|
}
|
||||||
|
--
|
||||||
|
2.33.0
|
||||||
|
|
||||||
@ -1,7 +1,7 @@
|
|||||||
#needsrootforbuild
|
#needsrootforbuild
|
||||||
Name: multipath-tools
|
Name: multipath-tools
|
||||||
Version: 0.8.7
|
Version: 0.8.7
|
||||||
Release: 8
|
Release: 9
|
||||||
Summary: Tools to manage multipath devices with the device-mapper
|
Summary: Tools to manage multipath devices with the device-mapper
|
||||||
License: GPL-2.0-or-later and LGPL-2.0-only
|
License: GPL-2.0-or-later and LGPL-2.0-only
|
||||||
URL: http://christophe.varoqui.free.fr/
|
URL: http://christophe.varoqui.free.fr/
|
||||||
@ -32,6 +32,8 @@ Patch20: 0020-multipathd-make-all-cli_handlers-static.patch
|
|||||||
Patch21: 0021-multipathd-Fix-command-completion-in-interactive-mod.patch
|
Patch21: 0021-multipathd-Fix-command-completion-in-interactive-mod.patch
|
||||||
Patch22: 0022-multipathd-more-robust-command-parsing.patch
|
Patch22: 0022-multipathd-more-robust-command-parsing.patch
|
||||||
Patch23: 0023-multipathd-Fixed-multipathd-parameter-invoking-seque.patch
|
Patch23: 0023-multipathd-Fixed-multipathd-parameter-invoking-seque.patch
|
||||||
|
Patch24: 0024-libmultipath-io_err_stat-don-t-free-aio-memory-befor.patch
|
||||||
|
Patch25: 0025-libmultipath-io_err_stat-call-io_destroy-inside-free.patch
|
||||||
|
|
||||||
BuildRequires: multipath-tools, libcmocka, libcmocka-devel
|
BuildRequires: multipath-tools, libcmocka, libcmocka-devel
|
||||||
BuildRequires: gcc, libaio-devel, userspace-rcu-devel, device-mapper-devel >= 1.02.89
|
BuildRequires: gcc, libaio-devel, userspace-rcu-devel, device-mapper-devel >= 1.02.89
|
||||||
@ -179,6 +181,9 @@ fi
|
|||||||
|
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Tue Dec 19 2023 wangzhiqiang <wangzhiqiang95@huawei.com> - 0.8.7-9
|
||||||
|
- fix coredump in io_err_stat
|
||||||
|
|
||||||
* Tue Nov 29 2022 Zhiqiang Liu <liuzhiqiang26@huawei.com> - 0.8.7-8
|
* Tue Nov 29 2022 Zhiqiang Liu <liuzhiqiang26@huawei.com> - 0.8.7-8
|
||||||
- set Source0 to URL link.
|
- set Source0 to URL link.
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user