lkml.org 
[lkml]   [2019]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/5] mm: return valid info from hmm_range_unregister
Date
On Wed, Jul 03, 2019 at 10:28:57PM +0200, Christoph Hellwig wrote:
> On Wed, Jul 03, 2019 at 07:00:50PM +0000, Jason Gunthorpe wrote:
> > I don't think the API should be encouraging some shortcut here..
> >
> > We can't do the above pattern because the old hmm_vma API didn't allow
> > it, which is presumably a reason why it is obsolete.
> >
> > I'd rather see drivers move to a consistent pattern so we can then
> > easily hoist the seqcount lock scheme into some common mmu notifier
> > code, as discussed.
>
> So you don't like the version in amdgpu_ttm_tt_get_user_pages_done in
> linux-next either?

I looked at this for 5 mins, and I can't see the key elements of the
collision retry lock:

- Where is the retry loop?
- Where is the lock around the final test to valid prior to using
the output of range?

For instance looking at amdgpu_gem_userptr_ioctl()..

We can't be holding a lock when we do hmm_range_wait_until_valid()
(inside amdgpu_ttm_tt_get_user_pages), otherwise it deadlocks, and
there are not other locks that would encompass the final is_valid check.

And amdgpu_gem_userptr_ioctl() looks like a syscall entry point, so
having it fail just because the lock collided (ie is_valid == false)
can't possibly be the right thing.

I'm also unclear when the device data is updated in that sequence..

So.. I think this locking is wrong. Maybe AMD team can explain how it
should work?

Jason

\
 
 \ /
  Last update: 2019-07-03 22:40    [W:0.049 / U:10.660 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site