[lkml]   [2019]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 5/6] powerpc/mmu: drop mmap_sem now that locked_vm is atomic
On Tue, 23 Apr 2019, Bueso wrote:

>On Wed, 03 Apr 2019, Daniel Jordan wrote:
>>On Wed, Apr 03, 2019 at 06:58:45AM +0200, Christophe Leroy wrote:
>>>Le 02/04/2019 à 22:41, Daniel Jordan a écrit :
>>>> With locked_vm now an atomic, there is no need to take mmap_sem as
>>>> writer. Delete and refactor accordingly.
>>>Could you please detail the change ?
>>Ok, I'll be more specific in the next version, using some of your language in
>>fact. :)
>>>It looks like this is not the only
>>>change. I'm wondering what the consequences are.
>>>Before we did:
>>>- lock
>>>- calculate future value
>>>- check the future value is acceptable
>>>- update value if future value acceptable
>>>- return error if future value non acceptable
>>>- unlock
>>>Now we do:
>>>- atomic update with future (possibly too high) value
>>>- check the new value is acceptable
>>>- atomic update back with older value if new value not acceptable and return
>>>So if a concurrent action wants to increase locked_vm with an acceptable
>>>step while another one has temporarily set it too high, it will now fail.
>>>I think we should keep the previous approach and do a cmpxchg after
>>>validating the new value.
>Wouldn't the cmpxchg alternative also be exposed the locked_vm changing between
>validating the new value and the cmpxchg() and we'd bogusly fail even when there
>is still just because the value changed (I'm assuming we don't hold any locks,
>otherwise all this is pointless).
> current_locked = atomic_read(&mm->locked_vm);
> new_locked = current_locked + npages;
> if (new_locked < lock_limit)
> if (cmpxchg(&mm->locked_vm, current_locked, new_locked) == current_locked)

Err, this being != of course.

 \ /
  Last update: 2019-04-24 04:33    [W:0.075 / U:1.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site