Messages in this thread Patch in this message | | | From | Chaitanya Kulkarni <> | Subject | Re: [RFC] nvmet: Always remove processed AER elements from list | Date | Sun, 3 Nov 2019 18:55:14 +0000 |
| |
On 10/31/2019 07:51 AM, Christoph Hellwig wrote: > On Wed, Oct 30, 2019 at 04:24:18PM +0100, Daniel Wagner wrote: >> All async events are enqueued via nvmet_add_async_event() which >> updates the ctrl->async_event_cmds[] array and additionally an struct >> nvmet_async_event is added to the ctrl->async_events list. >> >> Under normal operations the nvmet_async_event_work() updates again the >> ctrl->async_event_cmds and removes the corresponding struct >> nvmet_async_event from the list again. Though nvmet_sq_destroy() could >> be called which calles nvmet_async_events_free() which only updates >> the ctrl->async_event_cmds[] array. >> >> Add a new function nvmet_async_events_process() which processes the >> async events and updates both array and the list. With this we avoid >> having two places where the array and list are modified. > > I don't think this patch is correct. We can have AEN commands pending > that aren't used - that is the host sent the command, but the target > did not have even event yet. That means the command sits in > async_event_cmds, but there is no entry in >async_events for it yet. > > That being said I think what we want is to do the loop in your new > nvmet_async_events_free first, but after that still call the loop in > the existing nvmet_async_events_free after that. That ensures we first > flush out everything in ->async_events, and then also return any > potential remaining entry. > Something like following on the top of this patch ? (compile tested only).
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index b1b9dc58c3b4..36a859082846 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -153,6 +153,18 @@ static void nvmet_async_events_process(struct nvmet_ctrl *ctrl, u16 status) mutex_unlock(&ctrl->lock); nvmet_req_complete(req, status); } + + while (1) { + mutex_lock(&ctrl->lock); + if (!ctrl->nr_async_event_cmds) { + mutex_unlock(&ctrl->lock); + return; + } + + req = ctrl->async_event_cmds[--ctrl->nr_async_event_cmds]; + mutex_unlock(&ctrl->lock); + nvmet_req_complete(req, NVME_SC_INTERNAL | NVME_SC_DNR); + } }
static void nvmet_async_events_free(struct nvmet_ctrl *ctrl)
| |