From 98a38b6cc22aae7f5a39fd9965fef161e85bf158 Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Fri, 15 Sep 2023 06:07:43 -0500 Subject: [PATCH 1/2] CID1508860: double lock confusion --- iocore/aio/AIO.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/iocore/aio/AIO.cc b/iocore/aio/AIO.cc index 889002715d8..139e9a4cef7 100644 --- a/iocore/aio/AIO.cc +++ b/iocore/aio/AIO.cc @@ -423,9 +423,8 @@ ink_aio_thread_num_set(int thread_num) void * AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) { - AIO_Reqs *my_aio_req = thr_info->req; - AIO_Reqs *current_req = nullptr; - AIOCallback *op = nullptr; + AIO_Reqs *my_aio_req = thr_info->req; + AIOCallback *op = nullptr; ink_mutex_acquire(&my_aio_req->aio_mutex); for (;;) { do { @@ -433,7 +432,6 @@ AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) ink_mutex_release(&my_aio_req->aio_mutex); return nullptr; } - current_req = my_aio_req; /* check if any pending requests on the atomic list */ aio_move(my_aio_req); if (!(op = my_aio_req->aio_todo.pop())) { @@ -442,8 +440,10 @@ AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) #ifdef AIO_STATS num_requests--; current_req->queued--; - ink_atomic_increment((int *)¤t_req->pending, 1); + ink_atomic_increment((int *)&my_aio_req->pending, 1); #endif + ink_mutex_release(&my_aio_req->aio_mutex); + // update the stats; if (op->aiocb.aio_lio_opcode == LIO_WRITE) { Metrics::increment(aio_rsb.write_count); @@ -452,9 +452,8 @@ AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) Metrics::increment(aio_rsb.read_count); Metrics::increment(aio_rsb.kb_read, op->aiocb.aio_nbytes >> 10); } - ink_mutex_release(¤t_req->aio_mutex); cache_op((AIOCallbackInternal *)op); - ink_atomic_increment(¤t_req->requests_queued, -1); + ink_atomic_increment(&my_aio_req->requests_queued, -1); #ifdef AIO_STATS ink_atomic_increment((int *)¤t_req->pending, -1); #endif From 3bc51263dafb15cad444e54f064f4cb7e47caddb Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Tue, 19 Sep 2023 05:15:49 -0500 Subject: [PATCH 2/2] fix renames inside conditional code. Fix casts --- iocore/aio/AIO.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/iocore/aio/AIO.cc b/iocore/aio/AIO.cc index 139e9a4cef7..d08a979a3c3 100644 --- a/iocore/aio/AIO.cc +++ b/iocore/aio/AIO.cc @@ -77,8 +77,9 @@ AIOTestData::ink_aio_stats(int event, void *d) ink_hrtime now = ink_get_hrtime(); double time_msec = (double)(now - start) / (double)HRTIME_MSECOND; int i = (aio_reqs[0] == nullptr) ? 1 : 0; - for (; i < num_filedes; ++i) + for (; i < num_filedes; ++i) { printf("%0.2f\t%i\t%i\t%i\n", time_msec, aio_reqs[i]->filedes, aio_reqs[i]->pending, aio_reqs[i]->queued); + } printf("Num Requests: %i Num Queued: %i num Moved: %i\n\n", data->num_req, data->num_queue, data->num_temp); eventProcessor.schedule_in(this, HRTIME_MSECONDS(50), ET_CALL); return EVENT_DONE; @@ -306,7 +307,7 @@ aio_queue_req(AIOCallbackInternal *op, int fromAPI = 0) op->link.next = nullptr; op->link.prev = nullptr; #ifdef AIO_STATS - ink_atomic_increment((int *)&data->num_req, 1); + ink_atomic_increment(&data->num_req, 1); #endif if (!fromAPI && (!req || req->filedes != op->aiocb.aio_fildes)) { /* search for the matching file descriptor */ @@ -439,8 +440,8 @@ AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) } #ifdef AIO_STATS num_requests--; - current_req->queued--; - ink_atomic_increment((int *)&my_aio_req->pending, 1); + my_aio_req->queued--; + ink_atomic_increment(&my_aio_req->pending, 1); #endif ink_mutex_release(&my_aio_req->aio_mutex); @@ -452,10 +453,10 @@ AIOThreadInfo::aio_thread_main(AIOThreadInfo *thr_info) Metrics::increment(aio_rsb.read_count); Metrics::increment(aio_rsb.kb_read, op->aiocb.aio_nbytes >> 10); } - cache_op((AIOCallbackInternal *)op); + cache_op(reinterpret_cast(op)); ink_atomic_increment(&my_aio_req->requests_queued, -1); #ifdef AIO_STATS - ink_atomic_increment((int *)¤t_req->pending, -1); + ink_atomic_increment(&my_aio_req->pending, -1); #endif op->link.prev = nullptr; op->link.next = nullptr;