Re: RCU lockup issues when CONFIG_SOFTLOCKUP_DETECTOR=n - any one else seeing this?

[Paul E. McKenney]

On Sun, Aug 20, 2017 at 02:45:53PM +1000, Nicholas Piggin wrote:
> On Wed, 16 Aug 2017 09:27:31 -0700
> "Paul E. McKenney"  wrote:
> > On Wed, Aug 16, 2017 at 05:56:17AM -0700, Paul E. McKenney wrote:
> > 
> > Thomas, John, am I misinterpreting the timer trace event messages?
> 
> So I did some digging, and what you find is that rcu_sched seems to do a
> simple scheudle_timeout(1) and just goes out to lunch for many seconds.
> The process_timeout timer never fires (when it finally does wake after
> one of these events, it usually removes the timer with del_timer_sync).
> 
> So this patch seems to fix it. Testing, comments welcome.

Fired up some tests, which should complete in about 12 hours.

Here is hoping!  ;-)

Thanx, Paul

Re: RCU lockup issues when CONFIG_SOFTLOCKUP_DETECTOR=n - any one else seeing this?

[David Miller]

From: Nicholas Piggin 
Date: Sun, 20 Aug 2017 14:45:53 +1000

> [PATCH] timers: Fix excessive granularity of new timers after a nohz idle

I just booted into this on my sparc64 box, let's see if it does the
trick :-)

Re: RCU lockup issues when CONFIG_SOFTLOCKUP_DETECTOR=n - any one else seeing this?

[Nicholas Piggin]

On Wed, 16 Aug 2017 09:27:31 -0700
"Paul E. McKenney"  wrote:
> On Wed, Aug 16, 2017 at 05:56:17AM -0700, Paul E. McKenney wrote:
> 
> Thomas, John, am I misinterpreting the timer trace event messages?

So I did some digging, and what you find is that rcu_sched seems to do a
simple scheudle_timeout(1) and just goes out to lunch for many seconds.
The process_timeout timer never fires (when it finally does wake after
one of these events, it usually removes the timer with del_timer_sync).

So this patch seems to fix it. Testing, comments welcome.

Thanks,
Nick

[PATCH] timers: Fix excessive granularity of new timers after a nohz idle

When a timer base is idle, it is forwarded when a new timer is added to
ensure that granularity does not become excessive. When not idle, the
timer tick is expected to increment the base.

However there is a window after a timer is restarted from nohz, when it
is marked not-idle, and before the timer tick on this CPU, where a timer
may be added on an ancient base that does not get forwarded (beacause
the timer appears not-idle).

This results in excessive granularity. So much so that a 1 jiffy timeout
has blown out to 10s of seconds and triggered the RCU stall warning
detector.

Fix this by always forwarding the base when adding a new timer if it is
more than 1 jiffy behind. Another approach I looked at first was to note
if the base was idle but not yet run or forwarded, however this just
seemed to add more branches and complexity when it seems we can just
cover it with this test.

Also add a comment noting a case where we could get an unexpectedly
large granularity for a timer. I debugged this problem by adding
warnings for such cases, but it seems we can't add them in general due
to this corner case.

Signed-off-by: Nicholas Piggin 
---
 kernel/time/timer.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 8f5d1bf18854..8f69b3105b8f 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -859,10 +859,10 @@ static inline void forward_timer_base(struct timer_base 
*base)
unsigned long jnow = READ_ONCE(jiffies);
 
/*
-* We only forward the base when it's idle and we have a delta between
-* base clock and jiffies.
+* We only forward the base when we have a delta between base clock
+* and jiffies. In the common case, run_timers will take care of it.
 */
-   if (!base->is_idle || (long) (jnow - base->clk) < 2)
+   if ((long) (jnow - base->clk) < 2)
return;
 
/*
@@ -938,6 +938,13 @@ __mod_timer(struct timer_list *timer, unsigned long 
expires, bool pending_only)
 * same array bucket then just return:
 */
