On Tue, Oct 29, 2019 at 06:11:01PM +0000, Parav Pandit wrote: > > > > From: Jason Gunthorpe <jgg@xxxxxxxx> > > Sent: Tuesday, October 29, 2019 10:55 AM > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens > > <danielj@xxxxxxxxxxxx> > > Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core distribute cache > > update events > > > > On Tue, Oct 29, 2019 at 03:47:42PM +0000, Parav Pandit wrote: > > > > > > > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > Sent: Tuesday, October 29, 2019 10:34 AM > > > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > > > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; > > RDMA > > > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens > > > > <danielj@xxxxxxxxxxxx> > > > > Subject: Re: [PATCH rdma-next v1 1/2] IB/core: Let IB core > > > > distribute cache update events > > > > > > > > On Tue, Oct 29, 2019 at 03:32:46PM +0000, Parav Pandit wrote: > > > > > > > > > > > +/** > > > > > > > + * 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? > > > > > > > > > > > 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. > > > > > > > > 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. > > > 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 guarentee the cache is the first entry in the chain. Jason