On 3/26/18 6:00 PM, Tetsuo Handa wrote:
Cyrill Gorcunov wrote:
On Tue, Mar 27, 2018 at 06:10:09AM +0900, Tetsuo Handa wrote:
On 2018/03/27 4:21, Cyrill Gorcunov wrote:
That said I think using read-lock here would be a bug.
If I understand correctly, the caller can't set both fields atomically, for
prctl() does not receive both fields at one call.
prctl(PR_SET_MM, PR_SET_MM_ARG_START xor PR_SET_MM_ARG_END xor PR_SET_MM_ENV_START xor PR_SET_MM_ENV_END, new value, 0, 0);
True, but the key moment is that two/three/four system calls can
run simultaneously. And while previously they are ordered by "write",
with read lock they are completely unordered and this is really
worries me.
Yes, we need exclusive lock when updating these fields.
To be fair I would prefer to drop this old per-field
interface completely. This per-field interface was rather an ugly
solution from my side.
But this is userspace visible API and thus we cannot change.
Then, I wonder whether reading arg_start|end and env_start|end atomically makes
sense. Just retry reading if arg_start > env_end or env_start > env_end is fine?
Tetsuo, let me re-read this code tomorrow, maybe I miss something obvious.
You are not missing my point. What I thought is
+retry:
- down_read(&mm->mmap_sem);
arg_start = mm->arg_start;
arg_end = mm->arg_end;
env_start = mm->env_start;
env_end = mm->env_end;
- up_read(&mm->mmap_sem);
- BUG_ON(arg_start > arg_end);
- BUG_ON(env_start > env_end);
+ if (unlikely(arg_start > arg_end || env_start > env_end)) {
+ cond_resched();
+ goto retry;
Can't it trap into dead loop if the condition is always false?
+ }
for reading these fields.
By the way, /proc/pid/ readers are serving as a canary who tells something
mm_mmap related problem is happening. On the other hand, it is sad that
such canary cannot be terminated by signal due to use of unkillable waits.
I wish we can use killable waits.
I already proposed patches (https://lkml.org/lkml/2018/2/26/1197) to do
this a few weeks ago. In the review, akpm suggested mitigate the
mmap_sem contention instead of using killable version workaround. Then
the preliminary unmaping by section patches
(https://lkml.org/lkml/2018/3/20/786) were proposed. In the discussion,
we decided to eliminate the mmap_sem abuse, this is where the patch came
from.
Yang
