RE: [RFC PATCH] IB/core: Simplify locking around data_vec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux