!143 fix coredump in io_err_stat
From: @foolstrong Reviewed-by: @swf504 Signed-off-by: @swf504
This commit is contained in:
commit
372be44064
@ -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
|
||||
Name: multipath-tools
|
||||
Version: 0.8.7
|
||||
Release: 8
|
||||
Release: 9
|
||||
Summary: Tools to manage multipath devices with the device-mapper
|
||||
License: GPL-2.0-or-later and LGPL-2.0-only
|
||||
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
|
||||
Patch22: 0022-multipathd-more-robust-command-parsing.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: gcc, libaio-devel, userspace-rcu-devel, device-mapper-devel >= 1.02.89
|
||||
@ -179,6 +181,9 @@ fi
|
||||
|
||||
|
||||
%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
|
||||
- set Source0 to URL link.
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user