if (timer_pending(timer)) {
+   /*
+* The downside of this optimization is that it can result in
+* larger granularity than you would get from adding a new
+* timer with this expiry. Would a timer flag for networking
+* be appropriate, then we can try to keep expiry of general
+* timers within ~1/8th of their interval?
+*/
if (timer->expires == expires)
return 1;
 
-- 
2.13.3

Re: [RFC v6 35/62] powerpc: Deliver SEGV signal on pkey violation

[Eric W. Biederman]

Ram Pai  writes:

> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index d4e545d..fe1e7c7 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -247,6 +248,15 @@ void user_single_step_siginfo(struct task_struct *tsk,
>   info->si_addr = (void __user *)regs->nip;
>  }
>  
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +static void fill_sig_info_pkey(int si_code, siginfo_t *info, unsigned long 
> addr)
> +{
> + if (si_code != SEGV_PKUERR)
> + return;

Given that SEGV_PKUERR is a signal specific si_code this test is
insufficient to detect an pkey error.  You also need to check
that signr == SIGSEGV

> + info->si_pkey = get_paca()->paca_pkey;
> +}
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> +
>  void _exception(int signr, struct pt_regs *regs, int code, unsigned long 
> addr)
>  {
>   siginfo_t info;
> @@ -274,6 +284,11 @@ void _exception(int signr, struct pt_regs *regs, int 
> code, unsigned long addr)
>   info.si_signo = signr;
>   info.si_code = code;
>   info.si_addr = (void __user *) addr;
> +
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> + fill_sig_info_pkey(code, , addr);
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> +
>   force_sig_info(signr, , current);
>  }

Eric

Re: [RFT PATCH] tpm: ibmvtpm: simplify crq initialization and document crq format

[Jarkko Sakkinen]

Applied. I'll put this to 4.14 PR.

/Jarkko

On Sat, Aug 19, 2017 at 08:18:59PM +0300, Jarkko Sakkinen wrote:
> Ugh. I'll apply this. My apologies.
> 
> /Jarkko
> 
> On Fri, Aug 18, 2017 at 12:21:45AM +0200, msuchanek wrote:
> > ping?
> > 
> > On Fri, 24 Feb 2017 20:35:16 +0100
> > Michal Suchanek  wrote:
> > 
> > > The crq is passed in registers and is the same on BE and LE hosts.
> > > However, current implementation allocates a structure on-stack to
> > > represent the crq, initializes the members swapping them to BE, and
> > > loads the structure swapping it from BE. This is pointless and causes
> > > GCC warnings about ununitialized members. Get rid of the structure and
> > > the warnings.
> > > 
> > > Signed-off-by: Michal Suchanek 
> > > Reviewed-by: Jarkko Sakkinen 
> > > ---
> > > v2
> > > 
> > > fix typos and spelling in comments
> > > ---
> > >  drivers/char/tpm/tpm_ibmvtpm.c | 96
> > > ++ 1 file changed, 60
> > > insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c
> > > b/drivers/char/tpm/tpm_ibmvtpm.c index 1b9d61ffe991..89027339d55f
> > > 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c
> > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> > > @@ -39,19 +39,63 @@ static struct vio_device_id
> > > tpm_ibmvtpm_device_table[] = { MODULE_DEVICE_TABLE(vio,
> > > tpm_ibmvtpm_device_table); 
> > >  /**
> > > + *
> > > + * ibmvtpm_send_crq_word - Send a CRQ request
> > > + * @vdev:vio device struct
> > > + * @w1:  pre-constructed first word of tpm crq (second
> > > word is reserved)
> > > + *
> > > + * Return:
> > > + *   0 - Success
> > > + *   Non-zero - Failure
> > > + */
> > > +static int ibmvtpm_send_crq_word(struct vio_dev *vdev, u64 w1)
> > > +{
> > > + return plpar_hcall_norets(H_SEND_CRQ, vdev->unit_address,
> > > w1, 0); +}
> > > +
> > > +/**
> > > + *
> > >   * ibmvtpm_send_crq - Send a CRQ request
> > >   *
> > >   * @vdev:vio device struct
> > > - * @w1:  first word
> > > - * @w2:  second word
> > > + * @valid:   Valid field
> > > + * @msg: Type field
> > > + * @len: Length field
> > > + * @data:Data field
> > > + *
> > > + * The ibmvtpm crq is defined as follows:
> > > + *
> > > + * Byte  |   0   |   1   |   2   |   3   |   4   |   5   |   6   |
> > > 7
> > > + *
> > > ---
> > > + * Word0 | Valid | Type  | Length|  Data
> > > + *
> > > ---
> > > + * Word1 |Reserved
> > > + *
> > > ---
> > > + *
> > > + * Which matches the following structure (on bigendian host):
> > > + *
> > > + * struct ibmvtpm_crq {
> > > + * u8 valid;
> > > + * u8 msg;
> > > + * __be16 len;
> > > + * __be32 data;
> > > + * __be64 reserved;
> > > + * } __attribute__((packed, aligned(8)));
> > > + *
> > > + * However, the value is passed in a register so just compute the
> > > numeric value
> > > + * to load into the register avoiding byteswap altogether. Endian
> > > only affects
> > > + * memory loads and stores - registers are internally represented
> > > the same. *
> > >   * Return:
> > > - *   0 -Sucess
> > > + *   0 (H_SUCCESS) - Success
> > >   *   Non-zero - Failure
> > >   */
> > > -static int ibmvtpm_send_crq(struct vio_dev *vdev, u64 w1, u64 w2)
> > > +static int ibmvtpm_send_crq(struct vio_dev *vdev,
> > > + u8 valid, u8 msg, u16 len, u32 data)
> > >  {
> > > - return plpar_hcall_norets(H_SEND_CRQ, vdev->unit_address,
> > > w1, w2);
> > > + u64 w1 = ((u64)valid << 56) | ((u64)msg << 48) | ((u64)len
> > > << 32) |
> > > + (u64)data;
> > > + return ibmvtpm_send_crq_word(vdev, w1);
> > >  }
> > >  
> > >  /**
> > > @@ -109,8 +153,6 @@ static int tpm_ibmvtpm_recv(struct tpm_chip
> > > *chip, u8 *buf, size_t count) static int tpm_ibmvtpm_send(struct
> > > tpm_chip *chip, u8 *buf, size_t count) {
> > >   struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
> > > - struct ibmvtpm_crq crq;
> > > - __be64 *word = (__be64 *)
> > >   int rc, sig;
> > >  
> > >   if (!ibmvtpm->rtce_buf) {
> > > @@ -137,10 +179,6 @@ static int tpm_ibmvtpm_send(struct tpm_chip
> > > *chip, u8 *buf, size_t count) spin_lock(>rtce_lock);
> > >   ibmvtpm->res_len = 0;
> > >   memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count);
> > > - crq.valid = (u8)IBMVTPM_VALID_CMD;
> > > - crq.msg = (u8)VTPM_TPM_COMMAND;
> > > - crq.len = cpu_to_be16(count);
> > > - crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
> > >  
> > >   /*
> > >* set the processing flag before the Hcall, since we may
> > > get the @@ -148,8 +186,9 @@ static int tpm_ibmvtpm_send(struct
> > > tpm_chip *chip, u8 *buf, size_t count) */
> > >   ibmvtpm->tpm_processing_cmd = true;

Re: [PATCH] tpm: vtpm: constify vio_device_id

[Jarkko Sakkinen]

On Fri, Aug 18, 2017 at 02:31:56PM -0600, Jason Gunthorpe wrote:
> On Fri, Aug 18, 2017 at 09:32:46PM +1000, Michael Ellerman wrote:
> 
> > >>  drivers/char/tpm/tpm_ibmvtpm.c | 2 +-
> > 
> > Who merges changes for this driver? I assume it's Jarkko?
> 
> Yes
> 
> Jason

Applied.

/Jarkko

Re: [RFT PATCH] tpm: ibmvtpm: simplify crq initialization and document crq format

[Jarkko Sakkinen]

Ugh. I'll apply this. My apologies.

/Jarkko

On Fri, Aug 18, 2017 at 12:21:45AM +0200, msuchanek wrote:
> ping?
> 
> On Fri, 24 Feb 2017 20:35:16 +0100
> Michal Suchanek  wrote:
> 
> > The crq is passed in registers and is the same on BE and LE hosts.
> > However, current implementation allocates a structure on-stack to
> > represent the crq, initializes the members swapping them to BE, and
> > loads the structure swapping it from BE. This is pointless and causes
> > GCC warnings about ununitialized members. Get rid of the structure and
> > the warnings.
> > 
> > Signed-off-by: Michal Suchanek 
> > Reviewed-by: Jarkko Sakkinen 
> > ---
> > v2
> > 
> > fix typos and spelling in comments
> > ---
> >  drivers/char/tpm/tpm_ibmvtpm.c | 96
> > ++ 1 file changed, 60
> > insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c
> > b/drivers/char/tpm/tpm_ibmvtpm.c index 1b9d61ffe991..89027339d55f
> > 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c
> > +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> > @@ -39,19 +39,63 @@ static struct vio_device_id
> > tpm_ibmvtpm_device_table[] = { MODULE_DEVICE_TABLE(vio,
> > tpm_ibmvtpm_device_table); 
> >  /**
> > + *
> > + * ibmvtpm_send_crq_word - Send a CRQ request
> > + * @vdev:  vio device struct
> > + * @w1:pre-constructed first word of tpm crq (second
> > word is reserved)
> > + *
> > + * Return:
> > + * 0 - Success
> > + * Non-zero - Failure
> > + */
> > +static int ibmvtpm_send_crq_word(struct vio_dev *vdev, u64 w1)
> > +{
> > +   return plpar_hcall_norets(H_SEND_CRQ, vdev->unit_address,
> > w1, 0); +}
> > +
> > +/**
> > + *
> >   * ibmvtpm_send_crq - Send a CRQ request
> >   *
> >   * @vdev:  vio device struct
> > - * @w1:first word
> > - * @w2:second word
> > + * @valid: Valid field
> > + * @msg:   Type field
> > + * @len:   Length field
> > + * @data:  Data field
> > + *
> > + * The ibmvtpm crq is defined as follows:
> > + *
> > + * Byte  |   0   |   1   |   2   |   3   |   4   |   5   |   6   |
> > 7
> > + *
> > ---
> > + * Word0 | Valid | Type  | Length|  Data
> > + *
> > ---
> > + * Word1 |Reserved
> > + *
> > ---
> > + *
> > + * Which matches the following structure (on bigendian host):
> > + *
> > + * struct ibmvtpm_crq {
> > + * u8 valid;
> > + * u8 msg;
> > + * __be16 len;
> > + * __be32 data;
> > + * __be64 reserved;
> > + * } __attribute__((packed, aligned(8)));
> > + *
> > + * However, the value is passed in a register so just compute the
> > numeric value
> > + * to load into the register avoiding byteswap altogether. Endian
> > only affects
> > + * memory loads and stores - registers are internally represented
> > the same. *
> >   * Return:
> > - * 0 -Sucess
> > + * 0 (H_SUCCESS) - Success
> >   * Non-zero - Failure
> >   */
> > -static int ibmvtpm_send_crq(struct vio_dev *vdev, u64 w1, u64 w2)
> > +static int ibmvtpm_send_crq(struct vio_dev *vdev,
> > +   u8 valid, u8 msg, u16 len, u32 data)
> >  {
> > -   return plpar_hcall_norets(H_SEND_CRQ, vdev->unit_address,
> > w1, w2);
> > +   u64 w1 = ((u64)valid << 56) | ((u64)msg << 48) | ((u64)len
> > << 32) |
> > +   (u64)data;
> > +   return ibmvtpm_send_crq_word(vdev, w1);
> >  }
> >  
> >  /**
> > @@ -109,8 +153,6 @@ static int tpm_ibmvtpm_recv(struct tpm_chip
> > *chip, u8 *buf, size_t count) static int tpm_ibmvtpm_send(struct
> > tpm_chip *chip, u8 *buf, size_t count) {
> > struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(>dev);
> > -   struct ibmvtpm_crq crq;
> > -   __be64 *word = (__be64 *)
> > int rc, sig;
> >  
> > if (!ibmvtpm->rtce_buf) {
> > @@ -137,10 +179,6 @@ static int tpm_ibmvtpm_send(struct tpm_chip
> > *chip, u8 *buf, size_t count) spin_lock(>rtce_lock);
> > ibmvtpm->res_len = 0;
> > memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count);
> > -   crq.valid = (u8)IBMVTPM_VALID_CMD;
> > -   crq.msg = (u8)VTPM_TPM_COMMAND;
> > -   crq.len = cpu_to_be16(count);
> > -   crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
> >  
> > /*
> >  * set the processing flag before the Hcall, since we may
> > get the @@ -148,8 +186,9 @@ static int tpm_ibmvtpm_send(struct
> > tpm_chip *chip, u8 *buf, size_t count) */
> > ibmvtpm->tpm_processing_cmd = true;
> >  
> > -   rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
> > - be64_to_cpu(word[1]));
> > +   rc = ibmvtpm_send_crq(ibmvtpm->vdev,
> > +   IBMVTPM_VALID_CMD, VTPM_TPM_COMMAND,
> > +   count, ibmvtpm->rtce_dma_handle);
> > if (rc != H_SUCCESS) {
> > dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send 

Re: [PATCH v2 1/1] Split VGA default device handler out of VGA arbiter

[Bjorn Helgaas]

On Thu, Aug 17, 2017 at 09:30:28PM +1000, Daniel Axtens wrote:
> A system without PCI legacy resources (e.g. ARM64)

Can you be a little more specific about what you mean by "a system
without PCI legacy resources"?  I'm not sure what the connection with
ARM64 as an architecture is.

My understanding is that "legacy resources" are addresses that are
hard-wired into a device and not discoverable or configurable via a
BAR [1].  For example,

  VGA devices (see [2])
mem 0xa-0xb(frame buffer)
io  0x3b0-0x3bb(plus ISA aliases)
io  0x3c0-0x3df(plus ISA aliases)

  IDE devices in compatibility mode (see [3])
io  0x170-0x177(secondary command)
io  0x1f0-0x1f7(primary command)
io  0x376  (secondary control)
io  0x3f6  (primary control)

The meaning of these resources in the PCI/PCIe domain is defined by
the PCI specs, not the platform arch specs.

So if ARM64 doesn't have these PCI legacy resources, does that mean an
ARM64 host bridge cannot generate these legacy addresses on PCI?  That
is, there's no host bridge window that maps to those PCI addresses?
That seems like a curious restriction on host bridges, but I guess it
would be possible.

I think you're fixing an issue related to the HiSilicon [19e5:1610]
PCI-to-PCI bridge, which doesn't support VGA Enable (I first thought
the bridge was broken, but per [4], I think it is legal).  With VGA
Enable not being supported, a downstream VGA device will not work if
it depends on the legacy VGA resources.  But this is merely related to
one PCI-to-PCI bridge in the system; it's not a question of the system
as a whole or the CPU architecture.

Maybe I'm reading too much into the "ARM64 has no PCI legacy
resources" idea and the point here is simply that the VGA arbiter
previously would not mark a VGA device as default if it found that the
legacy resources are not routed to it, e.g., because an upstream
bridge doesn't support VGA ENABLE?

That's "safe" because if the arbiter selects a default VGA device that
does not receive accesses to the legacy VGA resources, that device may
not work.  For example, it looks like vga16fb.c would not work because
it depends on VGA_FB_PHYS at 0xA.  A native driver certainly
*could* work, but that depends on whether the specific device and
driver require the legacy resources.

IIUC, part of what this patch does is relax this so we can set a
default "VGA device" that doesn't receive accesses to legacy VGA
resources.  If so, it's probably worth mentioning somehow that some
VGA things, e.g., vga16fb.c, may not work with this default device.

[1] PCI spec r3.0, sec G
[2] PCI-to-PCI bridge spec r1.2, sec 3.2.5.18, sec 12.1.1
[3] PCI IDE Controller spec, r1.0, sec 2.1
[4] PCI-to-PCI bridge spec r1.2, sec 4.5

> may find that no
> default/boot VGA device has been marked, because the VGA arbiter
> checks for legacy resource decoding before marking a card as default.
> 
> Split the small bit of code that does default VGA handling out from
> the arbiter. Add a Kconfig option to allow the kernel to be built
> with just the default handling, or the arbiter and default handling.
> 
> Add handling for devices that should be marked as default but aren't
> handled by the vga arbiter by adding a late initcall and a class
> enable hook. If there is no default from vgaarb then the first card
> that is enabled, has a driver bound, and can decode memory or I/O
> will be marked as default.

> + * vga_default_device - return the default VGA device
> + *
> + * This can be defined by the platform. The default implementation
> + * is rather dumb and will probably only work properly on single
> + * vga card setups and/or x86 platforms.

In what sense can this be defined by the platform?  I guess this
comment isn't new and was just moved, but I don't see any arch hook or
obvious way to do a platform-specific definition.

[PATCH] qe/ic: make irq_chip const

[Bhumika Goyal]

Make this const as it is only used in a copy operation.
Done using Coccinelle.

Signed-off-by: Bhumika Goyal 
---
 drivers/soc/fsl/qe/qe_ic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index ec2ca86..00f48d5 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -238,7 +238,7 @@ static void qe_ic_mask_irq(struct irq_data *d)
raw_spin_unlock_irqrestore(_ic_lock, flags);
 }
 
-static struct irq_chip qe_ic_irq_chip = {
+static const struct irq_chip qe_ic_irq_chip = {
.name = "QEIC",
.irq_unmask = qe_ic_unmask_irq,
.irq_mask = qe_ic_mask_irq,
-- 
1.9.1

Re: [PATCH 5/5] Use __func__ instead of function name

[김동현]

I guess below code would be better idea to gather more debug information.
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 643bba7..0aae785 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -141,6 +141,11 @@ static int tpm_ibmvtpm_send(struct tpm_chip
*chip, u8 *buf, size_t count)
return -EIO;
}

+   if (!buf) {
+   dev_err(ibmvtpm->dev, "%s buf null at %d \n",__func__,
rc, __LINE__);
+   return 0;
+   }
+
spin_lock(>rtce_lock);
memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count);
crq.valid = (u8)IBMVTPM_VALID_CMD;
@@ -150,8 +155,9 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip,
u8 *buf, size_t count)

rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
  be64_to_cpu(word[1]));
+
if (rc != H_SUCCESS) {
-   dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
+   dev_err(ibmvtpm->dev, "%s failed rc=%d at %d
\n",__func__, rc, __LINE__);
rc = 0;
} else
rc = count;

2017-07-29 16:24 GMT+09:00 SZ Lin :
> Fix following checkpatch.pl warning:
> WARNING: Prefer using '"%s...", __func__' to using
> the function's name, in a string
>
> Signed-off-by: SZ Lin 
> ---
>  drivers/char/tpm/tpm_ibmvtpm.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index e75a674b44ac..2d33acc43e25 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -151,7 +151,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 
> *buf, size_t count)
> rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
>   be64_to_cpu(word[1]));
> if (rc != H_SUCCESS) {
> -   dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
> +   dev_err(ibmvtpm->dev, "%s failed rc=%d\n", __func__, rc);
> rc = 0;
> ibmvtpm->tpm_processing_cmd = false;
> } else
> @@ -193,7 +193,7 @@ static int ibmvtpm_crq_get_rtce_size(struct ibmvtpm_dev 
> *ibmvtpm)
>   cpu_to_be64(buf[1]));
> if (rc != H_SUCCESS)
> dev_err(ibmvtpm->dev,
> -   "ibmvtpm_crq_get_rtce_size failed rc=%d\n", rc);
> +   "%s failed rc=%d\n", __func__, rc);
>
> return rc;
>  }
> @@ -221,7 +221,7 @@ static int ibmvtpm_crq_get_version(struct ibmvtpm_dev 
> *ibmvtpm)
>   cpu_to_be64(buf[1]));
> if (rc != H_SUCCESS)
> dev_err(ibmvtpm->dev,
> -   "ibmvtpm_crq_get_version failed rc=%d\n", rc);
> +   "%s failed rc=%d\n", __func__, rc);
>
> return rc;
>  }
> @@ -241,7 +241,7 @@ static int ibmvtpm_crq_send_init_complete(struct 
> ibmvtpm_dev *ibmvtpm)
> rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_COMP_CMD, 0);
> if (rc != H_SUCCESS)
> dev_err(ibmvtpm->dev,
> -   "ibmvtpm_crq_send_init_complete failed rc=%d\n", rc);
> +   "%s rc=%d\n", __func__, rc);
>
> return rc;
>  }
> @@ -261,7 +261,7 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev 
> *ibmvtpm)
> rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_CMD, 0);
> if (rc != H_SUCCESS)
> dev_err(ibmvtpm->dev,
> -   "ibmvtpm_crq_send_init failed rc=%d\n", rc);
> +   "%s failed rc=%d\n", __func__, rc);
>
> return rc;
>  }
> @@ -351,7 +351,7 @@ static int tpm_ibmvtpm_suspend(struct device *dev)
>   cpu_to_be64(buf[1]));
> if (rc != H_SUCCESS)
> dev_err(ibmvtpm->dev,
> -   "tpm_ibmvtpm_suspend failed rc=%d\n", rc);
> +   "%s failed rc=%d\n", __func__, rc);
>
> return rc;
>  }
> --
> 2.13.3
>

