LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v8 1/4] mm/cma: Add PF flag to force non cma alloc
From: Vlastimil Babka @ 2019-02-28 12:20 UTC (permalink / raw)
  To: Aneesh Kumar K.V, akpm, Michal Hocko, Alexey Kardashevskiy,
	David Gibson, Andrea Arcangeli, mpe
  Cc: Peter Zijlstra, linux-kernel, Matthew Wilcox, linux-mm,
	Ingo Molnar, linuxppc-dev
In-Reply-To: <20190227144736.5872-2-aneesh.kumar@linux.ibm.com>

On 2/27/19 3:47 PM, Aneesh Kumar K.V wrote:
> This patch adds PF_MEMALLOC_NOCMA which make sure any allocation in that context
> is marked non-movable and hence cannot be satisfied by CMA region.
> 
> This is useful with get_user_pages_longterm where we want to take a page pin by
> migrating pages from CMA region. Marking the section PF_MEMALLOC_NOCMA ensures
> that we avoid unnecessary page migration later.
> 
> Suggested-by: Andrea Arcangeli <aarcange@redhat.com>
> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

+CC scheduler guys

Do we really take the last available PF flag just so that "we avoid
unnecessary page migration later"?
If yes, that's a third PF_MEMALLOC flag, should we get separate variable
for gfp context at this point?
Also I don't like the name PF_MEMALLOC_NOCMA, as it's unnecessarily tied
to CMA. If anything it should be e.g. PF_MEMALLOC_NOMOVABLE.

Thanks.

