Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: ISFXh6M9sd0sugAB594TitF5eVgs7M6AdJ+IJ6NFNd5ZIq/GG3u6Ezfp7FDLCp10+qwhojv7wb
 7avJQAY8hkttbd2R/UZID/lyZ72dqyKdP/3mZdwsJMZ57xN6bTLEb+RNIN9scdJ/Kw1rdpx0xw
 O0Z8HlX0uaZCNj8+n2RE9RErsTtp1p2RHgKgZstknVhuBFtZNMT5RyPgGG3SHDT2439R0P+cqL
 pieoLrp7L/id6y/YUUpdBJrIo+15bG0X3RXlF6jru4EJL9cDZIMwaKIU1tkynfOEE81beFN3WP
 N0o=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Christoph Hellwig <hch@lst.de>, Daniel Wagner <dwagner@suse.de>
CC:     Sagi Grimberg <sagi@grimberg.me>,
        Johannes Thumshirn <jthumshirn@suse.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [RFC] nvmet: Always remove processed AER elements from list
Thread-Topic: [RFC] nvmet: Always remove processed AER elements from list
Thread-Index: AQHVjzYhgmqnvQGm+02Suay/wXEq1Q==
Date:   Sun, 3 Nov 2019 18:55:14 +0000
Message-ID: <BYAPR04MB574907EE2666D6DA48DE30AD867C0@BYAPR04MB5749.namprd04.prod.outlook.com>
References: <20191030152418.23753-1-dwagner@suse.de>
 <20191031145127.GC6024@lst.de>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 95cb049a-9583-48d2-1954-08d7608f5ca4
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Nov 2019 18:55:14.4933
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: azcSfx1I/x7MwA5d11OHaxLHT0UCSQH0jXsyFQO5jTqSuxK0EJQFGRwTjK0gcyU5Umog3kn39fVM59eGUtu/jdCK8+0J/X6aOWwvIqfUbYQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4678
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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