* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).