> ---
>  include/linux/sched.h    |  1 +
>  include/linux/sched/mm.h | 48 +++++++++++++++++++++++++++++++++-------
>  2 files changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f9b43c989577..dfa90088ba08 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1403,6 +1403,7 @@ extern struct pid *cad_pid;
>  #define PF_UMH			0x02000000	/* I'm an Usermodehelper process */
>  #define PF_NO_SETAFFINITY	0x04000000	/* Userland is not allowed to meddle with cpus_allowed */
>  #define PF_MCE_EARLY		0x08000000      /* Early kill for mce process policy */
> +#define PF_MEMALLOC_NOCMA	0x10000000 /* All allocation request will have _GFP_MOVABLE cleared */
>  #define PF_MUTEX_TESTER		0x20000000	/* Thread belongs to the rt mutex tester */
>  #define PF_FREEZER_SKIP		0x40000000	/* Freezer should not count it as freezable */
>  #define PF_SUSPEND_TASK		0x80000000      /* This thread called freeze_processes() and should not be frozen */
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 3bfa6a0cbba4..0cd9f10423fb 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -148,17 +148,25 @@ static inline bool in_vfork(struct task_struct *tsk)
>   * Applies per-task gfp context to the given allocation flags.
>   * PF_MEMALLOC_NOIO implies GFP_NOIO
>   * PF_MEMALLOC_NOFS implies GFP_NOFS
> + * PF_MEMALLOC_NOCMA implies no allocation from CMA region.
>   */
>  static inline gfp_t current_gfp_context(gfp_t flags)
>  {
> -	/*
> -	 * NOIO implies both NOIO and NOFS and it is a weaker context
> -	 * so always make sure it makes precedence
> -	 */
> -	if (unlikely(current->flags & PF_MEMALLOC_NOIO))
> -		flags &= ~(__GFP_IO | __GFP_FS);
> -	else if (unlikely(current->flags & PF_MEMALLOC_NOFS))
> -		flags &= ~__GFP_FS;
> +	if (unlikely(current->flags &
> +		     (PF_MEMALLOC_NOIO | PF_MEMALLOC_NOFS | PF_MEMALLOC_NOCMA))) {
> +		/*
> +		 * NOIO implies both NOIO and NOFS and it is a weaker context
> +		 * so always make sure it makes precedence
> +		 */
> +		if (current->flags & PF_MEMALLOC_NOIO)
> +			flags &= ~(__GFP_IO | __GFP_FS);
> +		else if (current->flags & PF_MEMALLOC_NOFS)
> +			flags &= ~__GFP_FS;
> +#ifdef CONFIG_CMA
> +		if (current->flags & PF_MEMALLOC_NOCMA)
> +			flags &= ~__GFP_MOVABLE;
> +#endif
> +	}
>  	return flags;
>  }
>  
> @@ -248,6 +256,30 @@ static inline void memalloc_noreclaim_restore(unsigned int flags)
>  	current->flags = (current->flags & ~PF_MEMALLOC) | flags;
>  }
>  
> +#ifdef CONFIG_CMA
> +static inline unsigned int memalloc_nocma_save(void)
> +{
> +	unsigned int flags = current->flags & PF_MEMALLOC_NOCMA;
> +
> +	current->flags |= PF_MEMALLOC_NOCMA;
> +	return flags;
> +}
> +
> +static inline void memalloc_nocma_restore(unsigned int flags)
> +{
> +	current->flags = (current->flags & ~PF_MEMALLOC_NOCMA) | flags;
> +}
> +#else
> +static inline unsigned int memalloc_nocma_save(void)
> +{
> +	return 0;
> +}
> +
> +static inline void memalloc_nocma_restore(unsigned int flags)
> +{
> +}
> +#endif
> +
>  #ifdef CONFIG_MEMCG
>  /**
>   * memalloc_use_memcg - Starts the remote memcg charging scope.
> 


^ permalink raw reply

* Re: PROBLEM: monotonic clock going backwards on ppc64
From: Daniel Axtens @ 2019-02-28 11:17 UTC (permalink / raw)
  To: Mathieu Malaterre, Jakub Drnec; +Cc: linuxppc-dev
In-Reply-To: <CA+7wUswyzc5SGjjb+WS4QpopyL0qoGYj0WF1jNaTfsC5ZsMpdw@mail.gmail.com>

Mathieu Malaterre <malat@debian.org> writes:

> On Tue, Feb 26, 2019 at 9:36 PM Jakub Drnec <jaydee@email.cz> wrote:
>>
>> Hi all,
>>
>> I think I observed a potential problem, is this the correct place to report it? (CC me, not on list)
>>
>> [1.] One line summary: monotonic clock can be made to decrease on ppc64
>> [2.] Full description:
>> Setting the realtime clock can sometimes make the monotonic clock go back by over a hundred years.
>> Decreasing the realtime clock across the y2k38 threshold is one reliable way to reproduce.
>> Allegedly this can also happen just by running ntpd, I have not managed to reproduce that other
>> than booting with rtc at >2038 and then running ntp.
>
> Isn't it the expected behavior. Here is what I see for powermac:
>
> $ git show 22db552b50fa
> ...
>     This changes the logic to cast to an unsigned 32-bit number first for
>     the Macintosh time and then convert that to the Unix time, which then
>     gives us a time in the documented 1904..2040 year range. I decided not
>     to use the longer 1970..2106 range that other drivers use, for
>     consistency with the literal interpretation of the register, but that
>     could be easily changed if we decide we want to support any Mac after
>     2040.
> ...
>

My interpretation of that commit is that it relates to the kernel
reading the hardware RTC on a powermac, but this issue relates to
userspace fetching the time from the vDSO. I'm also not running on a
powermac, so my hardware should be able to deal with times > 2040...


Regards,
Daniel

>> When this happens, anything with timers (e.g. openjdk) breaks rather badly.
>>
>> [3.] Keywords: gettimeofday, ppc64, vdso
>> [4.] Kernel information
>> [4.1.] Kernel version: any (tested on 4.19)
>> [4.2.] Kernel .config file: any
>> [5.] Most recent kernel version which did not have the bug: not a regression
>> [6.] Output of Oops..: not applicable
>> [7.] Example program which triggers the problem
>> --- testcase.c
>> #include <stdio.h>
>> #include <time.h>
>> #include <stdlib.h>
>> #include <unistd.h>
>>
>> long get_time() {
>>   struct timespec tp;
>>   if (clock_gettime(CLOCK_MONOTONIC, &tp) != 0) {
>>     perror("clock_gettime failed");
>>     exit(1);
>>   }
>>   long result = tp.tv_sec + tp.tv_nsec / 1000000000;
>>   return result;
>> }
>>
>> int main() {
>>   printf("monitoring monotonic clock...\n");
>>   long last = get_time();
>>   while(1) {
>>     long now = get_time();
>>     if (now < last) {
>>       printf("clock went backwards by %ld seconds!\n",
>>         last - now);
>>     }
>>     last = now;
>>     sleep(1);
>>   }
>>   return 0;
>> }
>> ---
>> when running
>> # date -s 2040-1-1
>> # date -s 2037-1-1
>> program outputs: clock went backwards by 4294967295 seconds!
>>
>> [8.] Environment: any ppc64, currently reproducing on qemu-system-ppc64le running debian unstable
>> [X.] Other notes, patches, fixes, workarounds:
>> The problem seems to be in vDSO code in arch/powerpc/kernel/vdso64/gettimeofday.S.
>> (possibly because some values used in the calculation are only 32 bit?)
>> Slightly silly workaround:
>> nuke the "cmpwi cr1,r3,CLOCK_MONOTONIC" in __kernel_clock_gettime
>> Now it always goes through the syscall fallback which does not have the same problem.
>>
>> Regards,
>> Jakub Drnec

^ permalink raw reply

* Re: BUG: KASAN: stack-out-of-bounds
From: Dmitry Vyukov @ 2019-02-28  9:54 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: kasan-dev, Linux-MM, Alexander Potapenko, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <11314e32-6044-9207-a238-738e394ea2eb@virtuozzo.com>

On Thu, Feb 28, 2019 at 10:46 AM Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
>
>
>
> On 2/28/19 12:27 PM, Dmitry Vyukov wrote:
> > On Thu, Feb 28, 2019 at 10:22 AM Andrey Ryabinin
> > <aryabinin@virtuozzo.com> wrote:
> >>
> >>
> >>
> >> On 2/27/19 4:11 PM, Christophe Leroy wrote:
> >>>
> >>>
> >>> Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit :
> >>>>
> >>>>
> >>>> On 2/27/19 11:25 AM, Christophe Leroy wrote:
> >>>>> With version v8 of the series implementing KASAN on 32 bits powerpc (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm now able to activate KASAN on a mac99 is QEMU.
> >>>>>
> >>>>> Then I get the following reports at startup. Which of the two reports I get seems to depend on the option used to build the kernel, but for a given kernel I always get the same report.
> >>>>>
> >>>>> Is that a real bug, in which case how could I spot it ? Or is it something wrong in my implementation of KASAN ?
> >>>>>
> >>>>> I checked that after kasan_init(), the entire shadow memory is full of 0 only.
> >>>>>
> >>>>> I also made a try with the strong STACK_PROTECTOR compiled in, but no difference and nothing detected by the stack protector.
> >>>>>
> >>>>> ==================================================================
> >>>>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
> >>>>> Read of size 1 at addr c0ecdd40 by task swapper/0
> >>>>>
> >>>>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
> >>>>> Call Trace:
> >>>>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
> >>>>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
> >>>>> [c0e9dd10] [c089579c] memchr+0x24/0x74
> >>>>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
> >>>>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
> >>>>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
> >>>>> --- interrupt: c0e9df00 at 0x400f330
> >>>>>      LR = init_stack+0x1f00/0x2000
> >>>>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
> >>>>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
> >>>>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
> >>>>> [c0e9dff0] [00003484] 0x3484
> >>>>>
> >>>>> The buggy address belongs to the variable:
> >>>>>   __log_buf+0xec0/0x4020
> >>>>> The buggy address belongs to the page:
> >>>>> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
> >>>>> flags: 0x1000(reserved)
> >>>>> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
> >>>>> page dumped because: kasan: bad access detected
> >>>>>
> >>>>> Memory state around the buggy address:
> >>>>>   c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>>>   c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
> >>>>>                                     ^
> >>>>>   c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> >>>>>   c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>>> ==================================================================
> >>>>>
> >>>>
> >>>> This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is
> >>>>     "The buggy address belongs to the variable:  __log_buf+0xec0/0x4020"
> >>>>   which is printed by following code:
> >>>>     if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
> >>>>         pr_err("The buggy address belongs to the variable:\n");
> >>>>         pr_err(" %pS\n", addr);
> >>>>     }
> >>>>
> >>>> So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT?
> >>>> KASAN with stack instrumentation significantly increases stack usage.
> >>>>
> >>>
> >>> I get the above with THREAD_SHIFT set to 13 (default value).
> >>> If increasing it to 14, I get the following instead. That means that in that case the problem arises a lot earlier in the boot process (but still after the final kasan shadow setup).
> >>>
> >>
> >> We usually use 15 (with 4k pages), but I think 14 should be enough for the clean boot.
> >>
> >>> ==================================================================
> >>> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1f8/0x5d0
> >>> Read of size 1 at addr f6f37de0 by task swapper/0
> >>>
> >>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1143
> >>> Call Trace:
> >>> [c0e9fd60] [c01c43c0] print_address_description+0x164/0x2bc (unreliable)
> >>> [c0e9fd90] [c01c46a4] kasan_report+0xfc/0x180
> >>> [c0e9fdd0] [c0c226d4] pmac_nvram_init+0x1f8/0x5d0
> >>> [c0e9fef0] [c0c1f73c] pmac_setup_arch+0x298/0x314
> >>> [c0e9ff20] [c0c1ac40] setup_arch+0x250/0x268
> >>> [c0e9ff50] [c0c151dc] start_kernel+0xb8/0x488
> >>> [c0e9fff0] [00003484] 0x3484
> >>>
> >>>
> >>> Memory state around the buggy address:
> >>>  f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>  f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>> f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> >>>                                                ^
> >>>  f6f37e00: 00 00 01 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
> >>>  f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
> >>> ==================================================================
> >>
> >> Powerpc's show_stack() prints stack addresses, so we know that stack is something near 0xc0e9f... address.
> >> f6f37de0 is definitely not stack address and it's to far for the stack overflow.
> >> So it looks like shadow for stack  - kasan_mem_to_shadow(0xc0e9f...) and shadow for address in report - kasan_mem_to_shadow(0xf6f37de0)
> >> point to the same physical page.
> >
> > Shouldn't shadow start at 0xf8 for powerpc32? I did some math
> > yesterday which I think lead me to 0xf8.
>
> Dunno, maybe. How is this relevant? In case you referring to the 0xf6f* addresses in the report,
> these are not shadow, but accessed addresses.

Right. Then never mind.

^ permalink raw reply

* Re: [PATCH v2] powerpc/powernv/idle: Restore IAMR after idle
From: Gautham R Shenoy @ 2019-02-28  9:54 UTC (permalink / raw)
  To: Russell Currey; +Cc: linuxppc-dev
In-Reply-To: <20190208111103.4901-1-ruscur@russell.cc>

Hello Russell,

On Fri, Feb 08, 2019 at 10:11:03PM +1100, Russell Currey wrote:
> Without restoring the IAMR after idle, execution prevention on POWER9
> with Radix MMU is overwritten and the kernel can freely execute userspace without
> faulting.
> 
> This is necessary when returning from any stop state that modifies user
> state, as well as hypervisor state.
> 
> To test how this fails without this patch, load the lkdtm driver and
> do the following:
> 
>    echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
> 
> which won't fault, then boot the kernel with powersave=off, where it
> will fault.  Applying this patch will fix this.
> 
> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> space")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> Since v1:
>       - no longer use paca to save IAMR, instead use _DAR (thanks mpe)

Looks good to me. Once we move to Nick Piggin's C-based save/restore
code, we will be saving all these SPR values on the stack anyway.

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

--
Thanks and Regards
gautham.


^ permalink raw reply

* Re: [PATCH] powernv: powercap: Add hard minimum powercap
From: Gautham R Shenoy @ 2019-02-28  9:52 UTC (permalink / raw)
  To: Shilpasri G Bhat; +Cc: ego, linuxppc-dev, linux-kernel, Daniel Axtens
In-Reply-To: <57fc8253-c5c3-6103-77cd-34aa4ec250a3@linux.vnet.ibm.com>

Hi Shilpa,

On Thu, Feb 28, 2019 at 11:25:25AM +0530, Shilpasri G Bhat wrote:
> Hi,
> 
> On 02/28/2019 10:14 AM, Daniel Axtens wrote:
> > Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> > 
> >> In POWER9, OCC(On-Chip-Controller) provides for hard and soft system
> >> powercapping range. The hard powercap range is guaranteed while soft
> >> powercap may or may not be asserted due to various power-thermal
> >> reasons based on system configuration and workloads. This patch adds
> >> a sysfs file to export the hard minimum powercap limit to allow the
> >> user to set the appropriate powercap value that can be managed by the
> >> system.
> > 
> > Maybe it's common terminology and I'm just not aware of it, but what do
> > you mean by "asserted"? It doesn't appear elsewhere in the documentation
> > you're patching, and it's not a use of assert that I'm familiar with...
> > 
> > Regards,
> > Daniel
> > 
> 
> I meant to say powercap will not be assured in the soft powercap range, i.e,
> system may or may not be throttled of CPU frequency to remain within the powercap.
> 
> I can reword the document and commit message.

I agree with Daniel. How about replacing "asserted" with "enforced by
the OCC"?


> 
> Thanks and Regards,
> Shilpa

--
Thanks and Regards
gautham.

> 
> >>
> >> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> >> ---
> >>  .../ABI/testing/sysfs-firmware-opal-powercap       | 10 ++++
> >>  arch/powerpc/platforms/powernv/opal-powercap.c     | 66 +++++++++-------------
> >>  2 files changed, 37 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-firmware-opal-powercap b/Documentation/ABI/testing/sysfs-firmware-opal-powercap
> >> index c9b66ec..65db4c1 100644
> >> --- a/Documentation/ABI/testing/sysfs-firmware-opal-powercap
> >> +++ b/Documentation/ABI/testing/sysfs-firmware-opal-powercap
> >> @@ -29,3 +29,13 @@ Description:	System powercap directory and attributes applicable for
> >>  		  creates a request for setting a new-powercap. The
> >>  		  powercap requested must be between powercap-min
> >>  		  and powercap-max.
> >> +
> >> +What:		/sys/firmware/opal/powercap/system-powercap/powercap-hard-min
> >> +Date:		Feb 2019
> >> +Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> >> +Description:	Hard minimum powercap
> >> +
> >> +		This file provides the hard minimum powercap limit in
> >> +		Watts. The powercap value above hard minimum is always
> >> +		guaranteed to be asserted and the powercap value below
> >> +		the hard minimum limit may or may not be guaranteed.
> >> diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c b/arch/powerpc/platforms/powernv/opal-powercap.c
> >> index d90ee4f..38408e7 100644
> >> --- a/arch/powerpc/platforms/powernv/opal-powercap.c
> >> +++ b/arch/powerpc/platforms/powernv/opal-powercap.c
> >> @@ -139,10 +139,24 @@ static void powercap_add_attr(int handle, const char *name,
> >>  	attr->handle = handle;
> >>  	sysfs_attr_init(&attr->attr.attr);
> >>  	attr->attr.attr.name = name;
> >> -	attr->attr.attr.mode = 0444;
> >> +
> >> +	if (!strncmp(name, "powercap-current", strlen(name))) {
> >> +		attr->attr.attr.mode = 0664;
> >> +		attr->attr.store = powercap_store;
> >> +	} else {
> >> +		attr->attr.attr.mode = 0444;
> >> +	}
> >> +
> >>  	attr->attr.show = powercap_show;
> >>  }
> >>  
> >> +static const char * const powercap_strs[] = {
> >> +	"powercap-max",
> >> +	"powercap-min",
> >> +	"powercap-current",
> >> +	"powercap-hard-min",
> >> +};
> >> +
> >>  void __init opal_powercap_init(void)
> >>  {
> >>  	struct device_node *powercap, *node;
> >> @@ -167,60 +181,34 @@ void __init opal_powercap_init(void)
> >>  
> >>  	i = 0;
> >>  	for_each_child_of_node(powercap, node) {
> >> -		u32 cur, min, max;
> >> -		int j = 0;
> >> -		bool has_cur = false, has_min = false, has_max = false;
> >> +		u32 id;
> >> +		int j, count = 0;
> >>  
> >> -		if (!of_property_read_u32(node, "powercap-min", &min)) {
> >> -			j++;
> >> -			has_min = true;
> >> -		}
> >> -
> >> -		if (!of_property_read_u32(node, "powercap-max", &max)) {
> >> -			j++;
> >> -			has_max = true;
> >> -		}
> >> +		for (j = 0; j < ARRAY_SIZE(powercap_strs); j++)
> >> +			if (!of_property_read_u32(node, powercap_strs[j], &id))
> >> +				count++;
> >>  
> >> -		if (!of_property_read_u32(node, "powercap-current", &cur)) {
> >> -			j++;
> >> -			has_cur = true;
> >> -		}
> >> -
> >> -		pcaps[i].pattrs = kcalloc(j, sizeof(struct powercap_attr),
> >> +		pcaps[i].pattrs = kcalloc(count, sizeof(struct powercap_attr),
> >>  					  GFP_KERNEL);
> >>  		if (!pcaps[i].pattrs)
> >>  			goto out_pcaps_pattrs;
> >>  
> >> -		pcaps[i].pg.attrs = kcalloc(j + 1, sizeof(struct attribute *),
> >> +		pcaps[i].pg.attrs = kcalloc(count + 1,
> >> +					    sizeof(struct attribute *),
> >>  					    GFP_KERNEL);
> >>  		if (!pcaps[i].pg.attrs) {
> >>  			kfree(pcaps[i].pattrs);
> >>  			goto out_pcaps_pattrs;
> >>  		}
> >>  
> >> -		j = 0;
> >>  		pcaps[i].pg.name = kasprintf(GFP_KERNEL, "%pOFn", node);
> >> -		if (has_min) {
> >> -			powercap_add_attr(min, "powercap-min",
> >> -					  &pcaps[i].pattrs[j]);
> >> -			pcaps[i].pg.attrs[j] = &pcaps[i].pattrs[j].attr.attr;
> >> -			j++;
> >> -		}
> >> -
> >> -		if (has_max) {
> >> -			powercap_add_attr(max, "powercap-max",
> >> -					  &pcaps[i].pattrs[j]);
> >> -			pcaps[i].pg.attrs[j] = &pcaps[i].pattrs[j].attr.attr;
> >> -			j++;
> >> -		}
> >> +		for (j = 0; j < ARRAY_SIZE(powercap_strs); j++) {
> >> +			if (of_property_read_u32(node, powercap_strs[j], &id))
> >> +				continue;
> >>  
> >> -		if (has_cur) {
> >> -			powercap_add_attr(cur, "powercap-current",
> >> +			powercap_add_attr(id, powercap_strs[j],
> >>  					  &pcaps[i].pattrs[j]);
> >> -			pcaps[i].pattrs[j].attr.attr.mode |= 0220;
> >> -			pcaps[i].pattrs[j].attr.store = powercap_store;
> >>  			pcaps[i].pg.attrs[j] = &pcaps[i].pattrs[j].attr.attr;
> >> -			j++;
> >>  		}
> >>  
> >>  		if (sysfs_create_group(powercap_kobj, &pcaps[i].pg)) {
> >> -- 
> >> 1.8.3.1
> > 


^ permalink raw reply

* Re: [PATCH v2] powerpc/pseries: Only wait for dying CPU after call to rtas_stop_self()
From: Gautham R Shenoy @ 2019-02-28  9:47 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: Gautham R Shenoy, linux-kernel, Michael Bringmann, Tyrel Datwyler,
	linuxppc-dev
In-Reply-To: <20190222225752.6375-1-bauerman@linux.ibm.com>

Hello Thiago,

On Fri, Feb 22, 2019 at 07:57:52PM -0300, Thiago Jung Bauermann wrote:
> When testing DLPAR CPU add/remove on a system under stress,
> pseries_cpu_die() doesn't wait long enough for a CPU to die:
> 
> [  446.983944] cpu 148 (hwid 148) Ready to die...
> [  446.984062] cpu 149 (hwid 149) Ready to die...
> [  446.993518] cpu 150 (hwid 150) Ready to die...
> [  446.993543] Querying DEAD? cpu 150 (150) shows 2
> [  446.994098] cpu 151 (hwid 151) Ready to die...
> [  447.133726] cpu 136 (hwid 136) Ready to die...
> [  447.403532] cpu 137 (hwid 137) Ready to die...
> [  447.403772] cpu 138 (hwid 138) Ready to die...
> [  447.403839] cpu 139 (hwid 139) Ready to die...
> [  447.403887] cpu 140 (hwid 140) Ready to die...
> [  447.403937] cpu 141 (hwid 141) Ready to die...
> [  447.403979] cpu 142 (hwid 142) Ready to die...
> [  447.404038] cpu 143 (hwid 143) Ready to die...
> [  447.513546] cpu 128 (hwid 128) Ready to die...
> [  447.693533] cpu 129 (hwid 129) Ready to die...
> [  447.693999] cpu 130 (hwid 130) Ready to die...
> [  447.703530] cpu 131 (hwid 131) Ready to die...
> [  447.704087] Querying DEAD? cpu 132 (132) shows 2
> [  447.704102] cpu 132 (hwid 132) Ready to die...
> [  447.713534] cpu 133 (hwid 133) Ready to die...
> [  447.714064] Querying DEAD? cpu 134 (134) shows 2
> 
> This is a race between one CPU stopping and another one calling
> pseries_cpu_die() to wait for it to stop. That function does a short busy
> loop calling RTAS query-cpu-stopped-state on the stopping CPU to verify
> that it is stopped, but I think there's a lot for the stopping CPU to do
> which may take longer than this loop allows.
> 
> As can be seen in the dmesg right before or after the "Querying DEAD?"
> messages, if pseries_cpu_die() waited a little longer it would have seen
> the CPU in the stopped state.

> 
> I see two cases that can be causing this race:
> 
> 1. It's possible that CPU 134 was inactive at the time it was unplugged. In
>    that case, dlpar_offline_cpu() calls H_PROD on that CPU and immediately
>    calls pseries_cpu_die(). Meanwhile, the prodded CPU activates and start
>    the process of stopping itself. It's possible that the busy loop is not
>    long enough to allow for the CPU to wake up and complete the stopping
>    process.

The problem is a bit more severe since, after printing "Querying
DEAD?" for CPU X, this CPU can prod another offline CPU Y on the same
core which, on waking up, will call rtas_stop_self. Thus we can have two
concurrent calls to rtas-stop-self, which is prohibited by the PAPR.


> 
> 2. If CPU 134 was online at the time it was unplugged, it would have gone
>    through the new CPU hotplug state machine in kernel/cpu.c that was
>    introduced in v4.6 to get itself stopped. It's possible that the busy
>    loop in pseries_cpu_die() was long enough for the older hotplug code but
>    not for the new hotplug state machine.

I haven't been able to observe the "Querying DEAD?" messages for the
online CPU which was being offlined and dlpar'ed out.

> 
> I don't know if this race condition has any ill effects, but we can make
> the race a lot more even if we only start querying if the CPU is stopped
> when the stopping CPU is close to call rtas_stop_self().
> 
> Since pseries_mach_cpu_die() sets the CPU current state to offline almost
> immediately before calling rtas_stop_self(), we use that as a signal that
> it is either already stopped or very close to that point, and we can start
> the busy loop.
> 
> As suggested by Michael Ellerman, this patch also changes the busy loop to
> wait for a fixed amount of wall time.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> I tried to estimate good amounts for the timeout and loop delays, but
> I'm not sure how reasonable my numbers are. The busy loops will wait for
> 100 µs between each try, and spin_event_timeout() will timeout after
> 100 ms. I'll be happy to change these values if you have better
> suggestions.

Based on the measurements that I did on a POWER9 system, in successful
cases of smp_query_cpu_stopped(cpu) returning affirmative, the maximum
time spent inside the loop was was 10ms.


> Gautham was able to test this patch and it solved the race condition.
> 
> v1 was a cruder patch which just increased the number of loops:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-February/153734.html
> 
> v1 also mentioned a kernel crash but Gautham narrowed it down to a bug
> in RTAS, which is in the process of being fixed.
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 97feb6e79f1a..424146cc752e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -214,13 +214,21 @@ static void pseries_cpu_die(unsigned int cpu)
>  			msleep(1);
>  		}
>  	} else if (get_preferred_offline_state(cpu) == CPU_STATE_OFFLINE) {
> +		/*
> +		 * If the current state is not offline yet, it means that the
> +		 * dying CPU (which is in pseries_mach_cpu_die) didn't have a
> +		 * chance to call rtas_stop_self yet and therefore it's too
> +		 * early to query if the CPU is stopped.
> +		 */
> +		spin_event_timeout(get_cpu_current_state(cpu) == CPU_STATE_OFFLINE,
> +				   100000, 100);
> 
>  		for (tries = 0; tries < 25; tries++) {

Can we bumped up the tries to 100, so that we wait for 10ms before
printing the warning message ?

>  			cpu_status = smp_query_cpu_stopped(pcpu);
>  			if (cpu_status == QCSS_STOPPED ||
>  			    cpu_status == QCSS_HARDWARE_ERROR)
>  				break;
> -			cpu_relax();
> +			udelay(100);
>  		}
>  	}
> 


