Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

2020-05-05 Thread Guenter Roeck
On 5/4/20 8:39 AM, Mike Rapoport wrote:
> On Sun, May 03, 2020 at 11:43:00AM -0700, Guenter Roeck wrote:
>> On Sun, May 03, 2020 at 10:41:38AM -0700, Guenter Roeck wrote:
>>> Hi,
>>>
>>> On Wed, Apr 29, 2020 at 03:11:23PM +0300, Mike Rapoport wrote:
 From: Mike Rapoport 

 Some architectures (e.g. ARC) have the ZONE_HIGHMEM zone below the
 ZONE_NORMAL. Allowing free_area_init() parse max_zone_pfn array even it is
 sorted in descending order allows using free_area_init() on such
 architectures.

 Add top -> down traversal of max_zone_pfn array in free_area_init() and use
 the latter in ARC node/zone initialization.

 Signed-off-by: Mike Rapoport 
>>>
>>> This patch causes my microblazeel qemu boot test in linux-next to fail.
>>> Reverting it fixes the problem.
>>>
>> The same problem is seen with s390 emulations.
> 
> Yeah, this patch breaks some others as well :(
> 
> My assumption that max_zone_pfn defines architectural limit for maximal
> PFN that can belong to a zone was over-optimistic. Several arches
> actually do that, but others do
> 
>   max_zone_pfn[ZONE_DMA] = MAX_DMA_PFN;
>   max_zone_pfn[ZONE_NORMAL] = max_pfn;
> 
> where MAX_DMA_PFN is build-time constrain and max_pfn is run time limit
> for the current system.
> 
> So, when max_pfn is lower than MAX_DMA_PFN, the free_init_area() will
> consider max_zone_pfn as descending and will wrongly calculate zone
> extents.
> 
> That said, instead of trying to create a generic way to special case
> ARC, I suggest to simply use the below patch instead.
> 

As a reminder, I reported the problem against s390 and microblazeel
(interestingly enough, microblaze (big endian) works), not against arc.

Guenter

> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index 41eb9be1653c..386959bac3d2 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -77,6 +77,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 
> size)
>   base, TO_MB(size), !in_use ? "Not used":"");
>  }
>  
> +bool arch_has_descending_max_zone_pfns(void)
> +{
> + return true;
> +}
> +
>  /*
>   * First memory setup routine called from setup_arch()
>   * 1. setup swapper's mm @init_mm
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b990e9734474..114f0e027144 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7307,6 +7307,15 @@ static void check_for_memory(pg_data_t *pgdat, int nid)
>   }
>  }
>  
> +/*
> + * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below ZONE_NORMAL. For
> + * such cases we allow max_zone_pfn sorted in the descending order
> + */
> +bool __weak arch_has_descending_max_zone_pfns(void)
> +{
> + return false;
> +}
> +
>  /**
>   * free_area_init - Initialise all pg_data_t and zone data
>   * @max_zone_pfn: an array of max PFNs for each zone
> @@ -7324,7 +7333,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
>  {
>   unsigned long start_pfn, end_pfn;
>   int i, nid, zone;
> - bool descending = false;
> + bool descending;
>  
>   /* Record where the zone boundaries are */
>   memset(arch_zone_lowest_possible_pfn, 0,
> @@ -7333,14 +7342,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
>   sizeof(arch_zone_highest_possible_pfn));
>  
>   start_pfn = find_min_pfn_with_active_regions();
> -
> - /*
> -  * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below
> -  * ZONE_NORMAL. For such cases we allow max_zone_pfn sorted in the
> -  * descending order
> -  */
> - if (MAX_NR_ZONES > 1 && max_zone_pfn[0] > max_zone_pfn[1])
> - descending = true;
> + descending = arch_has_descending_max_zone_pfns();
>  
>   for (i = 0; i < MAX_NR_ZONES; i++) {
>   if (descending)
> 
>> Guenter
>>
>>> qemu command line:
>>>
>>> qemu-system-microblazeel -M petalogix-ml605 -m 256 \
>>> -kernel arch/microblaze/boot/linux.bin -no-reboot \
>>> -initrd rootfs.cpio \
>>> -append 'panic=-1 slub_debug=FZPUA rdinit=/sbin/init 
>>> console=ttyS0,115200' \
>>> -monitor none -serial stdio -nographic
>>>
>>> initrd:
>>> 
>>> https://github.com/groeck/linux-build-test/blob/master/rootfs/microblazeel/rootfs.cpio.gz
>>> configuration:
>>> 
>>> https://github.com/groeck/linux-build-test/blob/master/rootfs/microblazeel/qemu_microblazeel_ml605_defconfig
>>>
>>> Bisect log is below.
>>>
>>> Guenter
>>>
>>> ---
>>> # bad: [fb9d670f57e3f6478602328bbbf71138be06ca4f] Add linux-next specific 
>>> files for 20200501
>>> # good: [6a8b55ed4056ea5559ebe4f6a4b247f627870d4c] Linux 5.7-rc3
>>> git bisect start 'HEAD' 'v5.7-rc3'
>>> # good: [068b80b68a670f0b17288c8a3d1ee751f35162ab] Merge remote-tracking 
>>> branch 'drm/drm-next'
>>> git bisect good 068b80b68a670f0b17288c8a3d1ee751f35162ab
>>> # good: [46c70fc6a3ac35cd72ddad248dcbe4eee716d2a5] Merge remote-tracking 
>>> branch 'drivers-x86/for-next'
>>> git bisect good 46c70fc6a3ac35cd72ddad248dcbe4eee716

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

2020-05-05 Thread Mike Rapoport
On Tue, May 05, 2020 at 06:18:11AM -0700, Guenter Roeck wrote:
> On 5/4/20 8:39 AM, Mike Rapoport wrote:
> > On Sun, May 03, 2020 at 11:43:00AM -0700, Guenter Roeck wrote:
> >> On Sun, May 03, 2020 at 10:41:38AM -0700, Guenter Roeck wrote:
> >>> Hi,
> >>>
> >>> On Wed, Apr 29, 2020 at 03:11:23PM +0300, Mike Rapoport wrote:
>  From: Mike Rapoport 
> 
>  Some architectures (e.g. ARC) have the ZONE_HIGHMEM zone below the
>  ZONE_NORMAL. Allowing free_area_init() parse max_zone_pfn array even it 
>  is
>  sorted in descending order allows using free_area_init() on such
>  architectures.
> 
>  Add top -> down traversal of max_zone_pfn array in free_area_init() and 
>  use
>  the latter in ARC node/zone initialization.
> 
>  Signed-off-by: Mike Rapoport 
> >>>
> >>> This patch causes my microblazeel qemu boot test in linux-next to fail.
> >>> Reverting it fixes the problem.
> >>>
> >> The same problem is seen with s390 emulations.
> > 
> > Yeah, this patch breaks some others as well :(
> > 
> > My assumption that max_zone_pfn defines architectural limit for maximal
> > PFN that can belong to a zone was over-optimistic. Several arches
> > actually do that, but others do
> > 
> > max_zone_pfn[ZONE_DMA] = MAX_DMA_PFN;
> > max_zone_pfn[ZONE_NORMAL] = max_pfn;
> > 
> > where MAX_DMA_PFN is build-time constrain and max_pfn is run time limit
> > for the current system.
> > 
> > So, when max_pfn is lower than MAX_DMA_PFN, the free_init_area() will
> > consider max_zone_pfn as descending and will wrongly calculate zone
> > extents.
> > 
> > That said, instead of trying to create a generic way to special case
> > ARC, I suggest to simply use the below patch instead.
> > 
> 
> As a reminder, I reported the problem against s390 and microblazeel
> (interestingly enough, microblaze (big endian) works), not against arc.

With this fix microblazeel and s390 worked for me and also Christian had
reported that s390 is fixed.

microblaze (big endian) works because its defconfig does not enable HIGHMEM
while little endian does.

ARC is mentioned because it is the only arch that may have ZONE_HIGHMEM
and ZONE_NORMAL and this patch was required to consolidate
free_area_init* variants.

> Guenter
> 
> > diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> > index 41eb9be1653c..386959bac3d2 100644
> > --- a/arch/arc/mm/init.c
> > +++ b/arch/arc/mm/init.c
> > @@ -77,6 +77,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 
> > size)
> > base, TO_MB(size), !in_use ? "Not used":"");
> >  }
> >  
> > +bool arch_has_descending_max_zone_pfns(void)
> > +{
> > +   return true;
> > +}
> > +
> >  /*
> >   * First memory setup routine called from setup_arch()
> >   * 1. setup swapper's mm @init_mm
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index b990e9734474..114f0e027144 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -7307,6 +7307,15 @@ static void check_for_memory(pg_data_t *pgdat, int 
> > nid)
> > }
> >  }
> >  
> > +/*
> > + * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below ZONE_NORMAL. For
> > + * such cases we allow max_zone_pfn sorted in the descending order
> > + */
> > +bool __weak arch_has_descending_max_zone_pfns(void)
> > +{
> > +   return false;
> > +}
> > +
> >  /**
> >   * free_area_init - Initialise all pg_data_t and zone data
> >   * @max_zone_pfn: an array of max PFNs for each zone
> > @@ -7324,7 +7333,7 @@ void __init free_area_init(unsigned long 
> > *max_zone_pfn)
> >  {
> > unsigned long start_pfn, end_pfn;
> > int i, nid, zone;
> > -   bool descending = false;
> > +   bool descending;
> >  
> > /* Record where the zone boundaries are */
> > memset(arch_zone_lowest_possible_pfn, 0,
> > @@ -7333,14 +7342,7 @@ void __init free_area_init(unsigned long 
> > *max_zone_pfn)
> > sizeof(arch_zone_highest_possible_pfn));
> >  
> > start_pfn = find_min_pfn_with_active_regions();
> > -
> > -   /*
> > -* Some architecturs, e.g. ARC may have ZONE_HIGHMEM below
> > -* ZONE_NORMAL. For such cases we allow max_zone_pfn sorted in the
> > -* descending order
> > -*/
> > -   if (MAX_NR_ZONES > 1 && max_zone_pfn[0] > max_zone_pfn[1])
> > -   descending = true;
> > +   descending = arch_has_descending_max_zone_pfns();
> >  
> > for (i = 0; i < MAX_NR_ZONES; i++) {
> > if (descending)
> > 
> >> Guenter
> >>
> >>> qemu command line:
> >>>
> >>> qemu-system-microblazeel -M petalogix-ml605 -m 256 \
> >>>   -kernel arch/microblaze/boot/linux.bin -no-reboot \
> >>>   -initrd rootfs.cpio \
> >>>   -append 'panic=-1 slub_debug=FZPUA rdinit=/sbin/init 
> >>> console=ttyS0,115200' \
> >>>   -monitor none -serial stdio -nographic
> >>>
> >>> initrd:
> >>>   
> >>> https://github.com/groeck/linux-build-test/blob/master/rootfs/microblazeel/rootfs.cpio.gz
> >>> configuration:
> >>>   
> >>> https://github.com/groeck/linux-build-test/blob/master/ro

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

2020-05-05 Thread Vineet Gupta
On 5/5/20 6:18 AM, Guenter Roeck wrote:
> On 5/4/20 8:39 AM, Mike Rapoport wrote:
>> On Sun, May 03, 2020 at 11:43:00AM -0700, Guenter Roeck wrote:
>>> On Sun, May 03, 2020 at 10:41:38AM -0700, Guenter Roeck wrote:
 Hi,

 On Wed, Apr 29, 2020 at 03:11:23PM +0300, Mike Rapoport wrote:
> From: Mike Rapoport 
>
> Some architectures (e.g. ARC) have the ZONE_HIGHMEM zone below the
> ZONE_NORMAL. Allowing free_area_init() parse max_zone_pfn array even it is
> sorted in descending order allows using free_area_init() on such
> architectures.
>
> Add top -> down traversal of max_zone_pfn array in free_area_init() and 
> use
> the latter in ARC node/zone initialization.
>
> Signed-off-by: Mike Rapoport 
 This patch causes my microblazeel qemu boot test in linux-next to fail.
 Reverting it fixes the problem.

>>> The same problem is seen with s390 emulations.
>> Yeah, this patch breaks some others as well :(
>>
>> My assumption that max_zone_pfn defines architectural limit for maximal
>> PFN that can belong to a zone was over-optimistic. Several arches
>> actually do that, but others do
>>
>>  max_zone_pfn[ZONE_DMA] = MAX_DMA_PFN;
>>  max_zone_pfn[ZONE_NORMAL] = max_pfn;
>>
>> where MAX_DMA_PFN is build-time constrain and max_pfn is run time limit
>> for the current system.
>>
>> So, when max_pfn is lower than MAX_DMA_PFN, the free_init_area() will
>> consider max_zone_pfn as descending and will wrongly calculate zone
>> extents.
>>
>> That said, instead of trying to create a generic way to special case
>> ARC, I suggest to simply use the below patch instead.
>>
> As a reminder, I reported the problem against s390 and microblazeel
> (interestingly enough, microblaze (big endian) works), not against arc.

Understood and my comment was to point to any other problems in future.

Thx,
-Vineet
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH] semaphore: consolidate arch headers into a generic one

2020-05-05 Thread Vineet Gupta
On 5/5/20 12:05 PM, Adhemerval Zanella via Libc-alpha wrote:
>> diff --git a/sysdeps/s390/nptl/bits/semaphore.h 
>> b/sysdeps/unix/sysv/linux/bits/semaphore.h
>> similarity index 100%
>> rename from sysdeps/s390/nptl/bits/semaphore.h
>> rename to sysdeps/unix/sysv/linux/bits/semaphore.h
>
> Ok, although I think we should handle as a new file: add a online description 
> and
> remove any 'Contributed by' line.

Ok did explicit add/del but still git rename detection triggers, this time
matching it to x86 version (with 90% similarity). I'm pretty sure in the past
delete/add used to elide renames, perhaps the heuristics have gotten better. 
AFAIK
there is no gitconfig setting to disable the rename detection.

...
 sysdeps/{x86 => unix/sysv/linux}/bits/semaphore.h |  5 ++---
 sysdeps/unix/sysv/linux/powerpc/bits/semaphore.h  | 40
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V2 05/11] {x86,powerpc,microblaze}/kmap: Move preempt disable

