Messages in this thread Patch in this message | | | From | Alexander Shishkin <> | Subject | Re: [PATCH] perf/core: fix mlock accounting in perf_mmap() | Date | Mon, 20 Jan 2020 10:24:00 +0200 |
| |
Song Liu <songliubraving@fb.com> writes:
> sysctl_perf_event_mlock and user->locked_vm can change value > independently, so we can't guarantee: > > user->locked_vm <= user_lock_limit
This means: if the sysctl got sufficiently decreased, so that the existing locked_vm exceeds it, we need to deal with the overflow, right?
> diff --git a/kernel/events/core.c b/kernel/events/core.c > index a1f8bde19b56..89acdd1574ef 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -5920,11 +5920,31 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma) > > if (user_locked > user_lock_limit) { > /* > - * charge locked_vm until it hits user_lock_limit; > - * charge the rest from pinned_vm > + * sysctl_perf_event_mlock and user->locked_vm can change > + * value independently, so we can't guarantee: > + * > + * user->locked_vm <= user_lock_limit > + * > + * We need be careful to make sure user_extra >=0. > + * > + * Using "user_locked - user_extra" to avoid calling > + * atomic_long_read() again. > */ > - extra = user_locked - user_lock_limit; > - user_extra -= extra; > + if (user_locked - user_extra >= user_lock_limit) { > + /* > + * already used all user_locked_limit, charge all > + * to pinned_vm > + */ > + extra = user_extra; > + user_extra = 0; > + } else { > + /* > + * charge locked_vm until it hits user_lock_limit; > + * charge the rest from pinned_vm > + */ > + extra = user_locked - user_lock_limit; > + user_extra -= extra; > + }
How about the below for the sake of brevity?
diff --git a/kernel/events/core.c b/kernel/events/core.c index 763cf34b5a63..632505ce6c12 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5917,7 +5917,14 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma) */ user_lock_limit *= num_online_cpus(); - user_locked = atomic_long_read(&user->locked_vm) + user_extra; + user_locked = atomic_long_read(&user->locked_vm); + /* + * If perf_event_mlock has changed since earlier mmaps, so that + * it's smaller than user->locked_vm, discard the overflow. + */ + if (user_locked > user_lock_limit) + user_locked = user_lock_limit; + user_locked += user_extra; if (user_locked > user_lock_limit) { /* Regards, -- Alex
| |