| Message ID | 20150127161657.GA7155@node.dhcp.inet.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 27, 2015 at 06:16:57PM +0200, Kirill A. Shutemov wrote: > On Mon, Jan 26, 2015 at 06:03:57AM -0800, Guenter Roeck wrote: > > On 01/26/2015 04:29 AM, Kirill A. Shutemov wrote: > > >On Fri, Jan 23, 2015 at 09:52:07PM -0800, Guenter Roeck wrote: > > >>On Fri, Jan 23, 2015 at 01:55:19PM -0800, Andrew Morton wrote: > > >>>On Fri, 23 Jan 2015 07:07:56 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > > >>> > > >>>>>> > > >>>>>>qemu:microblaze generates warnings to the console. > > >>>>>> > > >>>>>>WARNING: CPU: 0 PID: 32 at mm/mmap.c:2858 exit_mmap+0x184/0x1a4() > > >>>>>> > > >>>>>>with various call stacks. See > > >>>>>>http://server.roeck-us.net:8010/builders/qemu-microblaze-mmotm/builds/15/steps/qemubuildcommand/logs/stdio > > >>>>>>for details. > > >>>>> > > >>>>>Could you try patch below? Completely untested. > > >>>>> > > >>>>>>From b584bb8d493794f67484c0b57c161d61c02599bc Mon Sep 17 00:00:00 2001 > > >>>>>From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > >>>>>Date: Fri, 23 Jan 2015 13:08:26 +0200 > > >>>>>Subject: [PATCH] microblaze: define __PAGETABLE_PMD_FOLDED > > >>>>> > > >>>>>Microblaze uses custom implementation of PMD folding, but doesn't define > > >>>>>__PAGETABLE_PMD_FOLDED, which generic code expects to see. Let's fix it. > > >>>>> > > >>>>>Defining __PAGETABLE_PMD_FOLDED will drop out unused __pmd_alloc(). > > >>>>>It also fixes problems with recently-introduced pmd accounting. > > >>>>> > > >>>>>Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > >>>>>Reported-by: Guenter Roeck <linux@roeck-us.net> > > >>>> > > >>>>Tested working. > > >>>> > > >>>>Tested-by: Guenter Roeck <linux@roeck-us.net> > > >>>> > > >>>>Any idea how to fix the sh problem ? > > >>> > > >>>Can you tell us more about it? All I'm seeing is "qemu:sh fails to > > >>>shut down", which isn't very clear. > > >> > > >>Turns out that the include file defining __PAGETABLE_PMD_FOLDED > > >>was not always included where used, resulting in a messed up mm_struct. > > > > > >What means "messed up" here? It should only affect size of mm_struct. > > > > > Plus the offset of all variables after the #ifndef. > > Okay, I guess the problem is that different parts of the kernel see > different mm_struct depending on include ordering. > > Tried to look for options, but don't see anything better than patch below. > Andrew, is it okay to you? > > From 0f113e16a058d47f3bc63a3b6ced5296afb934a6 Mon Sep 17 00:00:00 2001 > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Date: Tue, 27 Jan 2015 17:59:55 +0200 > Subject: [PATCH] mm: add nr_pmds into mm_struct unconditionally > > __PAGETABLE_PMD_FOLDED is defined during <asm/pgtable.h> which is not > included into <linux/mm_types.h>. And we cannot include it here since > many of <asm/pgtables> needs <linux/mm_types.h> to define struct page. > > I failed to come up with better solution rather than put nr_pmds into > mm_struct unconditionally. > > One possible solution would be to expose number of page table levels > architecture has via Kconfig, but that's ugly and requires changes to > all architectures. > FWIW, I tried a number of approaches. Ultimately I gave up and concluded that it has to be either this patch or, as you say here, we would have to add something like PAGETABLE_PMD_FOLDED as a Kconfig option. > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Tested working with all builds and all my qemu tests. Tested-by: Guenter Roeck <linux@roeck-us.net> Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 27 Jan 2015 08:24:28 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > > __PAGETABLE_PMD_FOLDED is defined during <asm/pgtable.h> which is not > > included into <linux/mm_types.h>. And we cannot include it here since > > many of <asm/pgtables> needs <linux/mm_types.h> to define struct page. > > > > I failed to come up with better solution rather than put nr_pmds into > > mm_struct unconditionally. > > > > One possible solution would be to expose number of page table levels > > architecture has via Kconfig, but that's ugly and requires changes to > > all architectures. > > > FWIW, I tried a number of approaches. Ultimately I gave up and concluded > that it has to be either this patch or, as you say here, we would have > to add something like PAGETABLE_PMD_FOLDED as a Kconfig option. It's certainly a big mess. Yes, I expect that moving __PAGETABLE_PMD_FOLDED and probably PAGETABLE_LEVELS into Kconfig logic would be a good fix. Adding 8 bytes to the mm_struct (sometimes) isn't a huge issue, but it does make the kernel just a little bit worse. Has anyone taken a look at what the Kconfig approach would look like? Possibly another fix for this would be to move mm_struct into its own header file, or something along those lines? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/27/2015 01:24 PM, Andrew Morton wrote: > On Tue, 27 Jan 2015 08:24:28 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > >>> __PAGETABLE_PMD_FOLDED is defined during <asm/pgtable.h> which is not >>> included into <linux/mm_types.h>. And we cannot include it here since >>> many of <asm/pgtables> needs <linux/mm_types.h> to define struct page. >>> >>> I failed to come up with better solution rather than put nr_pmds into >>> mm_struct unconditionally. >>> >>> One possible solution would be to expose number of page table levels >>> architecture has via Kconfig, but that's ugly and requires changes to >>> all architectures. >>> >> FWIW, I tried a number of approaches. Ultimately I gave up and concluded >> that it has to be either this patch or, as you say here, we would have >> to add something like PAGETABLE_PMD_FOLDED as a Kconfig option. > > It's certainly a big mess. Yes, I expect that moving > __PAGETABLE_PMD_FOLDED and probably PAGETABLE_LEVELS into Kconfig logic > would be a good fix. > > Adding 8 bytes to the mm_struct (sometimes) isn't a huge issue, but > it does make the kernel just a little bit worse. > > Has anyone taken a look at what the Kconfig approach would look like? > We would need something like config PAGETABLE_PMD_FOLDED (or maybe PAGETABLE_NOPMD) def_bool y for arc, arm64, avr32, cris, hexagon, metag, mips, mn10300, nios2, openrisc, powerpc, score, sh, tile, um, unicore32, x86, xtensa, arm, m32r, and microblaze. In several cases it would depend on secondary options, such as CONFIG_ARM64_PGTABLE_LEVELS for arm64 or PAGETABLE_LEVELS for x86 and sh. PAGETABLE_LEVELS is not a configuration option (yet), so, yes, that would have to be converted to a configuration option as well. Overall a lot of complexity. Not really sure if that is worth the gain. We would have to touch more than 20 Kconfig files plus about 20 source and include files which currently use _PAGETABLE_PMD_FOLDED. > Possibly another fix for this would be to move mm_struct into its own > header file, or something along those lines? > I suspect that might be just as messy. We would have to find all files which actually need mm_struct and make sure that the new mm_struct.h is included. Not sure which approach is better. Sure, the 8 (or 4) bytes are annoying, but I am not sure if the situation is bad enough to really bother. Ultimately it seems there may be other variables in mm_struct which could be made optional with much less effort, such as uprobes_state or mmap_legacy_base. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 79cdf6f5c746..199a03aab8dc 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -364,9 +364,7 @@ struct mm_struct { atomic_t mm_users; /* How many users with user space? */ atomic_t mm_count; /* How many references to "struct mm_struct" (users count as 1) */ atomic_long_t nr_ptes; /* PTE page table pages */ -#ifndef __PAGETABLE_PMD_FOLDED atomic_long_t nr_pmds; /* PMD page table pages */ -#endif int map_count; /* number of VMAs */ spinlock_t page_table_lock; /* Protects page tables and some counters */