On Thu, Oct 26, 2023 at 07:41:44PM +0200, [email protected] wrote:
> From: Martin Wilck <[email protected]>
> 
> 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 <[email protected]>
Reviewed-by: Benjamin Marzinski <[email protected]>
> Cc: Li Xiao Keng <[email protected]>
> Cc: Miao Guanqin <[email protected]>
> Cc: Guan Junxiong <[email protected]>
> ---
>  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


Reply via email to