Let IB core distribute cache update events

-----Original Message-----
From: Leon Romanovsky <[email protected]>
Sent: Tuesday, October 29, 2019 6:53 AM
To: Doug Ledford <[email protected]>; Jason Gunthorpe <[email protected]>
Cc: Leon Romanovsky <[email protected]>; RDMA mailing list <linux- [email protected]>; Daniel Jurgens <[email protected]>; Parav Pandit <[email protected]>
Subject: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

From: Parav Pandit <[email protected]>

Currently when low level driver notifies Pkey, GID, port change events, they are notified to the registered handlers in the order they are registered. IB core and other ULPs such as IPoIB are interested in GID, LID, Pkey change events.
Since all GID query done by ULPs is serviced by IB core, in below flow when GID change event occurs, IB core is yet to update the GID cache when IPoIB queries the GID, resulting into not updating IPoIB address.

mlx5_ib_handle_event()
ib_dispatch_event()

    ib_cache_event()
       queue_work() -> slow cache update

    [..]
    ipoib_event()
     queue_work()
       [..]
       work handler
         ipoib_ib_dev_flush_light()
           __ipoib_ib_dev_flush()
              ipoib_dev_addr_changed_valid()
                rdma_query_gid() <- Returns old GID, cache not updated.

Hence, all events which require cache update are handled first by the IB core. Once cache update work is completed, IB core distributes the event to subscriber clients.

