Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932451AbbDMOLb (ORCPT ); Mon, 13 Apr 2015 10:11:31 -0400 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:56580 "EHLO e06smtp14.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932124AbbDMOL3 (ORCPT ); Mon, 13 Apr 2015 10:11:29 -0400 Message-ID: <552BCE87.8040103@linux.vnet.ibm.com> Date: Mon, 13 Apr 2015 16:11:19 +0200 From: Laurent Dufour User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: "Kirill A. Shutemov" , Pavel Emelyanov CC: Andrew Morton , "Kirill A. Shutemov" , Hugh Dickins , Rik van Riel , Mel Gorman , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Ingo Molnar , linuxppc-dev@lists.ozlabs.org, cov@codeaurora.org, criu@openvz.org Subject: Re: [RESEND PATCH v3 1/2] mm: Introducing arch_remap hook References: <9d827fc618a718830b2c47aa87e8be546914c897.1428916945.git.ldufour@linux.vnet.ibm.com> <20150413115811.GA12354@node.dhcp.inet.fi> <552BB972.3010704@linux.vnet.ibm.com> <20150413131357.GC12354@node.dhcp.inet.fi> <552BC2CA.80309@linux.vnet.ibm.com> <552BC619.9080603@parallels.com> <20150413140219.GA14480@node.dhcp.inet.fi> In-Reply-To: <20150413140219.GA14480@node.dhcp.inet.fi> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15041314-0017-0000-0000-000003B104A2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5089 Lines: 122 On 13/04/2015 16:02, Kirill A. Shutemov wrote: > On Mon, Apr 13, 2015 at 04:35:21PM +0300, Pavel Emelyanov wrote: >> On 04/13/2015 04:21 PM, Laurent Dufour wrote: >>> On 13/04/2015 15:13, Kirill A. Shutemov wrote: >>>> On Mon, Apr 13, 2015 at 02:41:22PM +0200, Laurent Dufour wrote: >>>>> On 13/04/2015 13:58, Kirill A. Shutemov wrote: >>>>>> On Mon, Apr 13, 2015 at 11:56:27AM +0200, Laurent Dufour wrote: >>>>>>> Some architecture would like to be triggered when a memory area is moved >>>>>>> through the mremap system call. >>>>>>> >>>>>>> This patch is introducing a new arch_remap mm hook which is placed in the >>>>>>> path of mremap, and is called before the old area is unmapped (and the >>>>>>> arch_unmap hook is called). >>>>>>> >>>>>>> The architectures which need to call this hook should define >>>>>>> __HAVE_ARCH_REMAP in their asm/mmu_context.h and provide the arch_remap >>>>>>> service with the following prototype: >>>>>>> void arch_remap(struct mm_struct *mm, >>>>>>> unsigned long old_start, unsigned long old_end, >>>>>>> unsigned long new_start, unsigned long new_end); >>>>>>> >>>>>>> Signed-off-by: Laurent Dufour >>>>>>> Reviewed-by: Ingo Molnar >>>>>>> --- >>>>>>> mm/mremap.c | 19 +++++++++++++------ >>>>>>> 1 file changed, 13 insertions(+), 6 deletions(-) >>>>>>> >>>>>>> diff --git a/mm/mremap.c b/mm/mremap.c >>>>>>> index 2dc44b1cb1df..009db5565893 100644 >>>>>>> --- a/mm/mremap.c >>>>>>> +++ b/mm/mremap.c >>>>>>> @@ -25,6 +25,7 @@ >>>>>>> >>>>>>> #include >>>>>>> #include >>>>>>> +#include >>>>>>> >>>>>>> #include "internal.h" >>>>>>> >>>>>>> @@ -286,13 +287,19 @@ static unsigned long move_vma(struct vm_area_struct *vma, >>>>>>> old_len = new_len; >>>>>>> old_addr = new_addr; >>>>>>> new_addr = -ENOMEM; >>>>>>> - } else if (vma->vm_file && vma->vm_file->f_op->mremap) { >>>>>>> - err = vma->vm_file->f_op->mremap(vma->vm_file, new_vma); >>>>>>> - if (err < 0) { >>>>>>> - move_page_tables(new_vma, new_addr, vma, old_addr, >>>>>>> - moved_len, true); >>>>>>> - return err; >>>>>>> + } else { >>>>>>> + if (vma->vm_file && vma->vm_file->f_op->mremap) { >>>>>>> + err = vma->vm_file->f_op->mremap(vma->vm_file, new_vma); >>>>>>> + if (err < 0) { >>>>>>> + move_page_tables(new_vma, new_addr, vma, >>>>>>> + old_addr, moved_len, true); >>>>>>> + return err; >>>>>>> + } >>>>>>> } >>>>>>> +#ifdef __HAVE_ARCH_REMAP >>>>>> >>>>>> It would be cleaner to provide dummy arch_remap() for !__HAVE_ARCH_REMAP >>>>>> in some generic header. >>>>> >>>>> The idea was to not impact all the architectures as arch_unmap(), >>>>> arch_dup_mmap() or arch_exit_mmap() implies. >>>>> >>>>> I look at the headers where such a dummy arch_remap could be put but I >>>>> can't figure out one which will not impact all the architecture. >>>>> What about defining a dummy service earlier in mm/remap.c in the case >>>>> __HAVE_ARCH_REMAP is not defined ? >>>>> Something like : >>>>> #ifndef __HAVE_ARCH_REMAP >>>>> static inline void void arch_remap(struct mm_struct *mm, >>>>> unsigned long old_start, >>>>> unsigned long old_end, >>>>> unsigned long new_start, >>>>> unsigned long new_end) >>>>> { >>>>> } >>>>> #endif >>>> >>>> Or just #define arch_remap(...) do { } while (0) >>>> >>> >>> I guessed you wanted the arch_remap() prototype to be exposed somewhere >>> in the code. >>> >>> To be honest, I can't find the benefit of defining a dummy arch_remap() >>> in mm/remap.c if __HAVE_ARCH_REMAP is not defined instead of calling it >>> in move_vma if __HAVE_ARCH_REMAP is defined. >>> Is it really what you want ? >> >> I think Kirill meant something like e.g. the arch_enter_lazy_mmu_mode() >> is implemented and called in mm/mremap.c -- the "generic" part is in the >> include/asm-generic/pgtable.h and those architectures willing to have >> their own implementation are in arch/$arch/... >> >> Kirill, if I'm right with it, can you suggest the header where to put >> the "generic" mremap hook's (empty) body? > > I initially thought it would be enough to put it into > , expecting it works as > . But that's not the case. > > It probably worth at some point rework all to include > at the end as we do for . > But that's outside the scope of the patchset, I guess. > > I don't see any better candidate for such dummy header. :-/ Clearly, I'm not confortable with a rewrite of :( So what about this patch, is this v3 acceptable ? Cheers, Laurent. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/