^ permalink raw reply

* Re: BUG: KASAN: stack-out-of-bounds
From: Andrey Ryabinin @ 2019-02-28  9:47 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: kasan-dev, Linux-MM, Alexander Potapenko, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <CACT4Y+adjRarmcWTrQxotATzaHoFQ4TXbyiRXEpWozLPzjQBrQ@mail.gmail.com>



On 2/28/19 12:27 PM, Dmitry Vyukov wrote:
> On Thu, Feb 28, 2019 at 10:22 AM Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>>
>>
>>
>> On 2/27/19 4:11 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit :
>>>>
>>>>
>>>> On 2/27/19 11:25 AM, Christophe Leroy wrote:
>>>>> With version v8 of the series implementing KASAN on 32 bits powerpc (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm now able to activate KASAN on a mac99 is QEMU.
>>>>>
>>>>> Then I get the following reports at startup. Which of the two reports I get seems to depend on the option used to build the kernel, but for a given kernel I always get the same report.
>>>>>
>>>>> Is that a real bug, in which case how could I spot it ? Or is it something wrong in my implementation of KASAN ?
>>>>>
>>>>> I checked that after kasan_init(), the entire shadow memory is full of 0 only.
>>>>>
>>>>> I also made a try with the strong STACK_PROTECTOR compiled in, but no difference and nothing detected by the stack protector.
>>>>>
>>>>> ==================================================================
>>>>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
>>>>> Read of size 1 at addr c0ecdd40 by task swapper/0
>>>>>
>>>>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
>>>>> Call Trace:
>>>>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
>>>>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
>>>>> [c0e9dd10] [c089579c] memchr+0x24/0x74
>>>>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
>>>>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
>>>>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
>>>>> --- interrupt: c0e9df00 at 0x400f330
>>>>>      LR = init_stack+0x1f00/0x2000
>>>>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
>>>>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
>>>>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
>>>>> [c0e9dff0] [00003484] 0x3484
>>>>>
>>>>> The buggy address belongs to the variable:
>>>>>   __log_buf+0xec0/0x4020
>>>>> The buggy address belongs to the page:
>>>>> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
>>>>> flags: 0x1000(reserved)
>>>>> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
>>>>> page dumped because: kasan: bad access detected
>>>>>
>>>>> Memory state around the buggy address:
>>>>>   c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>>>   c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
>>>>>                                     ^
>>>>>   c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>>>>>   c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>>> ==================================================================
>>>>>
>>>>
>>>> This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is
>>>>     "The buggy address belongs to the variable:  __log_buf+0xec0/0x4020"
>>>>   which is printed by following code:
>>>>     if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
>>>>         pr_err("The buggy address belongs to the variable:\n");
>>>>         pr_err(" %pS\n", addr);
>>>>     }
>>>>
>>>> So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT?
>>>> KASAN with stack instrumentation significantly increases stack usage.
>>>>
>>>
>>> I get the above with THREAD_SHIFT set to 13 (default value).
>>> If increasing it to 14, I get the following instead. That means that in that case the problem arises a lot earlier in the boot process (but still after the final kasan shadow setup).
>>>
>>
>> We usually use 15 (with 4k pages), but I think 14 should be enough for the clean boot.
>>
>>> ==================================================================
>>> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1f8/0x5d0
>>> Read of size 1 at addr f6f37de0 by task swapper/0
>>>
>>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1143
>>> Call Trace:
>>> [c0e9fd60] [c01c43c0] print_address_description+0x164/0x2bc (unreliable)
>>> [c0e9fd90] [c01c46a4] kasan_report+0xfc/0x180
>>> [c0e9fdd0] [c0c226d4] pmac_nvram_init+0x1f8/0x5d0
>>> [c0e9fef0] [c0c1f73c] pmac_setup_arch+0x298/0x314
>>> [c0e9ff20] [c0c1ac40] setup_arch+0x250/0x268
>>> [c0e9ff50] [c0c151dc] start_kernel+0xb8/0x488
>>> [c0e9fff0] [00003484] 0x3484
>>>
>>>
>>> Memory state around the buggy address:
>>>  f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>  f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>>>                                                ^
>>>  f6f37e00: 00 00 01 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
>>>  f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
>>> ==================================================================
>>
>> Powerpc's show_stack() prints stack addresses, so we know that stack is something near 0xc0e9f... address.
>> f6f37de0 is definitely not stack address and it's to far for the stack overflow.
>> So it looks like shadow for stack  - kasan_mem_to_shadow(0xc0e9f...) and shadow for address in report - kasan_mem_to_shadow(0xf6f37de0)
>> point to the same physical page.
> 
> Shouldn't shadow start at 0xf8 for powerpc32? I did some math
> yesterday which I think lead me to 0xf8.