Fixes f35faa4ba956 ("IB/core: Simplify ib_query_gid to always refer to

cache")

Signed-off-by Parav Pandit <[email protected]>

Reviewed-by Daniel Jurgens <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/cache.c     | 84 ++++++++++++++---------------
 drivers/infiniband/core/core_priv.h |  2 +
 drivers/infiniband/core/device.c    | 27 ++++++----
 include/rdma/ib_verbs.h             |  1 -
 4 files changed, 57 insertions(+), 57 deletions(-)

drivers/infiniband/core/cache.c +3 -4

index d535995711c3..1d5c0df7dfab 100644
@@ -51,9 +51,8 @@ struct ib_pkey_cache {
 
 struct ib_update_work {
 	struct work_struct work;
-	struct ib_device  *device;
-	u8                 port_num;
-	bool		   enforce_security;
+	struct ib_event event;
+	bool enforce_security;
 };
 
 union ib_gid zgid;

@@ -130,7 +129,7 @@ static void dispatch_gid_change_event(struct
 ib_device *ib_dev, u8 port)
 	event.element.port_num	= port;
 	event.event		= IB_EVENT_GID_CHANGE;
 
-	ib_dispatch_event(&event);
+	ib_dispatch_cache_event_clients(&event);
 }
 

static const char * const gid_type_str[] = { @@ -1381,9 +1380,8 @@ static
int config_non_roce_gid_cache(struct ib_device *device,

drivers/infiniband/core/cache.c +2 -3

 	return ret;
 }
 
-static void ib_cache_update(struct ib_device *device,
-			    u8                port,
-			    bool	      enforce_security)
+static int
+ib_cache_update(struct ib_device *device, u8 port, bool

enforce_security)
{

drivers/infiniband/core/cache.c +2 -2

 	struct ib_port_attr       *tprops = NULL;
 	struct ib_pkey_cache      *pkey_cache = NULL, *old_pkey_cache;

@@ -1391,11 +1389,11 @@ static void ib_cache_update(struct ib_device
 *device,
 	int                        ret;
 
 	if (!rdma_is_port_valid(device, port))
-		return;
+		return -EINVAL;
 
 	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
 	if (!tprops)
-		return;
+		return -ENOMEM;
 
 	ret = ib_query_port(device, port, tprops);

drivers/infiniband/core/cache.c +28 -27

 	if (ret) {

@@ -1413,8 +1411,10 @@ static void ib_cache_update(struct ib_device
 *device,
 	pkey_cache = kmalloc(struct_size(pkey_cache, table,
 					 tprops->pkey_tbl_len),
 			     GFP_KERNEL);
-	if (!pkey_cache)
+	if (!pkey_cache) {
+		ret = -ENOMEM;
 		goto err;
+	}
 
 	pkey_cache->table_len = tprops->pkey_tbl_len;

@@ -1446,49 +1446,48 @@ static void ib_cache_update(struct ib_device
 *device,
 
 	kfree(old_pkey_cache);
 	kfree(tprops);
-	return;
+	return 0;
 
 err:
 	kfree(pkey_cache);
 	kfree(tprops);
+	return ret;
 }
 
 static void ib_cache_task(struct work_struct *_work)  {
 	struct ib_update_work *work =
 		container_of(_work, struct ib_update_work, work);
+	int ret;
+
+	ret = ib_cache_update(work->event.device, work-
 >event.element.port_num,
+			      work->enforce_security);
+
+	/* GID event is notified already for individual GID entries by
+	 * dispatch_gid_change_event(). Hence, notifiy for rest of the
+	 * events.
+	 */
+	if (!ret && work->event.event != IB_EVENT_GID_CHANGE)
+		ib_dispatch_cache_event_clients(&work->event);
 
-	ib_cache_update(work->device,
-			work->port_num,
-			work->enforce_security);
 	kfree(work);
 }
 
-static void ib_cache_event(struct ib_event_handler *handler,
-			   struct ib_event *event)
+void ib_enqueue_cache_update_event(const struct ib_event *event)
 {
 	struct ib_update_work *work;
 
-	if (event->event == IB_EVENT_PORT_ERR    ||
-	    event->event == IB_EVENT_PORT_ACTIVE ||
-	    event->event == IB_EVENT_LID_CHANGE  ||
-	    event->event == IB_EVENT_PKEY_CHANGE ||
-	    event->event == IB_EVENT_CLIENT_REREGISTER ||
-	    event->event == IB_EVENT_GID_CHANGE) {
-		work = kmalloc(sizeof *work, GFP_ATOMIC);
-		if (work) {
-			INIT_WORK(&work->work, ib_cache_task);
-			work->device   = event->device;
-			work->port_num = event->element.port_num;
-			if (event->event == IB_EVENT_PKEY_CHANGE ||
-			    event->event == IB_EVENT_GID_CHANGE)
-				work->enforce_security = true;
-			else
-				work->enforce_security = false;
-
-			queue_work(ib_wq, &work->work);
-		}
-	}
+	work = kzalloc(sizeof(*work), GFP_ATOMIC);
+	if (!work)
+		return;
+
+	INIT_WORK(&work->work, ib_cache_task);
+	work->event = *event;
+	if (event->event == IB_EVENT_PKEY_CHANGE ||
+	    event->event == IB_EVENT_GID_CHANGE)
+		work->enforce_security = true;
+
+	queue_work(ib_wq, &work->work);
 }
 

int ib_cache_setup_one(struct ib_device *device) @@ -1505,9 +1504,6 @@
int ib_cache_setup_one(struct ib_device *device)

drivers/infiniband/core/cache.c +4 -9

 	rdma_for_each_port (device, p)
 		ib_cache_update(device, p, true);
 
-	INIT_IB_EVENT_HANDLER(&device->cache.event_handler,
-			      device, ib_cache_event);
-	ib_register_event_handler(&device->cache.event_handler);
 	return 0;
 }

@@ -1529,14 +1525,12 @@ void ib_cache_release_one(struct ib_device
 *device)
 
 void ib_cache_cleanup_one(struct ib_device *device)  {
-	/* The cleanup function unregisters the event handler,
-	 * waits for all in-progress workqueue elements and cleans
-	 * up the GID cache. This function should be called after
-	 * the device was removed from the devices list and all
-	 * clients were removed, so the cache exists but is
+	/* The cleanup function waits for all in-progress workqueue
+	 * elements and cleans up the GID cache. This function should be
+	 * called after the device was removed from the devices list and
+	 * all clients were removed, so the cache exists but is
 	 * non-functional and shouldn't be updated anymore.
 	 */
-	ib_unregister_event_handler(&device->cache.event_handler);
 	flush_workqueue(ib_wq);
 	gid_table_cleanup_one(device);
 

drivers/infiniband/core/core_priv.h +0 -0

index 3a8b0911c3bc..137c98098489 100644
@@ -149,6 +149,8 @@ unsigned long roce_gid_type_mask_support(struct

ib_device *ib_dev, u8 port); int ib_cache_setup_one(struct ib_device *device); void ib_cache_cleanup_one(struct ib_device *device); void ib_cache_release_one(struct ib_device *device);

drivers/infiniband/core/core_priv.h +2 -0

+void ib_enqueue_cache_update_event(const struct ib_event *event); void
+ib_dispatch_cache_event_clients(struct ib_event *event);
 
 #ifdef CONFIG_CGROUP_RDMA
 void ib_device_register_rdmacg(struct ib_device *device); diff --git

a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index f8d383ceae05..9db229c628e9 100644

drivers/infiniband/core/device.c +15 -11

@@ -1931,8 +1931,8 @@ EXPORT_SYMBOL(ib_set_client_data);
  *
  * ib_register_event_handler() registers an event handler that will be
  * called back when asynchronous IB events occur (as defined in
- * chapter 11 of the InfiniBand Architecture Specification).  This
- * callback may occur in interrupt context.
+ * chapter 11 of the InfiniBand Architecture Specification). This
+ * callback always occurring in workqueue context.
  */
 void ib_register_event_handler(struct ib_event_handler *event_handler)  {

@@ -1962,15 +1962,7 @@ void ib_unregister_event_handler(struct
 ib_event_handler *event_handler)  }
 EXPORT_SYMBOL(ib_unregister_event_handler);
 
-/**
- * ib_dispatch_event - Dispatch an asynchronous event
- * @event:Event to dispatch
- *
- * Low-level drivers must call ib_dispatch_event() to dispatch the
- * event to all registered event handlers when an asynchronous event
- * occurs.
- */
-void ib_dispatch_event(struct ib_event *event)
+void ib_dispatch_cache_event_clients(struct ib_event *event)
 {
 	unsigned long flags;
 	struct ib_event_handler *handler;

@@ -1982,6 +1974,19 @@ void ib_dispatch_event(struct ib_event *event)
 
 	spin_unlock_irqrestore(&event->device->event_handler_lock, flags);
 }
+
+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to dispatch the
+ * event to all registered event handlers when an asynchronous event
+ * occurs.
+ */
+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);
 
 static int iw_query_port(struct ib_device *device,

include/rdma/ib_verbs.h +0 -2

index 0626b62ed107..6604115f7c85 100644
@@ -2148,7 +2148,6 @@ struct ib_port_cache {
 
 struct ib_cache {
 	rwlock_t                lock;
-	struct ib_event_handler event_handler;
 };
 
 struct ib_port_immutable {
--

2.20.1

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:24 AM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>; Parav Pandit <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

drivers/infiniband/core/cache.c static int config_non_roce_gid_cache(struct ib_device *device,

Quoted 5 lines · Leon Romanovsky

drivers/infiniband/core/cache.c +2 -3

-static void ib_cache_update(struct ib_device *device,
-			    u8                port,
-			    bool	      enforce_security)
+static int
+ib_cache_update(struct ib_device *device, u8 port, bool

enforce_security)

Quoted 1 line

{

Formatting

Quoted 6 lines

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to dispatch the
+ * event to all registered event handlers when an asynchronous event

drivers/infiniband/core/device.c void ib_dispatch_event(struct ib_event *event) · line 1990

Quoted 6 lines

+ * occurs.
+ */

drivers/infiniband/core/device.c +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Jason

Quoted 6 lines

Quoted 6 lines

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to dispatch the
+ * event to all registered event handlers when an asynchronous event

Quoted 9 lines

drivers/infiniband/core/device.c void ib_dispatch_event(struct ib_event *event) · line 1990

Quoted 6 lines

+ * occurs.
+ */

drivers/infiniband/core/device.c +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Same thought same to me when I had to add one liner call.
However the issue was device.c has the code for the event registration/unregistration and calling the handlers unrelated to cache. So moving ib_dispatch_event() to cache.c looked incorrect to me.

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:34 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 15 lines · Parav Pandit

Quoted 15 lines

drivers/infiniband/core/device.c void ib_dispatch_event(struct ib_event *event) · line 1990

Quoted 12 lines

drivers/infiniband/core/device.c +11 -0

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to dispatch the
+ * event to all registered event handlers when an asynchronous event
+ * occurs.
+ */
+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Quoted 2 lines

Same thought same to me when I had to add one liner call. However the issue was device.c has the code for the event

registration/unregistration and calling the handlers unrelated to cache.

Quoted 1 line

So moving ib_dispatch_event() to cache.c looked incorrect to me.

Well, maybe we can move the wq code from the cache.c into here?

It looks just as incorrect to have the one line call

Jason

Quoted 31 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:34 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 17 lines · Parav Pandit

Quoted 6 lines

Quoted 6 lines

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to dispatch the
+ * event to all registered event handlers when an asynchronous event

Quoted 9 lines

drivers/infiniband/core/device.c void ib_dispatch_event(struct ib_event *event) · line 1990

Quoted 6 lines

+ * occurs.
+ */

drivers/infiniband/core/device.c +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Same thought same to me when I had to add one liner call. However the issue was device.c has the code for the event

Quoted 4 lines

registration/unregistration and calling the handlers unrelated to cache.

Quoted 1 line

So moving ib_dispatch_event() to cache.c looked incorrect to me.

Well, maybe we can move the wq code from the cache.c into here?

I prefer to keep all cache specific code in cache.c because there is where we flush the ib_wq, queue work there.

Quoted 1 line

It looks just as incorrect to have the one line call

No, its not incorrect. Because device.c provides functionality to register/unregister handler and dispatch event. While cache layer deals with cache updates etc, and uses wq service provided device.c If we rename ib_dispatch_event() to ib_dispatch_cache_event() it make sense to move to cache.c However we have non cache specific events there too, so current code split between cache.c and device.c looks appropriate.

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:55 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 7 lines · Parav Pandit

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:34 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>;

RDMA

Quoted 29 lines

Quoted 29 lines

mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 17 lines · Parav Pandit

Quoted 8 lines

Quoted 8 lines

(quoted diff) +8 -0

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to
+dispatch the
+ * event to all registered event handlers when an
+asynchronous event

Quoted 9 lines

Review

Quoted 6 lines

+ * occurs.
+ */

(quoted diff) +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Quoted 2 lines

Same thought same to me when I had to add one liner call. However the issue was device.c has the code for the event

registration/unregistration and calling the handlers unrelated to cache.

Quoted 1 line

So moving ib_dispatch_event() to cache.c looked incorrect to me.

Quoted 4 lines

Quoted 1 line

Well, maybe we can move the wq code from the cache.c into here?

I prefer to keep all cache specific code in cache.c because there is where we flush the ib_wq, queue work there.

I think that is because the cache is not subscribing to a notifier, it really should, then things order properly and the flush is not needed.

Quoted 9 lines

Quoted 1 line

It looks just as incorrect to have the one line call

No, its not incorrect. Because device.c provides functionality to register/unregister handler and dispatch event. While cache layer deals with cache updates etc, and uses wq service provided device.c If we rename ib_dispatch_event() to ib_dispatch_cache_event() it make sense to move to cache.c However we have non cache specific events there too, so current code split between cache.c and device.c looks appropriate.

It always looks bad to have a single line function call like that, especially just for spurious reasons like file placement or functioning naming. It shows something is being done wrong

Quoted 52 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:55 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 7 lines · Parav Pandit

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:34 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>;

RDMA

Quoted 31 lines

Quoted 30 lines

mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 17 lines · Parav Pandit

Quoted 8 lines

Quoted 8 lines

drivers/infiniband/core/cache.c +8 -0

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to
+dispatch the
+ * event to all registered event handlers when an
+asynchronous event

Quoted 9 lines

Review

Quoted 6 lines

+ * occurs.
+ */

(quoted diff) +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Quoted 2 lines

Same thought same to me when I had to add one liner call. However the issue was device.c has the code for the event

registration/unregistration and calling the handlers unrelated to cache.

Quoted 1 line

So moving ib_dispatch_event() to cache.c looked incorrect to me.

Well, maybe we can move the wq code from the cache.c into here?

Quoted 6 lines

Quoted 2 lines

I prefer to keep all cache specific code in cache.c because there is where we flush the ib_wq, queue work there.

I think that is because the cache is not subscribing to a notifier, it really should, then things order properly and the flush is not needed.

Cache shouldn't subscribe to the notifier, that is the race condition issue.

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 1:26 PM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 7 lines · Parav Pandit

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:55 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>;

RDMA

Quoted 51 lines

Quoted 51 lines

mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 7 lines · Parav Pandit

Quoted 7 lines

From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, October 29, 2019 10:34 AM
To: Parav Pandit <[email protected]>
Cc: Leon Romanovsky <[email protected]>; Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>;

RDMA

Quoted 26 lines

Quoted 26 lines

mailing list <[email protected]>; Daniel Jurgens <[email protected]>
Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache update events

Quoted 19 lines · Parav Pandit

Quoted 8 lines

Quoted 8 lines

drivers/infiniband/core/cache.c +8 -0

+/**
+ * ib_dispatch_event - Dispatch an asynchronous event
+ * @event:Event to dispatch
+ *
+ * Low-level drivers must call ib_dispatch_event() to
+dispatch the
+ * event to all registered event handlers when an
+asynchronous event

Quoted 9 lines

Review

Quoted 6 lines

+ * occurs.
+ */

(quoted diff) +3 -0

+void ib_dispatch_event(struct ib_event *event) {
+	ib_enqueue_cache_update_event(event);
+}
 EXPORT_SYMBOL(ib_dispatch_event);

Why not just move this into cache.c?

Same thought same to me when I had to add one liner call. However the issue was device.c has the code for the event

Quoted 7 lines

Quoted 4 lines

registration/unregistration and calling the handlers unrelated to cache.

Quoted 1 line

So moving ib_dispatch_event() to cache.c looked incorrect to me.

Well, maybe we can move the wq code from the cache.c into here?

I prefer to keep all cache specific code in cache.c because there is where we flush the ib_wq, queue work there.

I think that is because the cache is not subscribing to a notifier, it really should, then things order properly and the flush is not needed.

Quoted 2 lines

Cache shouldn't subscribe to the notifier, that is the race condition issue.

Notifiers are ordered, as long as the cache subscribes first to the notifier list it is not a 'race condition'.

This is still somewhat problematic because 'ib_dispatch_event' still calls the handlers under a spinlock (ie the handlers are still non-blocking contexts)

I'm suggesting to swap it for a normal blocking notifier chain and guarantee

the cache is the first entry in the chain.

Jason