* [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
@ 2020-10-12 16:09 Nikita Ermakov
2020-10-20 10:19 ` Vlastimil Babka
0 siblings, 1 reply; 9+ messages in thread
From: Nikita Ermakov @ 2020-10-12 16:09 UTC (permalink / raw)
To: linux-mm; +Cc: Andrew Morton, Nikita Ermakov
Exit from the loop over the VMA in the case when the flags
contain only an MS_ASYNC and start < vm_start. In this case msync()
would return with -ENOMEM anyway so make it return early.
Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
---
mm/msync.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/msync.c b/mm/msync.c
index 69c6d2029531..ed20c3621d4c 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
goto out_unlock;
/* Here start < vma->vm_end. */
if (start < vma->vm_start) {
+ if (flags == MS_ASYNC)
+ goto out_unlock;
start = vma->vm_start;
if (start >= end)
goto out_unlock;
base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
--
2.28.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-12 16:09 [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start Nikita Ermakov
@ 2020-10-20 10:19 ` Vlastimil Babka
2020-10-20 14:38 ` Nikita
0 siblings, 1 reply; 9+ messages in thread
From: Vlastimil Babka @ 2020-10-20 10:19 UTC (permalink / raw)
To: Nikita Ermakov, linux-mm; +Cc: Andrew Morton
On 10/12/20 6:09 PM, Nikita Ermakov wrote:
> Exit from the loop over the VMA in the case when the flags
> contain only an MS_ASYNC and start < vm_start. In this case msync()
> would return with -ENOMEM anyway so make it return early.
>
> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
AFAICS it can still return -EBUSY if there's MS_INVALIDATE and a mlocked vma.
This is all subtle and I don't think we should risk breaking something for this
optimization.
> ---
> mm/msync.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/msync.c b/mm/msync.c
> index 69c6d2029531..ed20c3621d4c 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
> goto out_unlock;
> /* Here start < vma->vm_end. */
> if (start < vma->vm_start) {
> + if (flags == MS_ASYNC)
> + goto out_unlock;
> start = vma->vm_start;
> if (start >= end)
> goto out_unlock;
>
> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-20 10:19 ` Vlastimil Babka
@ 2020-10-20 14:38 ` Nikita
2020-10-20 15:03 ` Vlastimil Babka
0 siblings, 1 reply; 9+ messages in thread
From: Nikita @ 2020-10-20 14:38 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm; +Cc: Andrew Morton
Hi!
> On 20 Oct 2020, at 13:19, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 10/12/20 6:09 PM, Nikita Ermakov wrote:
>> Exit from the loop over the VMA in the case when the flags
>> contain only an MS_ASYNC and start < vm_start. In this case msync()
>> would return with -ENOMEM anyway so make it return early.
>> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
>
> AFAICS it can still return -EBUSY if there's MS_INVALIDATE and a mlocked vma. This is all subtle and I don't think we should risk breaking something for this optimization.
>
Yes, it could. But in this patch the optimization works only in the case if the flag is MS_ASYNC. If the flag is (MS_ASYNC | MS_INVALIDATE).
>> ---
>> mm/msync.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/mm/msync.c b/mm/msync.c
>> index 69c6d2029531..ed20c3621d4c 100644
>> --- a/mm/msync.c
>> +++ b/mm/msync.c
>> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>> goto out_unlock;
>> /* Here start < vma->vm_end. */
>> if (start < vma->vm_start) {
>> + if (flags == MS_ASYNC)
>> + goto out_unlock;
>> start = vma->vm_start;
>> if (start >= end)
>> goto out_unlock;
>> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-20 14:38 ` Nikita
@ 2020-10-20 15:03 ` Vlastimil Babka
2020-10-20 16:19 ` Nikita
0 siblings, 1 reply; 9+ messages in thread
From: Vlastimil Babka @ 2020-10-20 15:03 UTC (permalink / raw)
To: Nikita, linux-mm; +Cc: Andrew Morton
On 10/20/20 4:38 PM, Nikita wrote:
> Hi!
>
>> On 20 Oct 2020, at 13:19, Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 10/12/20 6:09 PM, Nikita Ermakov wrote:
>>> Exit from the loop over the VMA in the case when the flags
>>> contain only an MS_ASYNC and start < vm_start. In this case msync()
>>> would return with -ENOMEM anyway so make it return early.
>>> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
>>
>> AFAICS it can still return -EBUSY if there's MS_INVALIDATE and a mlocked vma. This is all subtle and I don't think we should risk breaking something for this optimization.
>>
> Yes, it could. But in this patch the optimization works only in the case if the flag is MS_ASYNC. If the flag is (MS_ASYNC | MS_INVALIDATE).
Ah, right, it compares "== MS_ASYNC" not "& MS_ASYNC" so it's correct. A comment
would be nice, such as:
We found an unmapped range and with MS_ASYNC and no MS_INVALIDATE there's
nothing to do and the result will always be -ENOMEM, so we can return immediately.
>>> ---
>>> mm/msync.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>> diff --git a/mm/msync.c b/mm/msync.c
>>> index 69c6d2029531..ed20c3621d4c 100644
>>> --- a/mm/msync.c
>>> +++ b/mm/msync.c
>>> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>>> goto out_unlock;
>>> /* Here start < vma->vm_end. */
>>> if (start < vma->vm_start) {
>>> + if (flags == MS_ASYNC)
>>> + goto out_unlock;
>>> start = vma->vm_start;
>>> if (start >= end)
>>> goto out_unlock;
>>> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-20 15:03 ` Vlastimil Babka
@ 2020-10-20 16:19 ` Nikita
2020-10-20 20:56 ` [PATCH v2] " Nikita Ermakov
0 siblings, 1 reply; 9+ messages in thread
From: Nikita @ 2020-10-20 16:19 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm; +Cc: Andrew Morton
> On 20 Oct 2020, at 18:03, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 10/20/20 4:38 PM, Nikita wrote:
>> Hi!
>>> On 20 Oct 2020, at 13:19, Vlastimil Babka <vbabka@suse.cz> wrote:
>>> On 10/12/20 6:09 PM, Nikita Ermakov wrote:
>>>> Exit from the loop over the VMA in the case when the flags
>>>> contain only an MS_ASYNC and start < vm_start. In this case msync()
>>>> would return with -ENOMEM anyway so make it return early.
>>>> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
>>> AFAICS it can still return -EBUSY if there's MS_INVALIDATE and a mlocked vma. This is all subtle and I don't think we should risk breaking something for this optimization.
>> Yes, it could. But in this patch the optimization works only in the case if the flag is MS_ASYNC. If the flag is (MS_ASYNC | MS_INVALIDATE).
>
> Ah, right, it compares "== MS_ASYNC" not "& MS_ASYNC" so it's correct. A comment would be nice, such as:
>
> We found an unmapped range and with MS_ASYNC and no MS_INVALIDATE there's nothing to do and the result will always be -ENOMEM, so we can return immediately.
>
Thanks for the comment!
I should probably implement this comment to the patch description and resubmit it as v2.
>>>> ---
>>>> mm/msync.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>> diff --git a/mm/msync.c b/mm/msync.c
>>>> index 69c6d2029531..ed20c3621d4c 100644
>>>> --- a/mm/msync.c
>>>> +++ b/mm/msync.c
>>>> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>>>> goto out_unlock;
>>>> /* Here start < vma->vm_end. */
>>>> if (start < vma->vm_start) {
>>>> + if (flags == MS_ASYNC)
>>>> + goto out_unlock;
>>>> start = vma->vm_start;
>>>> if (start >= end)
>>>> goto out_unlock;
>>>> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-20 16:19 ` Nikita
@ 2020-10-20 20:56 ` Nikita Ermakov
2020-10-23 11:12 ` Vlastimil Babka
0 siblings, 1 reply; 9+ messages in thread
From: Nikita Ermakov @ 2020-10-20 20:56 UTC (permalink / raw)
To: linux-mm; +Cc: Nikita Ermakov, Andrew Morton
If an unmapped region was found and the flag is MS_ASYNC (without
MS_INVALIDATE) there is nothing to do and the result would be always
-ENOMEM, so return immediately.
Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
---
mm/msync.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/msync.c b/mm/msync.c
index 69c6d2029531..ed20c3621d4c 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
goto out_unlock;
/* Here start < vma->vm_end. */
if (start < vma->vm_start) {
+ if (flags == MS_ASYNC)
+ goto out_unlock;
start = vma->vm_start;
if (start >= end)
goto out_unlock;
base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
--
2.28.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-20 20:56 ` [PATCH v2] " Nikita Ermakov
@ 2020-10-23 11:12 ` Vlastimil Babka
2020-10-25 9:29 ` [PATCH v3] " Nikita Ermakov
2020-10-25 9:38 ` [PATCH v2] " Nikita Ermakov
0 siblings, 2 replies; 9+ messages in thread
From: Vlastimil Babka @ 2020-10-23 11:12 UTC (permalink / raw)
To: Nikita Ermakov, linux-mm; +Cc: Andrew Morton
On 10/20/20 10:56 PM, Nikita Ermakov wrote:
> If an unmapped region was found and the flag is MS_ASYNC (without
> MS_INVALIDATE) there is nothing to do and the result would be always
> -ENOMEM, so return immediately.
>
> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
> ---
> mm/msync.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/msync.c b/mm/msync.c
> index 69c6d2029531..ed20c3621d4c 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
> goto out_unlock;
> /* Here start < vma->vm_end. */
> if (start < vma->vm_start) {
I hoped it would become a code comment.
> + if (flags == MS_ASYNC)
> + goto out_unlock;
> start = vma->vm_start;
> if (start >= end)
> goto out_unlock;
>
> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-23 11:12 ` Vlastimil Babka
@ 2020-10-25 9:29 ` Nikita Ermakov
2020-10-25 9:38 ` [PATCH v2] " Nikita Ermakov
1 sibling, 0 replies; 9+ messages in thread
From: Nikita Ermakov @ 2020-10-25 9:29 UTC (permalink / raw)
To: linux-mm; +Cc: Nikita Ermakov, Andrew Morton, Vlastimil Babka
If an unmapped region was found and the flag is MS_ASYNC (without
MS_INVALIDATE) there is nothing to do and the result would be always
-ENOMEM, so return immediately.
Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
---
mm/msync.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/mm/msync.c b/mm/msync.c
index 69c6d2029531..137d1c104f3e 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -55,7 +55,9 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
goto out;
/*
* If the interval [start,end) covers some unmapped address ranges,
- * just ignore them, but return -ENOMEM at the end.
+ * just ignore them, but return -ENOMEM at the end. Besides, if the
+ * flag is MS_ASYNC (w/o MS_INVALIDATE) the result would be -ENOMEM
+ * anyway and there is nothing left to do, so return immediately.
*/
mmap_read_lock(mm);
vma = find_vma(mm, start);
@@ -69,6 +71,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
goto out_unlock;
/* Here start < vma->vm_end. */
if (start < vma->vm_start) {
+ if (flags == MS_ASYNC)
+ goto out_unlock;
start = vma->vm_start;
if (start >= end)
goto out_unlock;
base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
--
2.28.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start
2020-10-23 11:12 ` Vlastimil Babka
2020-10-25 9:29 ` [PATCH v3] " Nikita Ermakov
@ 2020-10-25 9:38 ` Nikita Ermakov
1 sibling, 0 replies; 9+ messages in thread
From: Nikita Ermakov @ 2020-10-25 9:38 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: linux-mm, Andrew Morton
> On 23 Oct 2020, at 14:12, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 10/20/20 10:56 PM, Nikita Ermakov wrote:
>> If an unmapped region was found and the flag is MS_ASYNC (without
>> MS_INVALIDATE) there is nothing to do and the result would be always
>> -ENOMEM, so return immediately.
>> Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
>> ---
>> mm/msync.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/mm/msync.c b/mm/msync.c
>> index 69c6d2029531..ed20c3621d4c 100644
>> --- a/mm/msync.c
>> +++ b/mm/msync.c
>> @@ -69,6 +69,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>> goto out_unlock;
>> /* Here start < vma->vm_end. */
>> if (start < vma->vm_start) {
>
> I hoped it would become a code comment.
>
Ops, I'm sorry. I've implemented this suggestion in the v3.
>> + if (flags == MS_ASYNC)
>> + goto out_unlock;
>> start = vma->vm_start;
>> if (start >= end)
>> goto out_unlock;
>> base-commit: 6824a8a9b4861d7df7ee132a952bdf6f84a99cb8
--
Thanks,
Nikita
B8 00 4C CD 21
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-25 9:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 16:09 [PATCH] mm/msync: exit early when the flags is an MS_ASYNC and start < vm_start Nikita Ermakov
2020-10-20 10:19 ` Vlastimil Babka
2020-10-20 14:38 ` Nikita
2020-10-20 15:03 ` Vlastimil Babka
2020-10-20 16:19 ` Nikita
2020-10-20 20:56 ` [PATCH v2] " Nikita Ermakov
2020-10-23 11:12 ` Vlastimil Babka
2020-10-25 9:29 ` [PATCH v3] " Nikita Ermakov
2020-10-25 9:38 ` [PATCH v2] " Nikita Ermakov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.