2020-05-05 Thread Christoph Hellwig
On Sun, May 03, 2020 at 06:09:06PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> During this kmap() conversion series we must maintain bisect-ability.
> To do this, kmap_atomic_prot() in x86, powerpc, and microblaze need to
> remain functional.
> 
> Create a temporary inline version of kmap_atomic_prot within these
> architectures so we can rework their kmap_atomic() calls and then lift
> kmap_atomic_prot() to the core.
> 
> Signed-off-by: Ira Weiny 
> 
> ---
> Changes from V1:
>   New patch
> ---
>  arch/microblaze/include/asm/highmem.h | 11 ++-
>  arch/microblaze/mm/highmem.c  | 10 ++
>  arch/powerpc/include/asm/highmem.h| 11 ++-
>  arch/powerpc/mm/highmem.c |  9 ++---
>  arch/x86/include/asm/highmem.h| 11 ++-
>  arch/x86/mm/highmem_32.c  | 10 ++
>  6 files changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/highmem.h 
> b/arch/microblaze/include/asm/highmem.h
> index 0c94046f2d58..ec9954b091e1 100644
> --- a/arch/microblaze/include/asm/highmem.h
> +++ b/arch/microblaze/include/asm/highmem.h
> @@ -51,7 +51,16 @@ extern pte_t *pkmap_page_table;
>  #define PKMAP_NR(virt)  ((virt - PKMAP_BASE) >> PAGE_SHIFT)
>  #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
>  
> -extern void *kmap_atomic_prot(struct page *page, pgprot_t prot);
> +extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
> +void *kmap_atomic_prot(struct page *page, pgprot_t prot)

Shouldn't this be marked inline?

The rest looks fine:

Reviewed-by: Christoph Hellwig 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V2 06/11] arch/kmap_atomic: Consolidate duplicate code

2020-05-05 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V2 07/11] arch/kunmap_atomic: Consolidate duplicate code

2020-05-05 Thread Christoph Hellwig
On Sun, May 03, 2020 at 06:09:08PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> Every single architecture (including !CONFIG_HIGHMEM) calls...
> 
>   pagefault_enable();
>   preempt_enable();
> 
> ... before returning from __kunmap_atomic().  Lift this code into the
> kunmap_atomic() macro.
> 
> While we are at it rename __kunmap_atomic() to kunmap_atomic_high() to
> be consistent.
> 
> Signed-off-by: Ira Weiny 

Looks good,

Reviewed-by: Christoph Hellwig 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V2 08/11] arch/kmap: Ensure kmap_prot visibility

2020-05-05 Thread Christoph Hellwig
On Sun, May 03, 2020 at 06:09:09PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> We want to support kmap_atomic_prot() on all architectures and it makes
> sense to define kmap_atomic() to use the default kmap_prot.
> 
> So we ensure all arch's have a globally available kmap_prot either as a
> define or exported symbol.

FYI, I still think a

#ifndef kmap_prot
#define kmap_prot PAGE_KERNEL
#endif

in linux/highmem.h would be nicer.  Then only xtensa and sparc need
to override it and clearly stand out.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V2 10/11] arch/kmap: Define kmap_atomic_prot() for all arch's

2020-05-05 Thread Christoph Hellwig
On Sun, May 03, 2020 at 06:09:11PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> To support kmap_atomic_prot(), all architectures need to support
> protections passed to their kmap_atomic_high() function.  Pass
> protections into kmap_atomic_high() and change the name to
> kmap_atomic_high_prot() to match.
> 
> Then define kmap_atomic_prot() as a core function which calls
> kmap_atomic_high_prot() when needed.
> 
> Finally, redefine kmap_atomic() as a wrapper of kmap_atomic_prot() with
> the default kmap_prot exported by the architectures.
> 
> Signed-off-by: Ira Weiny 

Looks good,

Reviewed-by: Christoph Hellwig 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc