| Message ID | 20210926161259.238054-4-namit@vmware.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/madvise: support process_madvise(MADV_DONTNEED) | expand |
On Sun, Sep 26, 2021 at 09:12:54AM -0700, Nadav Amit wrote: > From: Nadav Amit <namit@vmware.com> > > madvise_free_single_vma() currently rechecks that the range fits within > the VMA, adapts it accordingly, and returns -EINVAL if the range is > entirely outside of the VMA. > > The error-code of -EINVAL is incorrect according to the man pages (as it > should have been -ENOMEM), but anyhow the range that is provided to > madvise_free_single_vma() should always be valid. It is set correctly in > do_madvise() and then rechecked in madvise_dontneed_free() is the > mmap-lock is dropped. > > Remove this check. > > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Colin Cross <ccross@google.com> > Cc: Suren Baghdasarya <surenb@google.com> > Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> > Signed-off-by: Nadav Amit <namit@vmware.com> > --- > mm/madvise.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index fe843513a4e8..17e39c70704b 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -716,14 +716,8 @@ static int madvise_free_single_vma(struct vm_area_struct *vma, > if (!vma_is_anonymous(vma)) > return -EINVAL; > > - range.start = max(vma->vm_start, start_addr); > - if (range.start >= vma->vm_end) > - return -EINVAL; > - range.end = min(vma->vm_end, end_addr); > - if (range.end <= vma->vm_start) > - return -EINVAL; How did you test this change? As far as I can see, you leave 'range' uninitialized, but used for walk_page_range() and mmu_notifiers. NAK. > mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm, > - range.start, range.end); > + start_addr, end_addr); > > lru_add_drain(); > tlb_gather_mmu(&tlb, mm); > -- > 2.25.1 > >
On Mon, Sep 27, 2021 at 12:17:09PM +0300, Kirill A. Shutemov wrote: > On Sun, Sep 26, 2021 at 09:12:54AM -0700, Nadav Amit wrote: > > From: Nadav Amit <namit@vmware.com> > > > > madvise_free_single_vma() currently rechecks that the range fits within > > the VMA, adapts it accordingly, and returns -EINVAL if the range is > > entirely outside of the VMA. > > > > The error-code of -EINVAL is incorrect according to the man pages (as it > > should have been -ENOMEM), but anyhow the range that is provided to > > madvise_free_single_vma() should always be valid. It is set correctly in > > do_madvise() and then rechecked in madvise_dontneed_free() is the s/is/if/ > > mmap-lock is dropped. > > > > Remove this check. > > > > Cc: Andrea Arcangeli <aarcange@redhat.com> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Minchan Kim <minchan@kernel.org> > > Cc: Colin Cross <ccross@google.com> > > Cc: Suren Baghdasarya <surenb@google.com> > > Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> > > Signed-off-by: Nadav Amit <namit@vmware.com> > > --- > > mm/madvise.c | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index fe843513a4e8..17e39c70704b 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -716,14 +716,8 @@ static int madvise_free_single_vma(struct vm_area_struct *vma, > > if (!vma_is_anonymous(vma)) > > return -EINVAL; > > > > - range.start = max(vma->vm_start, start_addr); > > - if (range.start >= vma->vm_end) > > - return -EINVAL; > > - range.end = min(vma->vm_end, end_addr); > > - if (range.end <= vma->vm_start) > > - return -EINVAL; > > How did you test this change? > > As far as I can see, you leave 'range' uninitialized, but used for > walk_page_range() and mmu_notifiers. > > NAK. Sorry. My bad. mmu_notifier_range_init will init the range even if mmu notifiers are disabled. > > > mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm, > > - range.start, range.end); > > + start_addr, end_addr); > > > > lru_add_drain(); > > tlb_gather_mmu(&tlb, mm); > > -- > > 2.25.1 > > > > > > -- > Kirill A. Shutemov
diff --git a/mm/madvise.c b/mm/madvise.c index fe843513a4e8..17e39c70704b 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -716,14 +716,8 @@ static int madvise_free_single_vma(struct vm_area_struct *vma, if (!vma_is_anonymous(vma)) return -EINVAL; - range.start = max(vma->vm_start, start_addr); - if (range.start >= vma->vm_end) - return -EINVAL; - range.end = min(vma->vm_end, end_addr); - if (range.end <= vma->vm_start) - return -EINVAL; mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm, - range.start, range.end); + start_addr, end_addr); lru_add_drain(); tlb_gather_mmu(&tlb, mm);