2022-05-28 19:02:27

by Kirill A. Shutemov

Subject: Re: [PATCH 2/2] [PATCH 2/2] mm: add merging after mremap resize

On Fri, May 27, 2022 at 12:48:10PM +0200, Jakub Matěna wrote:
> When mremap call results in expansion, it might be possible to merge the
> VMA with the next VMA which might become adjacent. This patch adds
> vma_merge call after the expansion is done to try and merge.
>
> Signed-off-by: Jakub Matěna <[email protected]>
> ---
> mm/mremap.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 303d3290b938..c41237e62156 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/mm.h>
> +#include <linux/mm_inline.h>
> #include <linux/hugetlb.h>
> #include <linux/shm.h>
> #include <linux/ksm.h>
> @@ -1022,8 +1023,11 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> }
> }
>
> - if (vma_adjust(vma, vma->vm_start, addr + new_len,
> - vma->vm_pgoff, NULL)) {
> + vma = vma_merge(mm, vma, addr + old_len, addr + new_len,
> + vma->vm_flags, vma->anon_vma, vma->vm_file,
> + vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
> + vma->vm_userfaultfd_ctx, anon_vma_name(vma));

The arguement list gets busy. Maybe some variables would help.
Calculation around vm_pgoff is not obvious and requires some explanation.

> + if (!vma) {
> vm_unacct_memory(pages);
> ret = -ENOMEM;
> goto out;
> --
> 2.35.1
>

--
Kirill A. Shutemov

2022-05-30 13:48:18

by Jakub Matěna

Subject: Re: [PATCH 2/2] [PATCH 2/2] mm: add merging after mremap resize

On Sat, May 28, 2022 at 1:45 AM Kirill A. Shutemov <[email protected]> wrote:
>
> On Fri, May 27, 2022 at 12:48:10PM +0200, Jakub Matěna wrote:
> > When mremap call results in expansion, it might be possible to merge the
> > VMA with the next VMA which might become adjacent. This patch adds
> > vma_merge call after the expansion is done to try and merge.
> >
> > Signed-off-by: Jakub Matěna <[email protected]>
> > ---
> > mm/mremap.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index 303d3290b938..c41237e62156 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -9,6 +9,7 @@
> > */
> >
> > #include <linux/mm.h>
> > +#include <linux/mm_inline.h>
> > #include <linux/hugetlb.h>
> > #include <linux/shm.h>
> > #include <linux/ksm.h>
> > @@ -1022,8 +1023,11 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> > }
> > }
> >
> > - if (vma_adjust(vma, vma->vm_start, addr + new_len,
> > - vma->vm_pgoff, NULL)) {
> > + vma = vma_merge(mm, vma, addr + old_len, addr + new_len,
> > + vma->vm_flags, vma->anon_vma, vma->vm_file,
> > + vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
> > + vma->vm_userfaultfd_ctx, anon_vma_name(vma));
>
> The arguement list gets busy. Maybe some variables would help.
> Calculation around vm_pgoff is not obvious and requires some explanation.

Ok, I will add the following comment:

"Function vma_merge() is called on the extension we are adding to the
already existing vma, vma_merge() will merge this extension with the
already existing vma (expand operation itself) and possibly also with
the next vma if it becomes adjacent to the expanded vma and otherwise
compatible."

And I will also introduce three new variables to better explain some
of the arguments:

unsigned long extension_start = addr + old_len;
unsigned long extension_end = addr + new_len;
pgoff_t extension_pgoff = vma->vm_pgoff + (old_len >> PAGE_SHIFT);

>
> > + if (!vma) {
> > vm_unacct_memory(pages);
> > ret = -ENOMEM;
> > goto out;
> > --
> > 2.35.1
> >
>
> --
> Kirill A. Shutemov

Jakub Matěna