[linux-next][bisected c64e09ce] sysctl command hung indefinitely

[Abdul Haleem]

Hi Michal,

'sysctl -a' command never completes on my PowerPC machine with latest
next kernel (As of next-20170811)

Machine Type : Power8 bare-metal
Kernel version : 4.13.0-rc4-next-20170817
gcc version : 4.8.5


command output
--
$ sysctl -a
[...
vm.hugetlb_shm_group = 0
vm.laptop_mode = 0
vm.legacy_va_layout = 0
vm.lowmem_reserve_ratio = 256   256 32
vm.max_map_count = 65530
vm.min_free_kbytes = 6637
vm.min_slab_ratio = 5
vm.min_unmapped_ratio = 1
vm.mmap_min_addr = 4096
vm.mmap_rnd_bits = 14
vm.mmap_rnd_compat_bits = 7
vm.nr_hugepages = 0
vm.nr_hugepages_mempolicy = 0
vm.nr_overcommit_hugepages = 0
vm.nr_pdflush_threads = 0
vm.numa_zonelist_order = Node
vm.numa_zonelist_order = e
vm.numa_zonelist_order = ode
vm.numa_zonelist_order = 
vm.numa_zonelist_order = de
vm.numa_zonelist_order = Node
vm.numa_zonelist_order = e
vm.numa_zonelist_order = ode
vm.numa_zonelist_order = 
vm.numa_zonelist_order = de
vm.numa_zonelist_order = Node
vm.numa_zonelist_order = e
vm.numa_zonelist_order = ode
vm.numa_zonelist_order = 
vm.numa_zonelist_order = de
vm.numa_zonelist_order = Node
vm.numa_zonelist_order = e
vm.numa_zonelist_order = ode
]

