Re: [PATCH] infiniband: fix a subtle race condition

2018-06-18 Thread Cong Wang
On Fri, Jun 15, 2018 at 12:08 PM, Jason Gunthorpe wrote: > > The point is, I don't care about the imbalance report. OK, we are not on the same page. Sorry for wasting my time.

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-18 Thread Cong Wang
On Fri, Jun 15, 2018 at 12:08 PM, Jason Gunthorpe wrote: > > The point is, I don't care about the imbalance report. OK, we are not on the same page. Sorry for wasting my time.

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-15 Thread Jason Gunthorpe
On Fri, Jun 15, 2018 at 10:57:49AM -0700, Cong Wang wrote: > > No, it *is* the point - you've proposed a solution, one of many, and > > we need to see an actual sensible design for how the locking around > > ctx->file should work correctly. > > I proposed to a solution for imbalance unlock, you

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-15 Thread Jason Gunthorpe
On Fri, Jun 15, 2018 at 10:57:49AM -0700, Cong Wang wrote: > > No, it *is* the point - you've proposed a solution, one of many, and > > we need to see an actual sensible design for how the locking around > > ctx->file should work correctly. > > I proposed to a solution for imbalance unlock, you

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-15 Thread Cong Wang
On Thu, Jun 14, 2018 at 7:57 PM, Jason Gunthorpe wrote: > On Thu, Jun 14, 2018 at 04:14:13PM -0700, Cong Wang wrote: >> On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: >> > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: >> >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-15 Thread Cong Wang
On Thu, Jun 14, 2018 at 7:57 PM, Jason Gunthorpe wrote: > On Thu, Jun 14, 2018 at 04:14:13PM -0700, Cong Wang wrote: >> On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: >> > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: >> >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 04:14:13PM -0700, Cong Wang wrote: > On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: > > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: > >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > >> > > >> > This was my brief reaction too, this

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 04:14:13PM -0700, Cong Wang wrote: > On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: > > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: > >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > >> > > >> > This was my brief reaction too, this

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: >> > >> > This was my brief reaction too, this code path almost certainly has a >> > use-after-free, and we should fix

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 10:24 AM, Jason Gunthorpe wrote: > On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: >> On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: >> > >> > This was my brief reaction too, this code path almost certainly has a >> > use-after-free, and we should fix

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: > On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > > > > This was my brief reaction too, this code path almost certainly has a > > use-after-free, and we should fix the concurrency between the two > > places in some correct way..

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 10:03:09AM -0700, Cong Wang wrote: > On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > > > > This was my brief reaction too, this code path almost certainly has a > > use-after-free, and we should fix the concurrency between the two > > places in some correct way..

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > > This was my brief reaction too, this code path almost certainly has a > use-after-free, and we should fix the concurrency between the two > places in some correct way.. First of all, why use-after-free could trigger an imbalance unlock?

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 7:24 AM, Jason Gunthorpe wrote: > > This was my brief reaction too, this code path almost certainly has a > use-after-free, and we should fix the concurrency between the two > places in some correct way.. First of all, why use-after-free could trigger an imbalance unlock?

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 12:01 AM, Leon Romanovsky wrote: > On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: >> On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: >> > >> > Hi Cong, >> > >> > If the compiler optimizes the first line (mutex_lock) as you wrote, >> > it will reuse

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Thu, Jun 14, 2018 at 12:01 AM, Leon Romanovsky wrote: > On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: >> On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: >> > >> > Hi Cong, >> > >> > If the compiler optimizes the first line (mutex_lock) as you wrote, >> > it will reuse

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 10:01:08AM +0300, Leon Romanovsky wrote: > On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: > > On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > > > > > Hi Cong, > > > > > > If the compiler optimizes the first line (mutex_lock) as you wrote, > > > it

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Jason Gunthorpe
On Thu, Jun 14, 2018 at 10:01:08AM +0300, Leon Romanovsky wrote: > On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: > > On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > > > > > Hi Cong, > > > > > > If the compiler optimizes the first line (mutex_lock) as you wrote, > > > it

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Leon Romanovsky
On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: > On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > > > Hi Cong, > > > > If the compiler optimizes the first line (mutex_lock) as you wrote, > > it will reuse "f" for the second line (mutex_unlock) too. > > Nope, check the

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Leon Romanovsky
On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: > On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > > > Hi Cong, > > > > If the compiler optimizes the first line (mutex_lock) as you wrote, > > it will reuse "f" for the second line (mutex_unlock) too. > > Nope, check the

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > Hi Cong, > > If the compiler optimizes the first line (mutex_lock) as you wrote, > it will reuse "f" for the second line (mutex_unlock) too. Nope, check the assembly if you don't trust me, at least my compiler always fetches ctx->file

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Cong Wang
On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky wrote: > > Hi Cong, > > If the compiler optimizes the first line (mutex_lock) as you wrote, > it will reuse "f" for the second line (mutex_unlock) too. Nope, check the assembly if you don't trust me, at least my compiler always fetches ctx->file

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Leon Romanovsky
On Wed, Jun 13, 2018 at 04:49:47PM -0700, Cong Wang wrote: > In ucma_event_handler() we lock the mutex like this: > > mutex_lock(>file->mut); > ... > mutex_unlock(>file->mut); > > which seems correct, but we could translate it into this: > > f = ctx->file; > mutex_lock(>mut); > ... > f =

Re: [PATCH] infiniband: fix a subtle race condition

2018-06-14 Thread Leon Romanovsky
On Wed, Jun 13, 2018 at 04:49:47PM -0700, Cong Wang wrote: > In ucma_event_handler() we lock the mutex like this: > > mutex_lock(>file->mut); > ... > mutex_unlock(>file->mut); > > which seems correct, but we could translate it into this: > > f = ctx->file; > mutex_lock(>mut); > ... > f =