| Message ID | 20231026174153.1133-6-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipath: aio, systemd, and documentation improvements | expand |
On Thu, Oct 26, 2023 at 07:41:44PM +0200, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > 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 c474c34..3f32e32 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.42.0
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c index c474c34..3f32e32 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); }