Dunno, maybe. How is this relevant? In case you referring to the 0xf6f* addresses in the report,
these are not shadow, but accessed addresses.

> This allows to cover at most 1GB of memory. Do you have more by any chance?
> 

^ permalink raw reply

* Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
From: Oliver @ 2019-02-28  9:40 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Dan Williams
  Cc: Jan Kara, Linux Kernel Mailing List, Linux MM, Ross Zwisler,
	Andrew Morton, linuxppc-dev, Kirill A . Shutemov
In-Reply-To: <20190228083522.8189-2-aneesh.kumar@linux.ibm.com>

On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Add a flag to indicate the ability to do huge page dax mapping. On architecture
> like ppc64, the hypervisor can disable huge page support in the guest. In
> such a case, we should not enable huge page dax mapping. This patch adds
> a flag which the architecture code will update to indicate huge page
> dax mapping support.

*groan*

> Architectures mostly do transparent_hugepage_flag = 0; if they can't
> do hugepages. That also takes care of disabling dax hugepage mapping
> with this change.
>
> Without this patch we get the below error with kvm on ppc64.
>
> [  118.849975] lpar: Failed hash pte insert with error -4
>
> NOTE: The patch also use
>
> echo never > /sys/kernel/mm/transparent_hugepage/enabled
> to disable dax huge page mapping.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> TODO:
> * Add Fixes: tag
>
>  include/linux/huge_mm.h | 4 +++-
>  mm/huge_memory.c        | 4 ++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 381e872bfde0..01ad5258545e 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
>                         pud_t *pud, pfn_t pfn, bool write);
>  enum transparent_hugepage_flag {
>         TRANSPARENT_HUGEPAGE_FLAG,
> +       TRANSPARENT_HUGEPAGE_DAX_FLAG,
>         TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
>         TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
>         TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG,
> @@ -111,7 +112,8 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>         if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
>                 return true;
>
> -       if (vma_is_dax(vma))
> +       if (vma_is_dax(vma) &&
> +           (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_DAX_FLAG)))
>                 return true;

Forcing PTE sized faults should be fine for fsdax, but it'll break
devdax. The devdax driver requires the fault size be >= the namespace
alignment since devdax tries to guarantee hugepage mappings will be
used and PMD alignment is the default. We can probably have devdax
fall back to the largest size the hypervisor has made available, but
it does run contrary to the design. Ah well, I suppose it's better off
being degraded rather than unusable.

>         if (transparent_hugepage_flags &
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index faf357eaf0ce..43d742fe0341 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -53,6 +53,7 @@ unsigned long transparent_hugepage_flags __read_mostly =
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE_MADVISE
>         (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG)|
>  #endif
> +       (1 << TRANSPARENT_HUGEPAGE_DAX_FLAG) |
>         (1<<TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG)|
>         (1<<TRANSPARENT_HUGEPAGE_DEFRAG_KHUGEPAGED_FLAG)|
>         (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG);
> @@ -475,6 +476,8 @@ static int __init setup_transparent_hugepage(char *str)
>                           &transparent_hugepage_flags);
>                 clear_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
>                           &transparent_hugepage_flags);
> +               clear_bit(TRANSPARENT_HUGEPAGE_DAX_FLAG,
> +                         &transparent_hugepage_flags);
>                 ret = 1;
>         }
>  out:

> @@ -753,6 +756,7 @@ static void insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
>         spinlock_t *ptl;
>
>         ptl = pmd_lock(mm, pmd);
> +       /* should we check for none here again? */

VM_WARN_ON() maybe? If THP is disabled and we're here then something
has gone wrong.

>         entry = pmd_mkhuge(pfn_t_pmd(pfn, prot));
>         if (pfn_t_devmap(pfn))
>                 entry = pmd_mkdevmap(entry);
> --
> 2.20.1
>

^ permalink raw reply

* Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
From: Jan Kara @ 2019-02-28  9:40 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Jan Kara, linux-kernel, linux-mm, Oliver O'Halloran,
	Ross Zwisler, akpm, linuxppc-dev, Dan Williams,
	Kirill A . Shutemov
In-Reply-To: <20190228083522.8189-2-aneesh.kumar@linux.ibm.com>

On Thu 28-02-19 14:05:22, Aneesh Kumar K.V wrote:
> Add a flag to indicate the ability to do huge page dax mapping. On architecture
> like ppc64, the hypervisor can disable huge page support in the guest. In
> such a case, we should not enable huge page dax mapping. This patch adds
> a flag which the architecture code will update to indicate huge page
> dax mapping support.
> 
> Architectures mostly do transparent_hugepage_flag = 0; if they can't
> do hugepages. That also takes care of disabling dax hugepage mapping
> with this change.
> 
> Without this patch we get the below error with kvm on ppc64.
> 
> [  118.849975] lpar: Failed hash pte insert with error -4
> 
> NOTE: The patch also use
> 
> echo never > /sys/kernel/mm/transparent_hugepage/enabled
> to disable dax huge page mapping.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

