Hi Jason, > -----Original Message----- > From: Jason Gunthorpe > Sent: Thursday, March 29, 2018 4:44 PM > To: linux-rdma@xxxxxxxxxxxxxxx > Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; Mark Bloch > <markb@xxxxxxxxxxxx>; Parav Pandit <parav@xxxxxxxxxxxx> > Subject: [RFC PATCH] IB/core: Simplify locking around data_vec > > Instead o using a complex scheme where we drop and re-acquire rwlock, switch > to a much simpler dual locking pattern where the sleepable lock and the spinlock > are held by all writers, while all readers only grab the spinlock. > > GID_TABLE_ENTRY_INVALID is used as the fence protected by that lock. > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > --- > drivers/infiniband/core/cache.c | 73 ++++++++++++++--------------------------- > 1 file changed, 24 insertions(+), 49 deletions(-) > > This is based off Parav's latest series. > > What do you think Parav? Can you give it some test if you like it? > > I only added two new mutex(table->locks)'s: > - cleanup_gid_table_port(). This is only called during add/remove one > device which is a sleepable context Yes. This was in the TODO list, after this series. Thiw as good enough for the feature to progress. > - ib_cache_update() This is only called from a work queue, so > it should be fine No. this is not fine. Mutex lo replacement is done under holding silly pkey cache lock. write_lock_irq(&device->cache.lock); [..] mutex_lock. I already have patch that clean up this three time check of roce port property in single function along with this lock. Yesterday night I queued to Dan for internal review. > > Couldn't find any reason not to structure the locking like this. The comments > alluded to some difference with IB, but I'm thinking that is gone now, if it ever > even existed? > Yes. It appears fine. I have next series based on this series, so I prefer to merge this suggested changed in v1 along with ib_cache_update cleanup patch. I split into two patches because it was possible and this patch was big enough. > I like this because the locking is now easier to audit and much saner. We never > have to drop a lock to call the driver, everything is simply fully serialized. All > writers have to hold the mutex, very simple. Yes. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html