The last string 'vm.numa_zonelist_order = ' keeps flooding the stdout
and command never exit.

A bisection resulted commit c64e09ce mm, page_alloc: rip out
ZONELIST_ORDER_ZONE

Command exits cleanly when the above commit is reverted.

from the commit I see some changes to kernel/sysctl.c

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6648fbb..0d51ec1 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1574,8 +1574,6 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
 #ifdef CONFIG_NUMA
{
.procname   = "numa_zonelist_order",
-   .data   = _zonelist_order,
-   .maxlen = NUMA_ZONELIST_ORDER_LEN,
.mode   = 0644,
.proc_handler   = numa_zonelist_order_handler,
},

does the above change has caused the noise ?

-- 
Regard's

Abdul Haleem
IBM Linux Technology Centre

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Benjamin Herrenschmidt]

On Fri, 2017-08-18 at 15:49 -0700, Ram Pai wrote:
> Coming back to the your main question, "why we need to provide the
> contents of AMR register to the signal handler?" --   the only reason
> i can see is, probably tools like gdb and ptrace may find it useful.
> 
> And since it was suggested that content of IAMR is also useful to the
> application, the value of which cannot be accessed from userspace,
> it may make sense to provide both the contents.

I'd rather you added ptrace calls to obtain & set it. I don't think we
need to touch the signal frame, it's too much of an ABI compatibility
burden.

Cheers,
Ben.