Added Dan to CC for opinion. I kind of fail to see why you don't use
TRANSPARENT_HUGEPAGE_FLAG for this. I know that technically DAX huge pages
and normal THPs are different things but so far we've tried to avoid making
that distinction visible to userspace.

								Honza
> ---
> TODO:
> * Add Fixes: tag
> 
>  include/linux/huge_mm.h | 4 +++-
>  mm/huge_memory.c        | 4 ++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 381e872bfde0..01ad5258545e 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
>  			pud_t *pud, pfn_t pfn, bool write);
>  enum transparent_hugepage_flag {
>  	TRANSPARENT_HUGEPAGE_FLAG,
> +	TRANSPARENT_HUGEPAGE_DAX_FLAG,
>  	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
>  	TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
>  	TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG,
> @@ -111,7 +112,8 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>  	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
>  		return true;
>  
> -	if (vma_is_dax(vma))
> +	if (vma_is_dax(vma) &&
> +	    (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_DAX_FLAG)))
>  		return true;
>  
>  	if (transparent_hugepage_flags &
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index faf357eaf0ce..43d742fe0341 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -53,6 +53,7 @@ unsigned long transparent_hugepage_flags __read_mostly =
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE_MADVISE
>  	(1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG)|
>  #endif
> +	(1 << TRANSPARENT_HUGEPAGE_DAX_FLAG) |
>  	(1<<TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG)|
>  	(1<<TRANSPARENT_HUGEPAGE_DEFRAG_KHUGEPAGED_FLAG)|
>  	(1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG);
> @@ -475,6 +476,8 @@ static int __init setup_transparent_hugepage(char *str)
>  			  &transparent_hugepage_flags);
>  		clear_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
>  			  &transparent_hugepage_flags);
> +		clear_bit(TRANSPARENT_HUGEPAGE_DAX_FLAG,
> +			  &transparent_hugepage_flags);
>  		ret = 1;
>  	}
>  out:
> @@ -753,6 +756,7 @@ static void insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
>  	spinlock_t *ptl;
>  
>  	ptl = pmd_lock(mm, pmd);
> +	/* should we check for none here again? */
>  	entry = pmd_mkhuge(pfn_t_pmd(pfn, prot));
>  	if (pfn_t_devmap(pfn))
>  		entry = pmd_mkdevmap(entry);
> -- 
> 2.20.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: BUG: KASAN: stack-out-of-bounds
From: Dmitry Vyukov @ 2019-02-28  9:27 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: kasan-dev, Linux-MM, Alexander Potapenko, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <7778f728-3ca2-7ad6-503f-72ca098863cb@virtuozzo.com>

On Thu, Feb 28, 2019 at 10:22 AM Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
>
>
>
> On 2/27/19 4:11 PM, Christophe Leroy wrote:
> >
> >
> > Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit :
> >>
> >>
> >> On 2/27/19 11:25 AM, Christophe Leroy wrote:
> >>> With version v8 of the series implementing KASAN on 32 bits powerpc (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm now able to activate KASAN on a mac99 is QEMU.
> >>>
> >>> Then I get the following reports at startup. Which of the two reports I get seems to depend on the option used to build the kernel, but for a given kernel I always get the same report.
> >>>
> >>> Is that a real bug, in which case how could I spot it ? Or is it something wrong in my implementation of KASAN ?
> >>>
> >>> I checked that after kasan_init(), the entire shadow memory is full of 0 only.
> >>>
> >>> I also made a try with the strong STACK_PROTECTOR compiled in, but no difference and nothing detected by the stack protector.
> >>>
> >>> ==================================================================
> >>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
> >>> Read of size 1 at addr c0ecdd40 by task swapper/0
> >>>
> >>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
> >>> Call Trace:
> >>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
> >>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
> >>> [c0e9dd10] [c089579c] memchr+0x24/0x74
> >>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
> >>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
> >>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
> >>> --- interrupt: c0e9df00 at 0x400f330
> >>>      LR = init_stack+0x1f00/0x2000
> >>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
> >>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
> >>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
> >>> [c0e9dff0] [00003484] 0x3484
> >>>
> >>> The buggy address belongs to the variable:
> >>>   __log_buf+0xec0/0x4020
> >>> The buggy address belongs to the page:
> >>> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
> >>> flags: 0x1000(reserved)
> >>> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
> >>> page dumped because: kasan: bad access detected
> >>>
> >>> Memory state around the buggy address:
> >>>   c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>   c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
> >>>                                     ^
> >>>   c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> >>>   c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> ==================================================================
> >>>
> >>
> >> This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is
> >>     "The buggy address belongs to the variable:  __log_buf+0xec0/0x4020"
> >>   which is printed by following code:
> >>     if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
> >>         pr_err("The buggy address belongs to the variable:\n");
> >>         pr_err(" %pS\n", addr);
> >>     }
> >>
> >> So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT?
> >> KASAN with stack instrumentation significantly increases stack usage.
> >>
> >
> > I get the above with THREAD_SHIFT set to 13 (default value).
> > If increasing it to 14, I get the following instead. That means that in that case the problem arises a lot earlier in the boot process (but still after the final kasan shadow setup).
> >
>
> We usually use 15 (with 4k pages), but I think 14 should be enough for the clean boot.
>
> > ==================================================================
> > BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1f8/0x5d0
> > Read of size 1 at addr f6f37de0 by task swapper/0
> >
> > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1143
> > Call Trace:
> > [c0e9fd60] [c01c43c0] print_address_description+0x164/0x2bc (unreliable)
> > [c0e9fd90] [c01c46a4] kasan_report+0xfc/0x180
> > [c0e9fdd0] [c0c226d4] pmac_nvram_init+0x1f8/0x5d0
> > [c0e9fef0] [c0c1f73c] pmac_setup_arch+0x298/0x314
> > [c0e9ff20] [c0c1ac40] setup_arch+0x250/0x268
> > [c0e9ff50] [c0c151dc] start_kernel+0xb8/0x488
> > [c0e9fff0] [00003484] 0x3484
> >
> >
> > Memory state around the buggy address:
> >  f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> >                                                ^
> >  f6f37e00: 00 00 01 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
> >  f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
> > ==================================================================
>
> Powerpc's show_stack() prints stack addresses, so we know that stack is something near 0xc0e9f... address.
> f6f37de0 is definitely not stack address and it's to far for the stack overflow.
> So it looks like shadow for stack  - kasan_mem_to_shadow(0xc0e9f...) and shadow for address in report - kasan_mem_to_shadow(0xf6f37de0)
> point to the same physical page.

Shouldn't shadow start at 0xf8 for powerpc32? I did some math
yesterday which I think lead me to 0xf8.
This allows to cover at most 1GB of memory. Do you have more by any chance?

^ permalink raw reply

* Re: BUG: KASAN: stack-out-of-bounds
From: Andrey Ryabinin @ 2019-02-28  9:22 UTC (permalink / raw)
  To: Christophe Leroy, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, kasan-dev, Daniel Axtens
In-Reply-To: <15a40476-2852-cf5a-0982-d899dd79d9c1@c-s.fr>



On 2/27/19 4:11 PM, Christophe Leroy wrote:
> 
> 
> Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit :
>>
>>
>> On 2/27/19 11:25 AM, Christophe Leroy wrote:
>>> With version v8 of the series implementing KASAN on 32 bits powerpc (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm now able to activate KASAN on a mac99 is QEMU.
>>>
>>> Then I get the following reports at startup. Which of the two reports I get seems to depend on the option used to build the kernel, but for a given kernel I always get the same report.
>>>
>>> Is that a real bug, in which case how could I spot it ? Or is it something wrong in my implementation of KASAN ?
>>>
>>> I checked that after kasan_init(), the entire shadow memory is full of 0 only.
>>>
>>> I also made a try with the strong STACK_PROTECTOR compiled in, but no difference and nothing detected by the stack protector.
>>>
>>> ==================================================================
>>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
>>> Read of size 1 at addr c0ecdd40 by task swapper/0
>>>
>>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
>>> Call Trace:
>>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
>>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
>>> [c0e9dd10] [c089579c] memchr+0x24/0x74
>>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
>>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
>>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
>>> --- interrupt: c0e9df00 at 0x400f330
>>>      LR = init_stack+0x1f00/0x2000
>>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
>>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
>>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
>>> [c0e9dff0] [00003484] 0x3484
>>>
>>> The buggy address belongs to the variable:
>>>   __log_buf+0xec0/0x4020
>>> The buggy address belongs to the page:
>>> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
>>> flags: 0x1000(reserved)
>>> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
>>> page dumped because: kasan: bad access detected
>>>
>>> Memory state around the buggy address:
>>>   c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>   c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
>>>                                     ^
>>>   c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>>>   c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ==================================================================
>>>
>>
>> This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is
>>     "The buggy address belongs to the variable:  __log_buf+0xec0/0x4020"
>>   which is printed by following code:
>>     if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
>>         pr_err("The buggy address belongs to the variable:\n");
>>         pr_err(" %pS\n", addr);
>>     }
>>
>> So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT?
>> KASAN with stack instrumentation significantly increases stack usage.
>>
> 
> I get the above with THREAD_SHIFT set to 13 (default value).
> If increasing it to 14, I get the following instead. That means that in that case the problem arises a lot earlier in the boot process (but still after the final kasan shadow setup).
> 

We usually use 15 (with 4k pages), but I think 14 should be enough for the clean boot.

