Messages in this thread | | | Date | Tue, 23 Apr 2019 19:31:52 -0700 | From | Davidlohr Bueso <> | Subject | Re: [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 >>>error >>> >>>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.
| |