On Mon, Feb 01, 2021 at 05:06:58PM -0800, Dan Williams wrote: > On Mon, Feb 1, 2021 at 4:40 PM Saleem, Shiraz <shiraz.saleem@xxxxxxxxx> wrote: > > > > > Subject: Re: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and > > > implement private channel OPs > > > > > > On Sat, Jan 30, 2021 at 01:19:36AM +0000, Saleem, Shiraz wrote: > > > > > Subject: Re: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver > > > > > and implement private channel OPs > > > > > > > > > > On Wed, Jan 27, 2021 at 07:16:41PM -0400, Jason Gunthorpe wrote: > > > > > > On Wed, Jan 27, 2021 at 10:17:56PM +0000, Saleem, Shiraz wrote: > > > > > > > > > > > > > Even with another core PCI driver, there still needs to be > > > > > > > private communication channel between the aux rdma driver and > > > > > > > this PCI driver to pass things like QoS updates. > > > > > > > > > > > > Data pushed from the core driver to its aux drivers should either > > > > > > be done through new callbacks in a struct device_driver or by > > > > > > having a notifier chain scheme from the core driver. > > > > > > > > > > Right, and internal to driver/core device_lock will protect from > > > > > parallel probe/remove and PCI flows. > > > > > > > > > > > > > OK. We will hold the device_lock while issuing the .ops callbacks from core > > > driver. > > > > This should solve our synchronization issue. > > > > > > > > There have been a few discussions in this thread. And I would like to > > > > be clear on what to do. > > > > > > > > So we will, > > > > > > > > 1. Remove .open/.close, .peer_register/.peer_unregister 2. Protect ops > > > > callbacks issued from core driver to the aux driver with device_lock > > > > > > A notifier chain is probably better, honestly. > > > > > > Especially since you don't want to split the netdev side, a notifier chain can be > > > used by both cases equally. > > > > > > > The device_lock seems to be a simple solution to this synchronization problem. > > May I ask what makes the notifier scheme better to solve this? > > > > Only loosely following the arguments here, but one of the requirements > of the driver-op scheme is that the notifying agent needs to know the > target device. With the notifier-chain approach the target device > becomes anonymous to the notifier agent. Yes, and you need to have an aux device in the first place. The netdev side has neither of this things. I think it would be a bit odd to have extensive callbacks that are for RDMA only, that suggests something in the core API is not general enough. Jason
