On Thu, Oct 26, 2023 at 07:41:44PM +0200, mwilck@xxxxxxxx wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> 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@xxxxxxxx>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Cc: Li Xiao Keng <lixiaokeng@xxxxxxxxxx>
> Cc: Miao Guanqin <miaoguanqin@xxxxxxxxxx>
> Cc: Guan Junxiong <guanjunxiong@xxxxxxxxxx>
> ---
> 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