> ==================================================================
> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1f8/0x5d0
> Read of size 1 at addr f6f37de0 by task swapper/0
> 
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1143
> Call Trace:
> [c0e9fd60] [c01c43c0] print_address_description+0x164/0x2bc (unreliable)
> [c0e9fd90] [c01c46a4] kasan_report+0xfc/0x180
> [c0e9fdd0] [c0c226d4] pmac_nvram_init+0x1f8/0x5d0
> [c0e9fef0] [c0c1f73c] pmac_setup_arch+0x298/0x314
> [c0e9ff20] [c0c1ac40] setup_arch+0x250/0x268
> [c0e9ff50] [c0c151dc] start_kernel+0xb8/0x488
> [c0e9fff0] [00003484] 0x3484
> 
> 
> Memory state around the buggy address:
>  f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>                                                ^
>  f6f37e00: 00 00 01 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
>  f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
> ==================================================================

Powerpc's show_stack() prints stack addresses, so we know that stack is something near 0xc0e9f... address.
f6f37de0 is definitely not stack address and it's to far for the stack overflow.
So it looks like shadow for stack  - kasan_mem_to_shadow(0xc0e9f...) and shadow for address in report - kasan_mem_to_shadow(0xf6f37de0)
point to the same physical page. 

^ permalink raw reply

* Re: powerpc/powernv: Make opal log only readable by root
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Jordan Niethe, linuxppc-dev; +Cc: Jordan Niethe
In-Reply-To: <20190227030229.12814-1-jniethe5@gmail.com>

On Wed, 2019-02-27 at 03:02:29 UTC, Jordan Niethe wrote:
> Currently the opal log is globally readable. It is kernel policy to limit
> the visibility of physical addresses / kernel pointers to root.
> Given this and the fact the opal log may contain this information it would
> be better to limit the readability to root.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> Reviewed-by: Stewart Smith <stewart@linux.ibm.com>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7b62f9bd2246b7d3d086e571397c14ba

cheers

^ permalink raw reply

* Re: powerpc/fsl: Fix the flush of branch predictor.
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Diana Craciun
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <dccb392cd3e2b1825608576ecf2487cc5d84466d.1551205086.git.christophe.leroy@c-s.fr>

On Tue, 2019-02-26 at 18:18:48 UTC, Christophe Leroy wrote:
> The commit identified below adds MC_BTB_FLUSH macro only when
> CONFIG_PPC_FSL_BOOK3E is defined. This results in the following error
> on some configs (seen several times with kisskb randconfig_defconfig)
> 
> arch/powerpc/kernel/exceptions-64e.S:576: Error: Unrecognized opcode: `mc_btb_flush'
> make[3]: *** [scripts/Makefile.build:367: arch/powerpc/kernel/exceptions-64e.o] Error 1
> make[2]: *** [scripts/Makefile.build:492: arch/powerpc/kernel] Error 2
> make[1]: *** [Makefile:1043: arch/powerpc] Error 2
> make: *** [Makefile:152: sub-make] Error 2
> 
> This patch adds a blank definition of MC_BTB_FLUSH for other cases.
> 
> Fixes: 10c5e83afd4a ("powerpc/fsl: Flush the branch predictor at each kernel entry (64bit)")
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Reviewed-by: Diana Craciun <diana.craciun@nxp.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/27da80719ef132cf8c80eb406d5aeb37

cheers

^ permalink raw reply

* Re: powerpc/powernv: move OPAL call wrapper tracing and interrupt handling to C
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20190226093035.7176-1-npiggin@gmail.com>

On Tue, 2019-02-26 at 09:30:35 UTC, Nicholas Piggin wrote:
> The OPAL call wrapper gets interrupt disabling wrong. It disables
> interrupts just by clearing MSR[EE], which has two problems:
> 
> - It doesn't call into the IRQ tracing subsystem, which means tracing
>   across OPAL calls does not always notice IRQs have been disabled.
> 
> - It doesn't go through the IRQ soft-mask code, which causes a minor
>   bug. MSR[EE] can not be restored by saving the MSR then clearing
>   MSR[EE], because a racing interrupt while soft-masked could clear
>   MSR[EE] between the two steps. This can cause MSR[EE] to be
>   incorrectly enabled when the OPAL call returns. Fortunately that
>   should only result in another masked interrupt being taken to
>   disable MSR[EE] again, but it's a bit sloppy.
> 
> The existing code also saves MSR to PACA, which is not re-entrant if
> there is a nested OPAL call from different MSR contexts, which can
> happen these days with SRESET interrupts on bare metal.
> 
> To fix these issues, move the tracing and IRQ handling code to C, and
> call into asm just for the low level call when everything is ready to
> go. Save the MSR on stack rather than PACA.
> 
> Performance cost is kept to a minimum with a few optimisations:
> 
> - The endian switch upon return is combined with the MSR restore,
>   which avoids an expensive context synchronizing operation for LE
>   kernels. This makes up for the additional mtmsrd to enable
>   interrupts with local_irq_enable().
> 
> - blr is now used to return from the opal_* functions that are called
>   as C functions, to avoid link stack corruption. This requires a
>   skiboot fix as well to keep the call stack balanced.
> 
> A NULL call is more costly after this, (410ns->430ns on POWER9), but
> OPAL calls are generally not performance critical at this scale.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/75d9fc7fd94eb43cdf0bec04499a27ce

cheers

^ permalink raw reply

* Re: [v4, 1/4] powerpc/64s: Fix HV NMI vs HV interrupt recoverability test
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20190226085110.29653-2-npiggin@gmail.com>

On Tue, 2019-02-26 at 08:51:07 UTC, Nicholas Piggin wrote:
> HV interrupts that use HSRR registers do not enter with MSR[RI] clear,
> but their entry code is not recoverable vs NMI, due to shared use of
> HSPRG1 as a scratch register to save r13.
> 
> This means that a system reset or machine check that hits in HSRR
> interrupt entry can cause r13 to be silently corrupted.
> 
> Fix this by marking NMIs non-recoverable if they land in HV interrupt
> ranges.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ccd477028a202993b9ddca5d2404fdac

cheers

^ permalink raw reply

* Re: powerpc/xmon: Fix opcode being uninitialized in print_insn_powerpc
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Nathan Chancellor, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, Nathan Chancellor, Nick Desaulniers, linux-kernel,
	Joel Stanley
In-Reply-To: <20190226053855.7020-1-natechancellor@gmail.com>

On Tue, 2019-02-26 at 05:38:55 UTC, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
> 
> arch/powerpc/xmon/ppc-dis.c:157:7: warning: variable 'opcode' is used
> uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>   if (cpu_has_feature(CPU_FTRS_POWER9))
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/xmon/ppc-dis.c:167:7: note: uninitialized use occurs here
>   if (opcode == NULL)
>       ^~~~~~
> arch/powerpc/xmon/ppc-dis.c:157:3: note: remove the 'if' if its
> condition is always true
>   if (cpu_has_feature(CPU_FTRS_POWER9))
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/xmon/ppc-dis.c:132:38: note: initialize the variable
> 'opcode' to silence this warning
>   const struct powerpc_opcode *opcode;
>                                      ^
>                                       = NULL
> 1 warning generated.
> 
> This warning seems to make no sense on the surface because opcode is set
> to NULL right below this statement. However, there is a comma instead of
> semicolon to end the dialect assignment, meaning that the opcode
> assignment only happens in the if statement. Properly terminate that
> line so that Clang no longer warns.
> 
> Fixes: 5b102782c7f4 ("powerpc/xmon: Enable disassembly files (compilation changes)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/390
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e7140639b1de65bba435a6bd772d1349

cheers

^ permalink raw reply

* Re: [PATCH 1/2] fs/dax: deposit pagetable even when installing zero page
From: Jan Kara @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Jan Kara, linux-kernel, linux-mm, Oliver O'Halloran,
	Ross Zwisler, akpm, linuxppc-dev, Kirill A . Shutemov
In-Reply-To: <20190228083522.8189-1-aneesh.kumar@linux.ibm.com>

On Thu 28-02-19 14:05:21, Aneesh Kumar K.V wrote:
> Architectures like ppc64 use the deposited page table to store hardware
> page table slot information. Make sure we deposit a page table when
> using zero page at the pmd level for hash.
> 
> Without this we hit
> 
> Unable to handle kernel paging request for data at address 0x00000000
> Faulting instruction address: 0xc000000000082a74
> Oops: Kernel access of bad area, sig: 11 [#1]
> ....
> 
> NIP [c000000000082a74] __hash_page_thp+0x224/0x5b0
> LR [c0000000000829a4] __hash_page_thp+0x154/0x5b0
> Call Trace:
>  hash_page_mm+0x43c/0x740
>  do_hash_page+0x2c/0x3c
>  copy_from_iter_flushcache+0xa4/0x4a0
>  pmem_copy_from_iter+0x2c/0x50 [nd_pmem]
>  dax_copy_from_iter+0x40/0x70
>  dax_iomap_actor+0x134/0x360
>  iomap_apply+0xfc/0x1b0
>  dax_iomap_rw+0xac/0x130
>  ext4_file_write_iter+0x254/0x460 [ext4]
>  __vfs_write+0x120/0x1e0
>  vfs_write+0xd8/0x220
>  SyS_write+0x6c/0x110
>  system_call+0x3c/0x130
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

Thanks for the patch. It looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

> ---
> TODO:
> * Add fixes tag 

Probably this is a problem since initial PPC PMEM support, isn't it?

								Honza

> 
>  fs/dax.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 6959837cc465..01bfb2ac34f9 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -33,6 +33,7 @@
>  #include <linux/sizes.h>
>  #include <linux/mmu_notifier.h>
>  #include <linux/iomap.h>
> +#include <asm/pgalloc.h>
>  #include "internal.h"
>  
>  #define CREATE_TRACE_POINTS
> @@ -1410,7 +1411,9 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
>  {
>  	struct address_space *mapping = vmf->vma->vm_file->f_mapping;
>  	unsigned long pmd_addr = vmf->address & PMD_MASK;
> +	struct vm_area_struct *vma = vmf->vma;
>  	struct inode *inode = mapping->host;
> +	pgtable_t pgtable = NULL;
>  	struct page *zero_page;
>  	spinlock_t *ptl;
>  	pmd_t pmd_entry;
> @@ -1425,12 +1428,22 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
>  	*entry = dax_insert_entry(xas, mapping, vmf, *entry, pfn,
>  			DAX_PMD | DAX_ZERO_PAGE, false);
>  
> +	if (arch_needs_pgtable_deposit()) {
> +		pgtable = pte_alloc_one(vma->vm_mm);
> +		if (!pgtable)
> +			return VM_FAULT_OOM;
> +	}
> +
>  	ptl = pmd_lock(vmf->vma->vm_mm, vmf->pmd);
>  	if (!pmd_none(*(vmf->pmd))) {
>  		spin_unlock(ptl);
>  		goto fallback;
>  	}
>  
> +	if (pgtable) {
> +		pgtable_trans_huge_deposit(vma->vm_mm, vmf->pmd, pgtable);
> +		mm_inc_nr_ptes(vma->vm_mm);
> +	}
>  	pmd_entry = mk_pmd(zero_page, vmf->vma->vm_page_prot);
>  	pmd_entry = pmd_mkhuge(pmd_entry);
>  	set_pmd_at(vmf->vma->vm_mm, pmd_addr, vmf->pmd, pmd_entry);
> @@ -1439,6 +1452,8 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
>  	return VM_FAULT_NOPAGE;
>  
>  fallback:
> +	if (pgtable)
> +		pte_free(vma->vm_mm, pgtable);
>  	trace_dax_pmd_load_hole_fallback(inode, vmf, zero_page, *entry);
>  	return VM_FAULT_FALLBACK;
>  }
> -- 
> 2.20.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [v2, 1/2] powerpc/hugetlb: Handle mmap_min_addr correctly in get_unmapped_area callback
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Aneesh Kumar K.V, npiggin, benh, paulus
  Cc: Aneesh Kumar K.V, Laurent Dufour, linuxppc-dev
In-Reply-To: <20190226043935.14053-1-aneesh.kumar@linux.ibm.com>

On Tue, 2019-02-26 at 04:39:34 UTC, "Aneesh Kumar K.V" wrote:
> After we ALIGN up the address we need to make sure we didn't overflow
> and resulted in zero address. In that case, we need to make sure that
> the returned address is greater than mmap_min_addr.
> 
> This fixes selftest va_128TBswitch --run-hugetlb reporting failures when
> run as non root user for
> 
> mmap(-1, MAP_HUGETLB)
> 
> The bug is that a non-root user requesting address -1 will be given address 0
> which will then fail, whereas they should have been given something else that
> would have succeeded.
> 
> We also avoid the first mmap(-1, MAP_HUGETLB) returning NULL address as mmap address
> with this change. So we think this is not a security issue, because it only affects
> whether we choose an address below mmap_min_addr, not whether we
> actually allow that address to be mapped. ie. there are existing capability
> checks to prevent a user mapping below mmap_min_addr and those will still be
> honoured even without this fix.
> 
> Fixes: 484837601d4d ("powerpc/mm: Add radix support for hugetlb")
> Reviewed-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/5330367fa300742a97e20e953b1f77f4

cheers

^ permalink raw reply

* Re: [v2, 1/6] powerpc: sstep: Add support for maddhd, maddhdu, maddld instructions
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Sandipan Das, sandipan; +Cc: naveen.n.rao, linuxppc-dev, paulus, ravi.bangoria
In-Reply-To: <d2a6e5ff179fcb351f79f87e156168d50a3e091c.1550817742.git.sandipan@linux.ibm.com>

On Fri, 2019-02-22 at 06:53:27 UTC, Sandipan Das wrote:
> This adds emulation support for the following integer instructions:
>   * Multiply-Add High Doubleword (maddhd)
>   * Multiply-Add High Doubleword Unsigned (maddhdu)
>   * Multiply-Add Low Doubleword (maddld)
> 
> As suggested by Michael, this uses a raw .long for specifying the
> instruction word when using inline assembly to retain compatibility
> with older binutils.
> 
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/930d6288a26787d2e7f633705434171a

cheers

^ permalink raw reply

* Re: [kernel, v2] powerpc/powernv/ioda: Fix locked_vm counting for memory used by IOMMU tables
From: Michael Ellerman @ 2019-02-28  9:21 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson
In-Reply-To: <20190213033818.51452-1-aik@ozlabs.ru>

On Wed, 2019-02-13 at 03:38:18 UTC, Alexey Kardashevskiy wrote:
> We store 2 multilevel tables in iommu_table - one for the hardware and
> one with the corresponding userspace addresses. Before allocating
> the tables, the iommu_table_group_ops::get_table_size() hook returns
> the combined size of the two and VFIO SPAPR TCE IOMMU driver adjusts
> the locked_vm counter correctly. When the table is actually allocated,
> the amount of allocated memory is stored in iommu_table::it_allocated_size
> and used to decrement the locked_vm counter when we release the memory
> used by the table; .get_table_size() and .create_table() calculate it
> independently but the result is expected to be the same.
> 
> However the allocator does not add the userspace table size to
> .it_allocated_size so when we destroy the table because of VFIO PCI
> unplug (i.e. VFIO container is gone but the userspace keeps running),
> we decrement locked_vm by just a half of size of memory we are releasing.
> 
> To make things worse, since we enabled on-demain allocation of
> indirect levels, it_allocated_size contains only the amount of memory
> actually allocated at the table creation time which can just be
> a fraction. It is not a problem with incrementing locked_vm (as
> get_table_size() value is used) but it is with decrementing.
> 
> As the result, we leak locked_vm and may not be able to allocate more
> IOMMU tables after few iterations of hotplug/unplug.
> 
> This sets it_allocated_size in the pnv_pci_ioda2_ops::create_table()
> hook to what pnv_pci_ioda2_get_table_size() returns so from now on
> we have a single place which calculates the maximum memory a table can
> occupy. The original meaning of it_allocated_size is somewhat lost now
> though.
> 
> We do not ditch it_allocated_size whatsoever here and we do not call
> get_table_size() from vfio_iommu_spapr_tce.c when decrementing locked_vm
> as we may have multiple IOMMU groups per container and even though they
> all are supposed to have the same get_table_size() implementation,
> there is a small chance for failure or confusion.
> 
> Fixes: 090bad39b "powerpc/powernv: Add indirect levels to it_userspace"
> Fixes: a68bd1267 "powerpc/powernv/ioda: Allocate indirect TCE levels on demand"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/11f5acce2fa43b015a8120fa7620fa4e

cheers

^ permalink raw reply

* [PATCH] powerpc: remove dead code in head_fsl_booke.S
From: Jason Yan @ 2019-02-28  8:31 UTC (permalink / raw)
  To: benh, paulus, mpe, diana.craciun, christophe.leroy, linuxppc-dev
  Cc: Jason Yan

This code is dead. Just remove it.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 arch/powerpc/kernel/head_fsl_booke.S | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 2386ce2a9c6e..b8450f017d85 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -194,13 +194,6 @@ set_ivor:
 #endif
 	mtspr	SPRN_MAS4, r2
 
-#if 0
-	/* Enable DOZE */
-	mfspr	r2,SPRN_HID0
-	oris	r2,r2,HID0_DOZE@h
-	mtspr	SPRN_HID0, r2
-#endif
-
 #if !defined(CONFIG_BDI_SWITCH)
 	/*
 	 * The Abatron BDI JTAG debugger does not tolerate others
-- 
2.14.5


^ permalink raw reply related

* [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
From: Aneesh Kumar K.V @ 2019-02-28  8:35 UTC (permalink / raw)
  To: akpm, Kirill A . Shutemov, Jan Kara, mpe, Ross Zwisler,
	Oliver O'Halloran
  Cc: linux-mm, linuxppc-dev, linux-kernel, Aneesh Kumar K.V
In-Reply-To: <20190228083522.8189-1-aneesh.kumar@linux.ibm.com>

Add a flag to indicate the ability to do huge page dax mapping. On architecture
like ppc64, the hypervisor can disable huge page support in the guest. In
such a case, we should not enable huge page dax mapping. This patch adds
a flag which the architecture code will update to indicate huge page
dax mapping support.

Architectures mostly do transparent_hugepage_flag = 0; if they can't
do hugepages. That also takes care of disabling dax hugepage mapping
with this change.

Without this patch we get the below error with kvm on ppc64.

[  118.849975] lpar: Failed hash pte insert with error -4

NOTE: The patch also use

echo never > /sys/kernel/mm/transparent_hugepage/enabled
to disable dax huge page mapping.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
TODO:
* Add Fixes: tag

 include/linux/huge_mm.h | 4 +++-
 mm/huge_memory.c        | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 381e872bfde0..01ad5258545e 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
 			pud_t *pud, pfn_t pfn, bool write);
 enum transparent_hugepage_flag {
 	TRANSPARENT_HUGEPAGE_FLAG,
+	TRANSPARENT_HUGEPAGE_DAX_FLAG,
 	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
 	TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
 	TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG,
@@ -111,7 +112,8 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
 	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
 		return true;
 
-	if (vma_is_dax(vma))
+	if (vma_is_dax(vma) &&
+	    (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_DAX_FLAG)))
 		return true;
 
 	if (transparent_hugepage_flags &
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index faf357eaf0ce..43d742fe0341 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -53,6 +53,7 @@ unsigned long transparent_hugepage_flags __read_mostly =
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE_MADVISE
 	(1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG)|
 #endif
+	(1 << TRANSPARENT_HUGEPAGE_DAX_FLAG) |
 	(1<<TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG)|
 	(1<<TRANSPARENT_HUGEPAGE_DEFRAG_KHUGEPAGED_FLAG)|
 	(1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG);
@@ -475,6 +476,8 @@ static int __init setup_transparent_hugepage(char *str)
 			  &transparent_hugepage_flags);
 		clear_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
 			  &transparent_hugepage_flags);
+		clear_bit(TRANSPARENT_HUGEPAGE_DAX_FLAG,
+			  &transparent_hugepage_flags);
 		ret = 1;
 	}
 out:
@@ -753,6 +756,7 @@ static void insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
 	spinlock_t *ptl;
 
 	ptl = pmd_lock(mm, pmd);
+	/* should we check for none here again? */
 	entry = pmd_mkhuge(pfn_t_pmd(pfn, prot));
 	if (pfn_t_devmap(pfn))
 		entry = pmd_mkdevmap(entry);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/2] fs/dax: deposit pagetable even when installing zero page
From: Aneesh Kumar K.V @ 2019-02-28  8:35 UTC (permalink / raw)
  To: akpm, Kirill A . Shutemov, Jan Kara, mpe, Ross Zwisler,
	Oliver O'Halloran
  Cc: linux-mm, linuxppc-dev, linux-kernel, Aneesh Kumar K.V

Architectures like ppc64 use the deposited page table to store hardware
page table slot information. Make sure we deposit a page table when
using zero page at the pmd level for hash.

Without this we hit

Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc000000000082a74
Oops: Kernel access of bad area, sig: 11 [#1]
....

NIP [c000000000082a74] __hash_page_thp+0x224/0x5b0
LR [c0000000000829a4] __hash_page_thp+0x154/0x5b0
Call Trace:
 hash_page_mm+0x43c/0x740
 do_hash_page+0x2c/0x3c
 copy_from_iter_flushcache+0xa4/0x4a0
 pmem_copy_from_iter+0x2c/0x50 [nd_pmem]
 dax_copy_from_iter+0x40/0x70
 dax_iomap_actor+0x134/0x360
 iomap_apply+0xfc/0x1b0
 dax_iomap_rw+0xac/0x130
 ext4_file_write_iter+0x254/0x460 [ext4]
 __vfs_write+0x120/0x1e0
 vfs_write+0xd8/0x220
 SyS_write+0x6c/0x110
 system_call+0x3c/0x130

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
TODO:
* Add fixes tag 

 fs/dax.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/dax.c b/fs/dax.c
index 6959837cc465..01bfb2ac34f9 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -33,6 +33,7 @@
 #include <linux/sizes.h>
 #include <linux/mmu_notifier.h>
 #include <linux/iomap.h>
+#include <asm/pgalloc.h>
 #include "internal.h"
 
 #define CREATE_TRACE_POINTS
@@ -1410,7 +1411,9 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
 {
 	struct address_space *mapping = vmf->vma->vm_file->f_mapping;
 	unsigned long pmd_addr = vmf->address & PMD_MASK;
+	struct vm_area_struct *vma = vmf->vma;
 	struct inode *inode = mapping->host;
+	pgtable_t pgtable = NULL;
 	struct page *zero_page;
 	spinlock_t *ptl;
 	pmd_t pmd_entry;
@@ -1425,12 +1428,22 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
 	*entry = dax_insert_entry(xas, mapping, vmf, *entry, pfn,
 			DAX_PMD | DAX_ZERO_PAGE, false);
 
+	if (arch_needs_pgtable_deposit()) {
+		pgtable = pte_alloc_one(vma->vm_mm);
+		if (!pgtable)
+			return VM_FAULT_OOM;
+	}
+
 	ptl = pmd_lock(vmf->vma->vm_mm, vmf->pmd);
 	if (!pmd_none(*(vmf->pmd))) {
 		spin_unlock(ptl);
 		goto fallback;
 	}
 
+	if (pgtable) {
+		pgtable_trans_huge_deposit(vma->vm_mm, vmf->pmd, pgtable);
+		mm_inc_nr_ptes(vma->vm_mm);
+	}
 	pmd_entry = mk_pmd(zero_page, vmf->vma->vm_page_prot);
 	pmd_entry = pmd_mkhuge(pmd_entry);
 	set_pmd_at(vmf->vma->vm_mm, pmd_addr, vmf->pmd, pmd_entry);
@@ -1439,6 +1452,8 @@ static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
 	return VM_FAULT_NOPAGE;
 
 fallback:
+	if (pgtable)
+		pte_free(vma->vm_mm, pgtable);
 	trace_dax_pmd_load_hole_fallback(inode, vmf, zero_page, *entry);
 	return VM_FAULT_FALLBACK;
 }
-- 
2.20.1


^ permalink raw reply related

* Re: PROBLEM: monotonic clock going backwards on ppc64
From: Mathieu Malaterre @ 2019-02-28  7:04 UTC (permalink / raw)
  To: Jakub Drnec; +Cc: linuxppc-dev
In-Reply-To: <HaC.ZfES.62bwlnvAvMP.1STMMj@seznam.cz>

On Tue, Feb 26, 2019 at 9:36 PM Jakub Drnec <jaydee@email.cz> wrote:
>
> Hi all,
>
> I think I observed a potential problem, is this the correct place to report it? (CC me, not on list)
>
> [1.] One line summary: monotonic clock can be made to decrease on ppc64
> [2.] Full description:
> Setting the realtime clock can sometimes make the monotonic clock go back by over a hundred years.
> Decreasing the realtime clock across the y2k38 threshold is one reliable way to reproduce.
> Allegedly this can also happen just by running ntpd, I have not managed to reproduce that other
> than booting with rtc at >2038 and then running ntp.

Isn't it the expected behavior. Here is what I see for powermac:

$ git show 22db552b50fa
...
    This changes the logic to cast to an unsigned 32-bit number first for
    the Macintosh time and then convert that to the Unix time, which then
    gives us a time in the documented 1904..2040 year range. I decided not
    to use the longer 1970..2106 range that other drivers use, for
    consistency with the literal interpretation of the register, but that
    could be easily changed if we decide we want to support any Mac after
    2040.
...

> When this happens, anything with timers (e.g. openjdk) breaks rather badly.
>
> [3.] Keywords: gettimeofday, ppc64, vdso
> [4.] Kernel information
> [4.1.] Kernel version: any (tested on 4.19)
> [4.2.] Kernel .config file: any
> [5.] Most recent kernel version which did not have the bug: not a regression
> [6.] Output of Oops..: not applicable
> [7.] Example program which triggers the problem
> --- testcase.c
> #include <stdio.h>
> #include <time.h>
> #include <stdlib.h>
> #include <unistd.h>
>
> long get_time() {
>   struct timespec tp;
>   if (clock_gettime(CLOCK_MONOTONIC, &tp) != 0) {
>     perror("clock_gettime failed");
>     exit(1);
>   }
>   long result = tp.tv_sec + tp.tv_nsec / 1000000000;
>   return result;
> }
>
> int main() {
>   printf("monitoring monotonic clock...\n");
>   long last = get_time();
>   while(1) {
>     long now = get_time();
>     if (now < last) {
>       printf("clock went backwards by %ld seconds!\n",
>         last - now);
>     }
>     last = now;
>     sleep(1);
>   }
>   return 0;
> }
> ---
> when running
> # date -s 2040-1-1
> # date -s 2037-1-1
> program outputs: clock went backwards by 4294967295 seconds!
>
> [8.] Environment: any ppc64, currently reproducing on qemu-system-ppc64le running debian unstable
> [X.] Other notes, patches, fixes, workarounds:
> The problem seems to be in vDSO code in arch/powerpc/kernel/vdso64/gettimeofday.S.
> (possibly because some values used in the calculation are only 32 bit?)
> Slightly silly workaround:
> nuke the "cmpwi cr1,r3,CLOCK_MONOTONIC" in __kernel_clock_gettime
> Now it always goes through the syscall fallback which does not have the same problem.
>
> Regards,
> Jakub Drnec

^ permalink raw reply

* Re: PROBLEM: monotonic clock going backwards on ppc64
From: Daniel Axtens @ 2019-02-28  6:48 UTC (permalink / raw)
  To: Jakub Drnec, linuxppc-dev
In-Reply-To: <HaC.ZfES.62bwlnvAvMP.1STMMj@seznam.cz>

Hi,

> I think I observed a potential problem, is this the correct place to report it? (CC me, not on list)

Yes

> [1.] One line summary: monotonic clock can be made to decrease on ppc64
> [2.] Full description:
> Setting the realtime clock can sometimes make the monotonic clock go back by over a hundred years.
> Decreasing the realtime clock across the y2k38 threshold is one reliable way to reproduce.
> Allegedly this can also happen just by running ntpd, I have not managed to reproduce that other
> than booting with rtc at >2038 and then running ntp.
> When this happens, anything with timers (e.g. openjdk) breaks rather badly.

I can confirm this on a physical 64bit book3e system and powerpc/next
with the provided reproducer.

> [X.] Other notes, patches, fixes, workarounds:
> The problem seems to be in vDSO code in arch/powerpc/kernel/vdso64/gettimeofday.S.
> (possibly because some values used in the calculation are only 32 bit?)
> Slightly silly workaround: 
> nuke the "cmpwi cr1,r3,CLOCK_MONOTONIC" in __kernel_clock_gettime
> Now it always goes through the syscall fallback which does not have the same problem.

I'll look at the asm but I'm no expert here so we might need to defer to
someone with real expertise. Falling back to the syscall sadly ruins
the performance gain of having a vDSO.

Regards,
Daniel


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox