All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND][PATCH 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
From: Bryan O'Donoghue @ 2017-10-04 22:25 UTC (permalink / raw)
  To: richard.leitner, srinivas.kandagatla, axel.lin, ping.bai,
	d.schultz, peng.fan, van.freenix, p.zabel
  Cc: linux-kernel, linux-arm-kernel, pure.logic
In-Reply-To: <1507155921-824-1-git-send-email-pure.logic@nexus-software.ie>

The i.MX7S/D takes the bank address in the CTRLn.ADDR field and the data
value in one of the DATAx {0, 1, 2, 3} register fields. The current write
routine is based on writing the CTRLn.ADDR field and writing a single DATA
register only.

Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 drivers/nvmem/imx-ocotp.c | 71 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index fed76a4..8034937 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -40,7 +40,10 @@
 #define IMX_OCOTP_ADDR_CTRL_SET		0x0004
 #define IMX_OCOTP_ADDR_CTRL_CLR		0x0008
 #define IMX_OCOTP_ADDR_TIMING		0x0010
-#define IMX_OCOTP_ADDR_DATA		0x0020
+#define IMX_OCOTP_ADDR_DATA0		0x0020
+#define IMX_OCOTP_ADDR_DATA1		0x0030
+#define IMX_OCOTP_ADDR_DATA2		0x0040
+#define IMX_OCOTP_ADDR_DATA3		0x0050
 
 #define IMX_OCOTP_BM_CTRL_ADDR		0x0000007F
 #define IMX_OCOTP_BM_CTRL_BUSY		0x00000100
@@ -55,6 +58,8 @@ static DEFINE_MUTEX(ocotp_mutex);
 
 struct octp_params {
 	unsigned int nregs;
+	bool banked;
+	unsigned int regs_per_bank;
 };
 
 struct ocotp_priv {
@@ -176,6 +181,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	u32 timing = 0;
 	u32 ctrl;
 	u8 waddr;
+	u8 word = 0;
 
 	/* allow only writing one complete OTP word at a time */
 	if ((bytes != priv->config->word_size) ||
@@ -228,8 +234,22 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 * description. Both the unlock code and address can be written in the
 	 * same operation.
 	 */
-	/* OTP write/read address specifies one of 128 word address locations */
-	waddr = offset / 4;
+	if (priv->params->banked) {
+		/*
+		 * In banked mode the OTP register bank goes into waddr see
+		 * i.MX 7Solo Applications Processor Reference Manual, Rev. 0.1
+		 * 6.4.3.1
+		 */
+		offset = offset / priv->config->word_size;
+		waddr = offset / priv->params->regs_per_bank;
+		word  = offset & (priv->params->regs_per_bank - 1);
+	} else {
+		/*
+		 * OTP write/read address specifies one of 128 word address
+		 * locations
+		 */
+		waddr = offset / 4;
+	}
 
 	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
 	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
@@ -255,8 +275,41 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 * shift right (with zero fill). This shifting is required to program
 	 * the OTP serially. During the write operation, HW_OCOTP_DATA cannot be
 	 * modified.
+	 * Note: on i.MX7 there are four data fields to write for banked write
+	 *       with the fuse blowing operation only taking place after data0
+	 *	 has been written. This is why data0 must always be the last
+	 *	 register written.
 	 */
-	writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA);
+	if (!priv->params->banked) {
+		writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA0);
+	} else {
+		switch (word) {
+		case 0:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 1:
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 2:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 3:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		}
+	}
 
 	/* 47.4.1.4.5
 	 * Once complete, the controller will clear BUSY. A write request to a
@@ -313,11 +366,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct octp_params params[] = {
-	{ .nregs = 128},
-	{ .nregs = 64},
-	{ .nregs = 128},
-	{ .nregs = 128},
-	{ .nregs = 64},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 64,  .banked = false, .regs_per_bank = 0},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 64,  .banked = true, .regs_per_bank = 4},
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] PCI: aspm: deal with missing root ports in link state handling
From: Bjorn Helgaas @ 2017-10-04 22:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-pci, bhelgaas, graeme.gregory, leif.lindholm,
	daniel.thompson
In-Reply-To: <20171002140840.7767-1-ard.biesheuvel@linaro.org>

On Mon, Oct 02, 2017 at 03:08:40PM +0100, Ard Biesheuvel wrote:
> Even though it is unconventional, some PCIe host implementations omit
> the root ports entirely, and simply consist of a host bridge (which
> is not modeled as a device in the PCI hierarchy) and a link.
> 
> When the downstream device is an endpoint, our current code does not
> seem to mind this unusual configuration. However, when PCIe switches
> are involved, the ASPM code assumes that any downstream switch port
> has a parent, and blindly derefences the bus->parent->self field of
> the pci_dev struct to chain the downstream link state to the link
> state of the root port. Given that the root port is missing, the link
> is not modeled at all, and nor is the link state, and attempting to
> access it results in a NULL pointer dereference and a crash.
> 
> So let's avoid this by allowing the link state chain to terminate at
> the downstream port if no root port exists.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Applied to pci/aspm for v4.15, thanks!

> ---
>  drivers/pci/pcie/aspm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 1dfa10cc566b..0bea8498b5a5 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -802,10 +802,14 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
>  
>  	/*
>  	 * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
> -	 * hierarchies.
> +	 * hierarchies.  Note that some PCIe host implementations omit
> +	 * the root ports entirely, in which case a downstream port on
> +	 * a switch may become the root of the link state chain for all
> +	 * its subordinate endpoints.
>  	 */
>  	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
> -	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE) {
> +	    pci_pcie_type(pdev) == PCI_EXP_TYPE_PCIE_BRIDGE ||
> +	    !pdev->bus->parent->self) {
>  		link->root = link;
>  	} else {
>  		struct pcie_link_state *parent;
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH] drm/gem-cma-helper: Change the level of the allocation failure message
From: Eric Anholt @ 2017-10-04 22:28 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, dri-devel; +Cc: Boris Brezillon
In-Reply-To: <20171004130839.20285-1-boris.brezillon@free-electrons.com>


[-- Attachment #1.1: Type: text/plain, Size: 740 bytes --]

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> drm_gem_cma_create() prints an error message when dma_alloc_wc() fails to
> allocate the amount of memory we requested. This can lead to annoying
> error messages when CMA is only one possible source of memory for the BO
> allocation.
>
> Turn this error message into a debug one and add a __must_check specifier
> to make sure all callers are checking the return value.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

The __must_check seems unnecessary to me -- you're definitely going to
be doing something with the return value, because otherwise why did you
call the object allocate function?

That said, thanks for cleaning up the error message.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: Kernel 4.13 is broken on m68k
From: John Paul Adrian Glaubitz @ 2017-10-04 22:28 UTC (permalink / raw)
  To: Linux/m68k; +Cc: Debian m68k
In-Reply-To: <a10e5dde-b3a6-a525-d403-154c16e60b35@physik.fu-berlin.de>

On 10/04/2017 11:56 PM, John Paul Adrian Glaubitz wrote:
> On 10/04/2017 11:49 PM, John Paul Adrian Glaubitz wrote:
>> I can trigger the problem by switching between 4.12 and 4.13, 4.12 is fin while 4.13 is broken.
> 
> Ok, switching just the initrd to the one from 4.13 while keeping the 4.12 kernel
> image results in the same crash. Maybe the glibc is broken?

I have tracked it down to busybox which was recently upgraded to 1.27.2 in Debian [1].

busybox_1:1.22.0-17:

root@pacman:~/rootskel/src/sbin# ls -l /root/sh
lrwxrwxrwx 1 root root 12 Oct  5 00:18 /root/sh -> /bin/busybox
root@pacman:~/rootskel/src/sbin# /root/sh ./reopen-console-linux
./reopen-console-linux: exec: line 74: /sbin/steal-ctty: not found
root@pacman:~/rootskel/src/sbin#

Despite the error messages, the above behavior is correct.

busybox_1:1.27.2-1:

root@pacman:~/rootskel/src/sbin# /root/sh ./reopen-console-linux
./reopen-console-linux: line 74: syntax error: unterminated quoted string
root@pacman:~/rootskel/src/sbin#

Now we need to find out whether busybox was either miscompiled or
whether the new upstream version is broken. I will recompile the
old version with gcc-7 next.

Adrian

> [1] https://packages.qa.debian.org/b/busybox/news/20170917T171907Z.html

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH] glib-2.0: Remove recommend shared-mime-info for MinGW
From: Alistair Francis @ 2017-10-04 22:29 UTC (permalink / raw)
  To: Burton, Ross; +Cc: OE-core
In-Reply-To: <CAJTo0LZLHN3CbuK3AZPT9sPotT3qcosa5vJVKzSYau3HcqZJnw@mail.gmail.com>

On Wed, Oct 4, 2017 at 3:23 PM, Burton, Ross <ross.burton@intel.com> wrote:
> Already queued for master and probably rocko.

Thanks for that.

Alistair

>
> Ross
>
> On 4 October 2017 at 22:50, Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Mon, Sep 25, 2017 at 3:56 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>> > Commit glib-2.0: recommend shared-mime-info
>> > (51e4f9ca5368af5cefa26f4ca50b282e858982f8) broke compilation when cross
>> > compiling for Windows. This patch removes the recommendation for
>> > shared-mime-info when using MinGW cross compile.
>> >
>> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> > Cc: Ross Burton <ross.burton@intel.com>
>> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
>>
>> Ping!
>>
>> Thanks,
>> Alistair
>>
>> > ---
>> >  meta/recipes-core/glib-2.0/glib.inc | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/meta/recipes-core/glib-2.0/glib.inc
>> > b/meta/recipes-core/glib-2.0/glib.inc
>> > index ce15ccefc3..8434b7dae3 100644
>> > --- a/meta/recipes-core/glib-2.0/glib.inc
>> > +++ b/meta/recipes-core/glib-2.0/glib.inc
>> > @@ -74,6 +74,8 @@ FILES_${PN}-codegen =
>> > "${datadir}/glib-2.0/codegen/*.py \
>> >  FILES_${PN}-utils = "${bindir}/*"
>> >
>> >  RRECOMMENDS_${PN} += "shared-mime-info"
>> > +# When cross compiling for Windows we don't want to include this
>> > +RRECOMMENDS_${PN}_remove_mingw32 = "shared-mime-info"
>> >
>> >  ARM_INSTRUCTION_SET_armv4 = "arm"
>> >  ARM_INSTRUCTION_SET_armv5 = "arm"
>> > --
>> > 2.11.0
>> >
>
>


^ permalink raw reply

* + revert-vmalloc-back-off-when-the-current-task-is-killed.patch added to -mm tree
From: akpm @ 2017-10-04 22:32 UTC (permalink / raw)
  To: hannes, alan, dvyukov, hch, mhocko, mm-commits


The patch titled
     Subject: Revert "vmalloc: back off when the current task is killed"
has been added to the -mm tree.  Its filename is
     revert-vmalloc-back-off-when-the-current-task-is-killed.patch

This patch should soon appear at
    http://ozlabs.org/~akpm/mmots/broken-out/revert-vmalloc-back-off-when-the-current-task-is-killed.patch
and later at
    http://ozlabs.org/~akpm/mmotm/broken-out/revert-vmalloc-back-off-when-the-current-task-is-killed.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Johannes Weiner <hannes@cmpxchg.org>
Subject: Revert "vmalloc: back off when the current task is killed"

This reverts 5d17a73a2ebe ("vmalloc: back off when the current task is
killed") and 171012f56127 ("mm: don't warn when vmalloc() fails due to a
fatal signal").

5d17a73a2ebe ("vmalloc: back off when the current task is killed") made
all vmalloc allocations from a signal-killed task fail.  We have seen
crashes in the tty driver from this, where a killed task exiting tries to
switch back to N_TTY, fails n_tty_open because of the vmalloc failing, and
later crashes when dereferencing tty->disc_data.

Arguably, relying on a vmalloc() call to succeed in order to properly exit
a task is not the most robust way of doing things.  There will be a
follow-up patch to the tty code to fall back to the N_NULL ldisc.

But the justification to make that vmalloc() call fail like this isn't
convincing, either.  The patch mentions an OOM victim exhausting the
memory reserves and thus deadlocking the machine.  But the OOM killer is
only one, improbable source of fatal signals.  It doesn't make sense to
fail allocations preemptively with plenty of memory in most cases.

The patch doesn't mention real-life instances where vmalloc sites would
exhaust memory, which makes it sound more like a theoretical issue to
begin with.  But just in case, the OOM access to memory reserves has been
restricted on the allocator side in cd04ae1e2dc8 ("mm, oom: do not rely on
TIF_MEMDIE for memory reserves access"), which should take care of any
theoretical concerns on that front.

Revert this patch, and the follow-up that suppresses the allocation
warnings when we fail the allocations due to a signal.

Link: http://lkml.kernel.org/r/20171004185906.GB2136@cmpxchg.org
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Alan Cox <alan@llwyncelyn.cymru>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/vmalloc.c |    6 ------
 1 file changed, 6 deletions(-)

diff -puN mm/vmalloc.c~revert-vmalloc-back-off-when-the-current-task-is-killed mm/vmalloc.c
--- a/mm/vmalloc.c~revert-vmalloc-back-off-when-the-current-task-is-killed
+++ a/mm/vmalloc.c
@@ -1695,11 +1695,6 @@ static void *__vmalloc_area_node(struct
 	for (i = 0; i < area->nr_pages; i++) {
 		struct page *page;
 
-		if (fatal_signal_pending(current)) {
-			area->nr_pages = i;
-			goto fail_no_warn;
-		}

^ permalink raw reply

* + tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup.patch added to -mm tree
From: akpm @ 2017-10-04 22:32 UTC (permalink / raw)
  To: hannes, alan, dvyukov, hch, mhocko, mm-commits


The patch titled
     Subject: tty: fall back to N_NULL if switching to N_TTY fails during hangup
has been added to the -mm tree.  Its filename is
     tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup.patch

This patch should soon appear at
    http://ozlabs.org/~akpm/mmots/broken-out/tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup.patch
and later at
    http://ozlabs.org/~akpm/mmotm/broken-out/tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Johannes Weiner <hannes@cmpxchg.org>
Subject: tty: fall back to N_NULL if switching to N_TTY fails during hangup

We have seen NULL-pointer dereference crashes in tty->disc_data when the
N_TTY fallback driver failed to open during hangup.  The immediate cause
of this open to fail has been addressed in the preceding patch to
vmalloc(), but this code could be more robust.

As Alan pointed out in 8a8dabf2dd68 ("tty: handle the case where we cannot
restore a line discipline"), the N_TTY driver, historically the safe
fallback that could never fail, can indeed fail, but the surrounding code
is not prepared to handle this.  To avoid crashes he added a new N_NULL
driver to take N_TTY's place as the last resort.

Hook that fallback up to the hangup path.  Update tty_ldisc_reinit() to
reflect the reality that n_tty_open can indeed fail.

Link: http://lkml.kernel.org/r/20171004185959.GC2136@cmpxchg.org
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Alan Cox <alan@llwyncelyn.cymru>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/tty/tty_ldisc.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff -puN drivers/tty/tty_ldisc.c~tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup drivers/tty/tty_ldisc.c
--- a/drivers/tty/tty_ldisc.c~tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup
+++ a/drivers/tty/tty_ldisc.c
@@ -694,10 +694,8 @@ int tty_ldisc_reinit(struct tty_struct *
 	tty_set_termios_ldisc(tty, disc);
 	retval = tty_ldisc_open(tty, tty->ldisc);
 	if (retval) {
-		if (!WARN_ON(disc == N_TTY)) {
-			tty_ldisc_put(tty->ldisc);
-			tty->ldisc = NULL;
-		}
+		tty_ldisc_put(tty->ldisc);
+		tty->ldisc = NULL;
 	}
 	return retval;
 }
@@ -752,8 +750,9 @@ void tty_ldisc_hangup(struct tty_struct
 
 	if (tty->ldisc) {
 		if (reinit) {
-			if (tty_ldisc_reinit(tty, tty->termios.c_line) < 0)
-				tty_ldisc_reinit(tty, N_TTY);
+			if (tty_ldisc_reinit(tty, tty->termios.c_line) < 0 &&
+			    tty_ldisc_reinit(tty, N_TTY) < 0)
+				WARN_ON(tty_ldisc_reinit(tty, N_NULL) < 0);
 		} else
 			tty_ldisc_kill(tty);
 	}
_

Patches currently in -mm which might be from hannes@cmpxchg.org are

revert-vmalloc-back-off-when-the-current-task-is-killed.patch
tty-fall-back-to-n_null-if-switching-to-n_tty-fails-during-hangup.patch


^ permalink raw reply

* Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
From: Andrew Morton @ 2017-10-04 22:32 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Alan Cox, Christoph Hellwig, Michal Hocko, linux-mm, linux-kernel,
	kernel-team
In-Reply-To: <20171004185906.GB2136@cmpxchg.org>

On Wed, 4 Oct 2017 14:59:06 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:

> This reverts commit 5d17a73a2ebeb8d1c6924b91e53ab2650fe86ffb and
> commit 171012f561274784160f666f8398af8b42216e1f.
> 
> 5d17a73a2ebe ("vmalloc: back off when the current task is killed")
> made all vmalloc allocations from a signal-killed task fail. We have
> seen crashes in the tty driver from this, where a killed task exiting
> tries to switch back to N_TTY, fails n_tty_open because of the vmalloc
> failing, and later crashes when dereferencing tty->disc_data.
> 
> Arguably, relying on a vmalloc() call to succeed in order to properly
> exit a task is not the most robust way of doing things. There will be
> a follow-up patch to the tty code to fall back to the N_NULL ldisc.
> 
> But the justification to make that vmalloc() call fail like this isn't
> convincing, either. The patch mentions an OOM victim exhausting the
> memory reserves and thus deadlocking the machine. But the OOM killer
> is only one, improbable source of fatal signals. It doesn't make sense
> to fail allocations preemptively with plenty of memory in most cases.
> 
> The patch doesn't mention real-life instances where vmalloc sites
> would exhaust memory, which makes it sound more like a theoretical
> issue to begin with. But just in case, the OOM access to memory
> reserves has been restricted on the allocator side in cd04ae1e2dc8
> ("mm, oom: do not rely on TIF_MEMDIE for memory reserves access"),
> which should take care of any theoretical concerns on that front.
> 
> Revert this patch, and the follow-up that suppresses the allocation
> warnings when we fail the allocations due to a signal.

You don't think they should be backported into -stables?

^ permalink raw reply

* Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
From: Andrew Morton @ 2017-10-04 22:32 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Alan Cox, Christoph Hellwig, Michal Hocko, linux-mm, linux-kernel,
	kernel-team
In-Reply-To: <20171004185906.GB2136@cmpxchg.org>

On Wed, 4 Oct 2017 14:59:06 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:

> This reverts commit 5d17a73a2ebeb8d1c6924b91e53ab2650fe86ffb and
> commit 171012f561274784160f666f8398af8b42216e1f.
> 
> 5d17a73a2ebe ("vmalloc: back off when the current task is killed")
> made all vmalloc allocations from a signal-killed task fail. We have
> seen crashes in the tty driver from this, where a killed task exiting
> tries to switch back to N_TTY, fails n_tty_open because of the vmalloc
> failing, and later crashes when dereferencing tty->disc_data.
> 
> Arguably, relying on a vmalloc() call to succeed in order to properly
> exit a task is not the most robust way of doing things. There will be
> a follow-up patch to the tty code to fall back to the N_NULL ldisc.
> 
> But the justification to make that vmalloc() call fail like this isn't
> convincing, either. The patch mentions an OOM victim exhausting the
> memory reserves and thus deadlocking the machine. But the OOM killer
> is only one, improbable source of fatal signals. It doesn't make sense
> to fail allocations preemptively with plenty of memory in most cases.
> 
> The patch doesn't mention real-life instances where vmalloc sites
> would exhaust memory, which makes it sound more like a theoretical
> issue to begin with. But just in case, the OOM access to memory
> reserves has been restricted on the allocator side in cd04ae1e2dc8
> ("mm, oom: do not rely on TIF_MEMDIE for memory reserves access"),
> which should take care of any theoretical concerns on that front.
> 
> Revert this patch, and the follow-up that suppresses the allocation
> warnings when we fail the allocations due to a signal.

You don't think they should be backported into -stables?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* + linux-kernelh-add-correct-kernel-doc-notation.patch added to -mm tree
From: akpm @ 2017-10-04 22:33 UTC (permalink / raw)
  To: rdunlap, mm-commits


The patch titled
     Subject: linux/kernel.h: add/correct kernel-doc notation
has been added to the -mm tree.  Its filename is
     linux-kernelh-add-correct-kernel-doc-notation.patch

This patch should soon appear at
    http://ozlabs.org/~akpm/mmots/broken-out/linux-kernelh-add-correct-kernel-doc-notation.patch
and later at
    http://ozlabs.org/~akpm/mmotm/broken-out/linux-kernelh-add-correct-kernel-doc-notation.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Randy Dunlap <rdunlap@infradead.org>
Subject: linux/kernel.h: add/correct kernel-doc notation

Add kernel-doc notation for some macros.  Correct kernel-doc comments &
typos for a few macros.

Link: http://lkml.kernel.org/r/76fa1403-1511-be4c-e9c4-456b43edfad3@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/kernel.h |   90 ++++++++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 16 deletions(-)

diff -puN include/linux/kernel.h~linux-kernelh-add-correct-kernel-doc-notation include/linux/kernel.h
--- a/include/linux/kernel.h~linux-kernelh-add-correct-kernel-doc-notation
+++ a/include/linux/kernel.h
@@ -44,6 +44,12 @@
 
 #define STACK_MAGIC	0xdeadbeef
 
+/**
+ * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value
+ * @x: value to repeat
+ *
+ * NOTE: @x is not checked for > 0xff; larger values produce odd results.
+ */
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
 /* @a is a power of 2 value */
@@ -57,6 +63,10 @@
 #define READ			0
 #define WRITE			1
 
+/**
+ * ARRAY_SIZE - get the number of elements in array @arr
+ * @arr: array to be sized
+ */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
 
 #define u64_to_user_ptr(x) (		\
@@ -76,7 +86,15 @@
 #define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
 #define round_down(x, y) ((x) & ~__round_mask(x, y))
 
+/**
+ * FIELD_SIZEOF - get the size of a struct's field
+ * @t: the target struct
+ * @f: the target struct's field
+ * Return: the size of @f in the struct definition without having a
+ * declared instance of @t.
+ */
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
+
 #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP
 
 #define DIV_ROUND_DOWN_ULL(ll, d) \
@@ -107,7 +125,7 @@
 /*
  * Divide positive or negative dividend by positive or negative divisor
  * and round to closest integer. Result is undefined for negative
- * divisors if he dividend variable type is unsigned and for negative
+ * divisors if the dividend variable type is unsigned and for negative
  * dividends if the divisor variable type is unsigned.
  */
 #define DIV_ROUND_CLOSEST(x, divisor)(			\
@@ -247,13 +265,13 @@ extern int _cond_resched(void);
  * @ep_ro: right open interval endpoint
  *
  * Perform a "reciprocal multiplication" in order to "scale" a value into
- * range [0, ep_ro), where the upper interval endpoint is right-open.
+ * range [0, @ep_ro), where the upper interval endpoint is right-open.
  * This is useful, e.g. for accessing a index of an array containing
- * ep_ro elements, for example. Think of it as sort of modulus, only that
+ * @ep_ro elements, for example. Think of it as sort of modulus, only that
  * the result isn't that of modulo. ;) Note that if initial input is a
  * small value, then result will return 0.
  *
- * Return: a result based on val in interval [0, ep_ro).
+ * Return: a result based on @val in interval [0, @ep_ro).
  */
 static inline u32 reciprocal_scale(u32 val, u32 ep_ro)
 {
@@ -618,8 +636,8 @@ do {									\
  * trace_printk - printf formatting in the ftrace buffer
  * @fmt: the printf format for printing
  *
- * Note: __trace_printk is an internal function for trace_printk and
- *       the @ip is passed in via the trace_printk macro.
+ * Note: __trace_printk is an internal function for trace_printk() and
+ *       the @ip is passed in via the trace_printk() macro.
  *
  * This function allows a kernel developer to debug fast path sections
  * that printk is not appropriate for. By scattering in various
@@ -629,7 +647,7 @@ do {									\
  * This is intended as a debugging tool for the developer only.
  * Please refrain from leaving trace_printks scattered around in
  * your code. (Extra memory is used for special buffers that are
- * allocated when trace_printk() is used)
+ * allocated when trace_printk() is used.)
  *
  * A little optization trick is done here. If there's only one
  * argument, there's no need to scan the string for printf formats.
@@ -681,7 +699,7 @@ int __trace_printk(unsigned long ip, con
  *       the @ip is passed in via the trace_puts macro.
  *
  * This is similar to trace_printk() but is made for those really fast
- * paths that a developer wants the least amount of "Heisenbug" affects,
+ * paths that a developer wants the least amount of "Heisenbug" effects,
  * where the processing of the print format is still too much.
  *
  * This function allows a kernel developer to debug fast path sections
@@ -692,7 +710,7 @@ int __trace_printk(unsigned long ip, con
  * This is intended as a debugging tool for the developer only.
  * Please refrain from leaving trace_puts scattered around in
  * your code. (Extra memory is used for special buffers that are
- * allocated when trace_puts() is used)
+ * allocated when trace_puts() is used.)
  *
  * Returns: 0 if nothing was written, positive # if string was.
  *  (1 when __trace_bputs is used, strlen(str) when __trace_puts is used)
@@ -771,6 +789,12 @@ static inline void ftrace_dump(enum ftra
 	t2 min2 = (y);					\
 	(void) (&min1 == &min2);			\
 	min1 < min2 ? min1 : min2; })
+
+/**
+ * min - return minimum of two values of the same or compatible types
+ * @x: first value
+ * @y: second value
+ */
 #define min(x, y)					\
 	__min(typeof(x), typeof(y),			\
 	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
@@ -781,12 +805,31 @@ static inline void ftrace_dump(enum ftra
 	t2 max2 = (y);					\
 	(void) (&max1 == &max2);			\
 	max1 > max2 ? max1 : max2; })
+
+/**
+ * max - return maximum of two values of the same or compatible types
+ * @x: first value
+ * @y: second value
+ */
 #define max(x, y)					\
 	__max(typeof(x), typeof(y),			\
 	      __UNIQUE_ID(max1_), __UNIQUE_ID(max2_),	\
 	      x, y)
 
+/**
+ * min3 - return minimum of three values
+ * @x: first value
+ * @y: second value
+ * @z: third value
+ */
 #define min3(x, y, z) min((typeof(x))min(x, y), z)
+
+/**
+ * max3 - return maximum of three values
+ * @x: first value
+ * @y: second value
+ * @z: third value
+ */
 #define max3(x, y, z) max((typeof(x))max(x, y), z)
 
 /**
@@ -805,8 +848,8 @@ static inline void ftrace_dump(enum ftra
  * @lo: lowest allowable value
  * @hi: highest allowable value
  *
- * This macro does strict typechecking of lo/hi to make sure they are of the
- * same type as val.  See the unnecessary pointer comparisons.
+ * This macro does strict typechecking of @lo/@hi to make sure they are of the
+ * same type as @val.  See the unnecessary pointer comparisons.
  */
 #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
 
@@ -816,11 +859,24 @@ static inline void ftrace_dump(enum ftra
  *
  * Or not use min/max/clamp at all, of course.
  */
+
+/**
+ * min_t - return minimum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
 #define min_t(type, x, y)				\
 	__min(type, type,				\
 	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
 	      x, y)
 
+/**
+ * max_t - return maximum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
 #define max_t(type, x, y)				\
 	__max(type, type,				\
 	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
@@ -834,7 +890,7 @@ static inline void ftrace_dump(enum ftra
  * @hi: maximum allowable value
  *
  * This macro does no typechecking and uses temporary variables of type
- * 'type' to make all the comparisons.
+ * @type to make all the comparisons.
  */
 #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
 
@@ -845,15 +901,17 @@ static inline void ftrace_dump(enum ftra
  * @hi: maximum allowable value
  *
  * This macro does no typechecking and uses temporary variables of whatever
- * type the input argument 'val' is.  This is useful when val is an unsigned
- * type and min and max are literals that will otherwise be assigned a signed
+ * type the input argument @val is.  This is useful when @val is an unsigned
+ * type and @lo and @hi are literals that will otherwise be assigned a signed
  * integer type.
  */
 #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
 
 
-/*
- * swap - swap value of @a and @b
+/**
+ * swap - swap values of @a and @b
+ * @a: first value
+ * @b: second value
  */
 #define swap(a, b) \
 	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
_

Patches currently in -mm which might be from rdunlap@infradead.org are

kernel-hacking-menu-runtime-testing-keep-tests-together.patch
linux-kernelh-add-correct-kernel-doc-notation.patch


^ permalink raw reply

* Re: [MPTCP] [PATCH 05/18] tcp6: Prepare for maximum TCP option-space
From: Mat Martineau @ 2017-10-04 22:33 UTC (permalink / raw)
  To: mptcp 

[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]


On Tue, 3 Oct 2017, Christoph Paasch wrote:

> With a generic TCP-option framework we don't know in advance how much
> space will be used. So, estimate the worst and allocate the maximum.
> Later on, we pull skb->tail back in again.

v2 of the option registration patch will conflict with this. The prepare 
callback does figure out the number of option bytes required before 
writing to the outgoing skb, so that can happen before the alloc_skb().


Mat


>
> Signed-off-by: Christoph Paasch <cpaasch(a)apple.com>
> ---
> net/ipv6/tcp_ipv6.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 64d94afa427f..17f7e0cd8755 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -784,17 +784,11 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
> 	struct flowi6 fl6;
> 	struct net *net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
> 	struct sock *ctl_sk = net->ipv6.tcp_sk;
> -	unsigned int tot_len = sizeof(struct tcphdr);
> +	unsigned int tot_len = sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE;
> +	unsigned int reduce = 0; /* By how much to pull skb->tail back in? */
> 	struct dst_entry *dst;
> 	__be32 *topt;
>
> -	if (tsecr)
> -		tot_len += TCPOLEN_TSTAMP_ALIGNED;
> -#ifdef CONFIG_TCP_MD5SIG
> -	if (key)
> -		tot_len += TCPOLEN_MD5SIG_ALIGNED;
> -#endif
> -
> 	buff = alloc_skb(MAX_HEADER + sizeof(struct ipv6hdr) + tot_len,
> 			 GFP_ATOMIC);
> 	if (!buff)
> @@ -823,6 +817,8 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
> 				(TCPOPT_TIMESTAMP << 8) | TCPOLEN_TIMESTAMP);
> 		*topt++ = htonl(tsval);
> 		*topt++ = htonl(tsecr);
> +
> +		reduce += TCPOLEN_TSTAMP_ALIGNED;
> 	}
>
> #ifdef CONFIG_TCP_MD5SIG
> @@ -832,9 +828,13 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
> 		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
> 				    &ipv6_hdr(skb)->saddr,
> 				    &ipv6_hdr(skb)->daddr, t1);
> +
> +		reduce += TCPOLEN_MD5SIG_ALIGNED;
> 	}
> #endif
>
> +	buff->tail -= reduce;
> +
> 	memset(&fl6, 0, sizeof(fl6));
> 	fl6.daddr = ipv6_hdr(skb)->saddr;
> 	fl6.saddr = ipv6_hdr(skb)->daddr;
> -- 
> 2.14.1
>
>

--
Mat Martineau
Intel OTC

^ permalink raw reply

* Re: [PATCH v5 0/3] run-time Linking support
From: Li, Xiaoyun @ 2017-10-04 22:33 UTC (permalink / raw)
  To: Ananyev, Konstantin, Richardson, Bruce
  Cc: Lu, Wenzhuo, Zhang, Helin, dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772585FAA4014@IRSMSX103.ger.corp.intel.com>

OK. Will send it later. Many thanks!

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, October 5, 2017 01:56
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v5 0/3] run-time Linking support
> 
> Hi Xiaouyn,
> 
> > -----Original Message-----
> > From: Li, Xiaoyun
> > Sent: Tuesday, October 3, 2017 4:00 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson,
> > Bruce <bruce.richardson@intel.com>
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; dev@dpdk.org; Li, Xiaoyun
> > <xiaoyun.li@intel.com>
> > Subject: [PATCH v5 0/3] run-time Linking support
> >
> > This patchset dynamically selects functions at run-time based on CPU
> > flags that current machine supports.This patchset modifies mempcy,
> > memcpy perf test and x86 EFD, using function pointers and bind them at
> constructor time.
> > Then in the cloud environment, users can compiler once for the minimum
> > target such as 'haswell'(not 'native') and run on different platforms
> > (equal or above
> > haswell) and can get ISA optimization based on running CPU.
> >
> > Xiaoyun Li (3):
> >   eal/x86: run-time dispatch over memcpy
> >   app/test: run-time dispatch over memcpy perf test
> >   efd: run-time dispatch over x86 EFD functions
> >
> > ---
> > v2
> > * Use gcc function multi-versioning to avoid compilation issues.
> > * Add macros for AVX512 and AVX2. Only if users enable AVX512 and the
> > compiler supports it, the AVX512 codes would be compiled. Only if the
> > compiler supports AVX2, the AVX2 codes would be compiled.
> >
> > v3
> > * Reduce function calls via only keep rte_memcpy_xxx.
> > * Add conditions that when copy size is small, use inline code path.
> > Otherwise, use dynamic code path.
> > * To support attribute target, clang version must be greater than 3.7.
> > Otherwise, would choose SSE/AVX code path, the same as before.
> > * Move two mocro functions to the top of the code since they would be
> > used in inline SSE/AVX and dynamic SSE/AVX codes.
> >
> > v4
> > * Modify rte_memcpy.h to several .c files and modify makefiles to
> > compile
> > AVX2 and AVX512 files.
> >
> > v5
> > * Delete redundant repeated codes of rte_memcpy_xxx.
> > * Modify makefiles to enable reuse of existing rte_memcpy.
> > * Delete redundant codes of rte_efd_x86.h in v4. Move it into .c file
> > and enable compilation -mavx2 for it in makefile since it is already chosen
> at run-time.
> >
> 
> Generally looks good, just two things to fix below.
> Konstantin
> 
> 1. [dpdk-dev,v5,1/3] eal/x86: run-time dispatch over memcpy
> 
> Shared target build fails:
> http://dpdk.org/ml/archives/test-report/2017-October/031032.html
> 
> I think you need to include rte_memcpy_ptr into the:
> lib/librte_eal/linuxapp/eal/rte_eal_version.map
> lib/librte_eal/bsdapp/eal/rte_eal_version.map
> to fix it.
> 
> 2. [dpdk-dev,v5,3/3] efd: run-time dispatch over x86 EFD functions
> 
> /lib/librte_efd/rte_efd_x86.c
> ....
> +efd_value_t
> +efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
> +		const efd_lookuptbl_t *group_lookup_table,
> +		const uint32_t hash_val_a, const uint32_t hash_val_b)
> { #ifdef
> +CC_SUPPORT_AVX2
> +	efd_value_t value = 0;
> +	uint32_t i = 0;
> +	__m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
> +	__m256i vhash_val_b = _mm256_set1_epi32(hash_val_b);
> +
> +	for (; i < RTE_EFD_VALUE_NUM_BITS; i += 8) {
> +		__m256i vhash_idx =
> +				_mm256_cvtepu16_epi32(EFD_LOAD_SI128(
> +				(__m128i const *) &group_hash_idx[i]));
> +		__m256i vlookup_table = _mm256_cvtepu16_epi32(
> +				EFD_LOAD_SI128((__m128i const *)
> +				&group_lookup_table[i]));
> +		__m256i vhash = _mm256_add_epi32(vhash_val_a,
> +				_mm256_mullo_epi32(vhash_idx,
> vhash_val_b));
> +		__m256i vbucket_idx = _mm256_srli_epi32(vhash,
> +				EFD_LOOKUPTBL_SHIFT);
> +		__m256i vresult = _mm256_srlv_epi32(vlookup_table,
> +				vbucket_idx);
> +
> +		value |= (_mm256_movemask_ps(
> +			(__m256) _mm256_slli_epi32(vresult, 31))
> +			& ((1 << (RTE_EFD_VALUE_NUM_BITS - i)) - 1)) << i;
> +	}
> +
> +	return value;
> +#else
> 
> We always build that file with AVX2 option, so I think we can safely  remove
> The #ifdef CC_SUPPORT_AVX2 and the code below.
> 
> +	RTE_SET_USED(group_hash_idx);
> +	RTE_SET_USED(group_lookup_table);
> +	RTE_SET_USED(hash_val_a);
> +	RTE_SET_USED(hash_val_b);
> +	/* Return dummy value, only to avoid compilation breakage */
> +	return 0;
> +#endif
> +
> +}
> 
> 
> >  lib/librte_eal/bsdapp/eal/Makefile                 |  19 +
> >  .../common/include/arch/x86/rte_memcpy.c           |  59 ++
> >  .../common/include/arch/x86/rte_memcpy.h           | 861 +------------------
> >  .../common/include/arch/x86/rte_memcpy_avx2.c      |  44 +
> >  .../common/include/arch/x86/rte_memcpy_avx512f.c   |  44 +
> >  .../common/include/arch/x86/rte_memcpy_internal.h  | 909
> +++++++++++++++++++++
> >  .../common/include/arch/x86/rte_memcpy_sse.c       |  40 +
> >  lib/librte_eal/linuxapp/eal/Makefile               |  19 +
> >  lib/librte_efd/Makefile                            |   6 +
> >  lib/librte_efd/rte_efd_x86.c                       |  87 ++
> >  lib/librte_efd/rte_efd_x86.h                       |  48 +-
> >  mk/rte.cpuflags.mk                                 |  14 +
> >  test/test/test_memcpy_perf.c                       |  40 +-
> >  13 files changed, 1285 insertions(+), 905 deletions(-)  create mode
> > 100644 lib/librte_eal/common/include/arch/x86/rte_memcpy.c
> >  create mode 100644
> > lib/librte_eal/common/include/arch/x86/rte_memcpy_avx2.c
> >  create mode 100644
> > lib/librte_eal/common/include/arch/x86/rte_memcpy_avx512f.c
> >  create mode 100644
> > lib/librte_eal/common/include/arch/x86/rte_memcpy_internal.h
> >  create mode 100644
> > lib/librte_eal/common/include/arch/x86/rte_memcpy_sse.c
> >  create mode 100644 lib/librte_efd/rte_efd_x86.c
> >
> > --
> > 2.7.4

^ permalink raw reply

* Re: [PATCH 09/13] drm/i915/cnl: Invert dvfs default level.
From: Manasi Navare @ 2017-10-04 22:40 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
In-Reply-To: <20171004193642.xrdlm62lfc22rfzl@intel.com>

On Wed, Oct 04, 2017 at 12:36:42PM -0700, Rodrigo Vivi wrote:
> On Wed, Oct 04, 2017 at 09:46:41AM +0000, Mika Kahola wrote:
> > On Tue, 2017-10-03 at 00:06 -0700, Rodrigo Vivi wrote:
> > > According to spec "If voltage is set too low,
> > > it will break functionality. If voltage is set too high,
> > >  it will waste power."
> > > 
> > > So, let's prefer the waste of power instead of breaking
> > > functionality.
> > > 
> > > But also the logic of deciding the level on spec
> > > tells "Else, use level 2."
> > > So, default is actually "2", not "0".
> > The spec also says
> > 
> > "If CD clock = 168 MHz AND DDI clock ≤ 594 MHz, use level 0.
> > Else If CD clock = 336 MHz AND DDI clock ≤ 594 MHz, use level 1.
> > Else, use level 2."
> > 
> > Should we add check for DDI clock rate as well here?
> 
> By this time dpll are disabled and not associated to any
> port yet. So portclock is 0. So if we invert the default
> we do respect the same algorightm you pasted.
> Also if you notice I'm just inverting this in a separeted
> patch to make it clear and if needed to bisect we end up
> on this single point of change. But on a later patch on
> this series I change to use your function with this algoright
> there, but giving portclock = 0.
> 
> So by the end of this series the flow is:
> 
> - enable cdclk
> - request dvfs level enough for cdclk in question.
> - enable pll
> - adjust dvfs level only if needed taking the port clock into account.
> - disable pll
> - put back level to cdclock level if needed.
> 
> > 
> > > 
> > > v2: Rebase moving it up to avoid some temporary code
> > >     duplication.
> > > 
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index af8411c2a6b9..7e9c4444c844 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1562,15 +1562,15 @@ static void cnl_set_cdclk(struct
> > > drm_i915_private *dev_priv,
> > >  	}
> > >  
> > >  	switch (cdclk) {
> > > -	case 528000:
> > > -		pcu_ack = 2;
> > > +	case 168000:
> > > +		pcu_ack = 0;
> > >  		break;
> > >  	case 336000:
> > >  		pcu_ack = 1;
> > >  		break;
> > > -	case 168000:
> > > +	case 528000:
> > >  	default:
> > > -		pcu_ack = 0;
> > > +		pcu_ack = 2;
> > >  		break;

The spec says "Else If CD clock ≤ 556.8 AND DDI clock > 594 MHz, use level 1"
So in case of cdclock = 528000, it is still < 556.8Mhz and the level should be 1

Manasi

> > >  	}
> > >  
> > -- 
> > Mika Kahola - Intel OTC
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume
From: Bart Van Assche @ 2017-10-04 22:36 UTC (permalink / raw)
  To: ming.lei@redhat.com
  Cc: hch@lst.de, linux-block@vger.kernel.org, oleksandr@natalenko.name,
	martin.petersen@oracle.com, axboe@kernel.dk, mcgrof@kernel.org
In-Reply-To: <20171004153149.GD9713@ming.t460p>

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDIzOjMxICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gT24g
V2VkLCBPY3QgMDQsIDIwMTcgYXQgMDM6MTg6NDVQTSArMDAwMCwgQmFydCBWYW4gQXNzY2hlIHdy
b3RlOg0KPiA+IE9uIFdlZCwgMjAxNy0xMC0wNCBhdCAxNTo1MyArMDgwMCwgTWluZyBMZWkgd3Jv
dGU6DQo+ID4gPiAyKSBSQ1UgcmVhZCBsb2NrIGlzIG1pc3NlZCBpbiBmYXN0IHBhdGgNCj4gPiAN
Cj4gPiBEaWQgeW91IHJlYWxseSBwb3N0IHRoaXMgYXMgYSByZXZpZXcgY29tbWVudCBvbiBteSBw
YXRjaCBzZXJpZXM/IEFueXdheSwgdGhpcw0KPiA+IGlzIGVhc3kgdG8gYWRkcmVzcy4NCj4gDQo+
IGh0dHBzOi8vbWFyYy5pbmZvLz9sPWxpbnV4LWJsb2NrJm09MTUwNjQxNDgxNzMyNTcyJnc9Mg0K
DQpUaGUgZS1tYWlsIHlvdSByZWZlciB0byBpcyBhIHJlcGx5IHRvIGEgcGF0Y2ggaW4gdjQgb2Yg
dGhpcyBwYXRjaCBzZXJpZXMgaW5zdGVhZA0Kb2YgdjUuDQoNCkJhcnQu

^ permalink raw reply

* Re: [PATCH v3 0/7] new mlx4 datapath bypassing ibverbs
From: Ferruh Yigit @ 2017-10-04 22:37 UTC (permalink / raw)
  To: Ophir Munk, Adrien Mazarguil
  Cc: dev, Thomas Monjalon, Olga Shern, Matan Azrad
In-Reply-To: <1507153746-31255-1-git-send-email-ophirmu@mellanox.com>

On 10/4/2017 10:48 PM, Ophir Munk wrote:
> Changes from v2:
> * Split "net/mlx4: support multi-segments Rx" commit from "net/mlx4: get back Rx flow functionality" commit
> * Semantics, code styling
> * Fix check-git-log warnings
> * Fix checkpatches warnings
> 
> Next (currently not included) changes:
> * Replacing MLX4_TRANSPOSE() macro (Generic macro to convert MLX4 to IBV flags) with a look-up table as in mlx5
> for example: mlx5_set_ptype_table() function - in order to improve performance.
> This change is delicate and should be verified first with regression tests
> 
> * PMD documentation update when no longer working with MLNX_OFED
> Documentation updtes require specific kernel, rdma_core and FW versions as well as installation procedures.
> These details should be supplied by regression team.
> 
> Moti Haimovsky (6):
>   net/mlx4: add simple Tx bypassing ibverbs
>   net/mlx4: get back Rx flow functionality
>   net/mlx4: support multi-segments Tx
>   net/mlx4: get back Tx checksum offloads
>   net/mlx4: get back Rx checksum offloads
>   net/mlx4: add loopback Tx from VF
> 
> Vasily Philipov (1):
>   net/mlx4: support multi-segments Rx

Adried looks like beat you with a few hours :)

Can you please clarify which set is valid one, thanks.

^ permalink raw reply

* Re: [RFC PATCH 1/2] kbuild: Add a cache for generated variables
From: Doug Anderson @ 2017-10-04 22:38 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Guenter Roeck, Brian Norris, Marcin Nowakowski,
	Matthias Kaehlcke, Behan Webster, Arnd Bergmann, Mark Charlebois,
	Cao jin, Linux Kbuild mailing list, open list:DOCUMENTATION,
	Jonathan Corbet, Linux Kernel Mailing List, Ingo Molnar
In-Reply-To: <CAK7LNASCy-zMNF1-7gie4NhyKC8xhAtyVHatpQ7JYLUz6UiG0w@mail.gmail.com>

Hi,

On Tue, Oct 3, 2017 at 9:05 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Thanks for the patches.  The idea is interesting.
>
> I am not a Chrome developer, but cc-option could be improved somehow.
>
>
> I examined two approaches to mitigate the pain.
>
> [1] Skip cc-option completely when we run non-build targets
>     such as "make help", "make clean", etc.
>
> [2] Cache the result of cc-option like this patch
>
>
> I wrote some patches for [1]
> https://patchwork.kernel.org/patch/9983825/
> https://patchwork.kernel.org/patch/9983829/
> https://patchwork.kernel.org/patch/9983833/
> https://patchwork.kernel.org/patch/9983827/
>
> Comments are welcome.  :)

OK, I'll take a look at them.  I'm not massively familiar with the
kernel Makefiles, but hopefully I can figure some of it out.  :-P


> [1] does not solve the slowness in the incremental build.
> Instead, it makes non-build targets faster
> from a clean source tree because cc-option is zero cost.
>
>
> [2] solves the issues in the incremental build.
> One funny thing is, it computes cc-option and store the cache file
> even for "make clean", where we know the cache file will be
> immediately deleted.
>
>
> We can apply [1] or [2], or maybe both of them
> because [1] and [2] are trying to solve the different issues.

Yeah, I'm much more interested in [2] than [1].  I run incremental
builds almost constantly and hate the slowness.  :(  I can certainly
appreciate that the inefficient compiler setup in Chrome OS isn't your
problem and that an extra .5 seconds for an incremental build for most
people isn't that huge, though.  ...but I'l probably move forward with
[2] since it still helps me a lot and still should help others.
Solving [1] seems worthwhile too, though...


> The cache approach seemed clever, but I do not like the implementation.
> The code is even more unreadable with lots of escape sequence.
>
>
> Here is my suggestion for simpler implementation (including bike-shed)
>
>
> Implement a new function "shell-cache".  It works like $(shell ...)
>
> The difference is
> $(call shell-cached,...) returns the cached result, or run the command
> if not cached.
>
>
>
> Also, add try-run-cached.  This is a cached variant of try-run.
>
>
> I just played with it, and seems working.
> (I did not have spend much time for testing, more wider test is needed.)
>
>
> The code is like something like this:
>
>
>
> make-cache := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/,$(if
> $(obj),$(obj)/)).cache.mk
>
> -include $(make-cache)

Thanks, this is much better!  :)


> define __run-and-store
> ifeq ($(origin $(1)),undefined)
>   $$(eval $(1) := $$(shell $$2))
>   $$(shell echo '$(1) := $$($(1))' >> $(make-cache))
> endif
> endef

I like your idea.  Essentially you're saying that we can just defer
the shell command, which was the really essential part that needed to
be deferred.  Nice!  It seems much nicer / cleaner.  Still a tiny bit
of voodoo, but with all of your improvements the voodoo is at least
contained to a very small part of the file.

OK, things seem pretty nice with this and I'll submit out a new patch.
I'm a bit conflicted about whether I should put myself as authorship
or you, since mostly the patch is your code now.  ;-)  For now I'll
leave my authorship but feel free to claim it and change me to just
"Suggested-by".  :-)

NOTE: one thing I noticed about your example code is that you weren't
always consistent about $(1) vs. $1.  I've changed things to be
consistent at the expense of extra parenthesis.  If you hate it, let
me know.


> # $(call,shell-cached,my_command)
> # This works like $(shell my_command), but the result is cached
> __shell-cached = $(eval $(call __run-and-store,$1))$($1)
> shell-cached = $(call __shell-cached,__cached_$(call __sanitize-opt,$(1)),$1)
>
> ...
>
>
> __try-run = set -e; \
>      TMP="$(TMPOUT).$$$$.tmp"; \
>      TMPO="$(TMPOUT).$$$$.o"; \
>      if ($(1)) >/dev/null 2>&1; \
>      then echo "$(2)"; \
>      else echo "$(3)"; \
>      fi; \
>      rm -f "$$TMP" "$$TMPO"
>
> # try-run
> # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> # Exit code chooses option. "$$TMP" serves as a temporary file and is
> # automatically cleaned up.
> try-run = $(shell $(__try-run))
>
> # try-run-cached
> # This works like try-run, but the result is cached.
> try-run-cached = $(call shell-cached,$(__try-run))
>
>
>
>
> Then, you can replace
>
>   $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)
>
> with
>
>   $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh
> $(CC) $(KBUILD_CFLAGS)
>
>
>
>
> replace
>
> __cc-option = $(call try-run,\
>          $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
>
> with
>
> __cc-option = $(call try-run-cached,\
>          $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
>
>
>
>
> What do you think?

Yes, much better!  Due to where you've placed this I now seem to get
by without my $(call __comma,...) calls and also the "$$$$"
conversion.  Yay!

...interestingly I need to add ":" to the "__sanitize-opt" now,
though.  That's for the arm64 line:

  brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . -
1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)



> A little more comments below.
>
>
>
>
>> +# Tools for caching Makefile variables that are "expensive" to compute.
>> +#
>> +# Here we want to help deal with variables that take a long time to compute
>> +# by making it easy to store these variables in a cache.
>> +#
>> +# The canonical example here is testing for compiler flags.  On a simple system
>> +# each call to the compiler takes 10 ms, but on a system with a compiler that's
>> +# called through various wrappers it can take upwards of 100 ms.  If we have
>> +# 100 calls to the compiler this can take 1 second (on a simple system) or 10
>> +# seconds (on a complicated system).
>> +#
>> +# The "cache" will be in Makefile syntax and can be directly included.
>> +# Any time we try to reference a variable that's not in the cache we'll
>> +# calculate it and store it in the cache for next time.
>> +
>> +# Include values from last time
>> +-include Makefile-cache.o
>
>
> Kbuild.include is included, so is Makefile-cache.o
> every time the build system descend into sub-directories.
>
> It is not efficient to include cached data unneeded in sub-directories.
> I prefixed $(obj)/
>
> Another problem is when building external modules.
> Makefile-cache.o is always created/updated in the kernel build tree.
>
> When we build external modules, the kernel source may be located under
> /usr/src/ , which is generally read-only for normal users.
> So, I prefixed $(KBUILD_EXTMOD) to create the cache file
> in the module tree if KBUILD_EXTMOD is defined.
>
> I do not like the suffix .o
> I prefer file name to be something else
> that starts with . to hide it.

Thanks for all the improvements!


>> +# Usage:   $(call cached-val,variable_name,escaped_expensive_operation)
>> +# Example: $(call cached-val,md5_val,$$(shell md5sum /usr/bin/gcc)
>> +#
>> +# If the variable is already defined we'll just use it.  If it's not, it will
>> +# be calculated and stored in a persistent (disk-based) cache for the next
>> +# invocation of Make.  The call will evaluate to the value of the variable.
>> +#
>> +# This is a bit of voodoo, but basic explanation is that if the variable
>> +# was undefined then we'll evaluate the expensive operation and store it into
>> +# the variable.  We'll then store that value in the cache and finally output
>> +# the value.
>> +define __set-and-store
>> +ifeq ($(origin $(1)),undefined)
>> +  $$(eval $(1) := $$(2))
>> +  $$(shell echo '$(1) := $$($(1))' >> Makefile-cache.o)
>> +endif
>> +endef
>> +cached-val = $(eval $(call __set-and-store,__cached_$(1)))$(__cached_$(1))
>
>
> This seems working, but I do not understand this trick.
>
>
> __set-and-store takes two arguments,
> but here, it is called with one argument __cached_$(1)
>
> Probably, $$(try-run, ...) will be passed as $2,
> but I do not know why this works.

Whew!  It's not just me that's confused by all this...  ;-)  It looks
like you continued to use this in your suggestion, so I guess you
thought it was useful, at least...  Yeah, it's a bit of magic.  The
goal was to have one less set of evaluating and passing going around.

So really '__set-and-store' takes _1_ argument.  It outputs a string
where it uses this argument.  Also part of the output string is a
reference to $(2).  This will refer to the caller's second variable.

===

Maybe a "simpler" example?

define example
$$(eval $(1) := $$(2))
endef

ex_usage = $(eval $(call example,$(1)))$($(1))

.PHONY: ex
ex:
  @echo $(call ex_usage,myvar,myval)
  @echo $(myvar)

If you do "make ex" you'll see "myval" printed twice.

--

Walking through that, let's first remove the "call" which we can do in
this case pretty easily because there are no "ifdefs", unlike the real
code.  Since the first argument to "example" is $(1), the above is the
same as:

ex_usage = $(eval $$(eval $(1) := $$(2)))$($(1))

...now we need to resolve that recursively where $(1) is 'myvar' and
$(2) is 'myval'.  So we get:

1. $(eval $$(eval $(1) := $$(2)))$($(1))
2. $(eval $$(eval myvar := $$(2)))$(myvar)
3. $(eval myvar := $(2))$(myvar)
4. $(eval myvar := myval)$(myvar)

--

An alternate version of my "simpler" example that doesn't use this magic:

define example
$$(eval $(1) := $(2))
endef

ex_usage = $(eval $(call example,$(1),$(2)))$($(1))

.PHONY: ex
ex:
  @echo $(call ex_usage,myvar,myval)
  @echo $(myvar)

...that works OK for the simple example case but you get into weird
esoteric issues because of the extra eval.  You'll find that the two
definitions behave differently for:

  @echo $(call ex_usage,myvar,my$$$$(cat /proc/cmdline)val)

...in that example the "intention" is that bash should be passed the
string: echo my$(cat /proc/cmdline)val, so it escapes the "$" once to
"defer" it and twice because it wants the "$" to go through to bash.

--

...and yes, I already punched myself in the face for the complexity of
all of the above.  ;-)

It's certainly possible that there's some way to get rid of an eval in
the above.  If you have a suggestion that would reduce the number
needed that'd be interesting.


>> +# Usage: $(call __sanitize-opt,Hello=Hola$(comma)Goodbye Adios)
>> +#
>> +# Convert all '$', ')', '(', '\', '=', ' ', ',' to '_'
>> +__sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(1))))))))
>> +
>> +# Usage = $(call __comma,something_with_comma)
>> +#
>> +# Convert ',' to '$(comma)' which can help it getting interpreted by eval.
>> +__comma = $(subst $(comma),$$(comma),$(1))
>> +
>>  # cc-cross-prefix
>>  # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-)
>>  # Return first prefix where a prefix$(CC) is found in PATH.
>> @@ -99,19 +148,34 @@ try-run = $(shell set -e;          \
>>  # as-option
>>  # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
>>
>> -as-option = $(call try-run,\
>> -       $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
>> +as-option = \
>> +       $(call cached-val,$(call __sanitize-opt,\
>> +               as_opt_$(CC)_$(KBUILD_CFLAGS)_$(1)_$(2)),\
>> +       $$(call try-run,\
>> +       $(CC) $(call __comma,$(KBUILD_CFLAGS)) $(call __comma,$(1)) \
>> +       -c -x assembler /dev/null \
>> +       -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2))))
>>
>>  # as-instr
>>  # Usage: cflags-y += $(call as-instr,instr,option1,option2)
>>
>> -as-instr = $(call try-run,\
>> -       printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
>> +as-instr = \
>> +       $(call cached-val,$(call __sanitize-opt,\
>> +               as_instr_$(CC)_$(KBUILD_AFLAGS)_$(1)_$(2)_$(3)),\
>> +       $$(call try-run,\
>> +       printf "%b\n" "$(call __comma,$(1))" | \
>> +       $(CC) $(call __comma,$(KBUILD_AFLAGS)) -c -x assembler \
>> +       -o "$$$$TMP" -,$(call __comma,$(2)),$(call __comma,$(3))))
>>
>>  # __cc-option
>>  # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
>> -__cc-option = $(call try-run,\
>> -       $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
>> +__cc-option = \
>> +       $(call cached-val,$(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)),\
>> +       $$(call try-run,\
>> +       $(call __comma,$(1)) -Werror \
>> +       $(call __comma,$(2)) \
>> +       $(call __comma,$(3)) -c -x c /dev/null \
>> +       -o "$$$$TMP",$(call __comma,$(strip $(3))),$(call __comma,$(strip $(4)))))
>
>
> I did not follow how this was working here...

Hoo boy.  Let's see...  Which part were you curious about?  I can try
to explain the bits and you can tell me if I got them all.  Note that
many of these aren't super important anymore since they're not needed
with your improvements:


* $(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)):

Tries to make a variable name that will be different if you have any
different parameters.


* $$(call try-run

The "$$" around this call is because it's deferred and a general
concept for "cached-val".  If (and only if) we decide that we need to
recompute this value then we'll run this through an "eval".  By having
the $$ then this action won't be taken unless the string is eval-ed.
Note that plenty of stuff _will_ still happen right away, though.  All
of the $(call __comma,$(1)) are _not_ deferred.  That's fine since
that doesn't lead to a fork-exec and thus isn't "expensive".


*  $(call __comma,$(1))

I'll admit that I didn't dig all the way into this.  I found at least
one case where "make" was incorrectly mixing up arguments due to the
"eval" and the "__comma" fixed it.  I decided it was important to do
this for all arguments.  Possibly other things might need to be
escaped too, but I didn't find any cases that needed it...  ...but no
longer needed with your improvements...


* $$$$TMP

An extra level of escaping is needed since it goes through an extra
"eval".  This means $$ => $$$$


* $(strip $(3))

This was important because otherwise the cache otherwise wasn't
completely "stable".  If you ran "make" the first time you'd get a
cache, then running "make" the 2nd time would give you extra entries
in the cache.  After the 3rd time you were OK, but it seemed nice to
make it stable faster.

To understand this, first understand that some people call __cc_option
with a space after the ','.  Make treats this as important, thus these
two are different:

val := $(call cc-option, -opt1, -opt2)
val := $(call cc-option,-opt1,-opt2)

In one case val would be " -opt1" or " -opt2".  In the other case
"-opt1" or "-opt2".

However, when we cache things the space suddenly becomes irrelevant.
That's because we've end up with something like:

  __cached_result :=  -opt1

...and even though there are two spaces after the ":=" make throws that away.

The result of all of that is that we'd get slightly different numbers
of spaces after the cache was populated.  That could probably be
ignored except that some flags are additive and previous flags are
passed as arguments to later calls to __cc-option.  That means that
previous flags are passed to __sanitize-opt.  ...and in __sanitize-opt
spaces are significant.  ...so you'd come up with a slightly different
variable name on the first run and the subsequent runs...

Probably waaaay more than you expected for a simple $(strip) call, huh?

In any case this doesn't seem to matter anymore with your improvements
(I didn't dig into why), though I have noticed that the cache does
still return slightly different spacing for some of the results...

---

OK, v2 coming right up!  Thanks again for all your help on this!


-Doug

^ permalink raw reply

* [PATCH v2 0/2] kbuild: Cache exploratory calls to the compiler
From: Douglas Anderson @ 2017-10-04 22:37 UTC (permalink / raw)
  To: yamada.masahiro, mmarek
  Cc: groeck, sjg, briannorris, Douglas Anderson, Marcin Nowakowski,
	Matthias Kaehlcke, Cao jin, Arnd Bergmann, Mark Charlebois,
	linux-kbuild, linux-doc, Jonathan Corbet, linux-kernel,
	James Hogan, Josh Poimboeuf, Ingo Molnar

This two-patch series attempts to speed incremental builds of the
kernel up by a bit.  How much of a speedup you get depends a lot on
your environment, specifically the speed of your workstation and how
fast it takes to invoke the compiler.

In the Chrome OS build environment you get a really big win.  For an
incremental build (via emerge) I measured a speedup from ~1 minute to
~35 seconds.  ...but Chrome OS calls the compiler through a number of
wrapper scripts and also calls the kernel make at least twice for an
emerge (during compile stage and install stage), so it's a bit of a
worst case.

Perhaps a more realistic measure of the speedup others might see is
running "time make help > /dev/null" outside of the Chrome OS build
environment on my system.  When I do this I see that it took more than
1.0 seconds before and less than 0.2 seconds after.  So presumably
this has the ability to shave ~0.8 seconds off an incremental build
for most folks out there.  While 0.8 seconds savings isn't huge, it
does make incremental builds feel a lot snappier.

Caveats from v1 still copied here, though with Masahiro Yamada's
suggestions from v1 this is starting to feel a little more baked and
I've even dropped the RFC from it (though extra testing still
appreciated):

Please note that I make no illusions of being a Makefile expert nor do
I have any belief that I fully understand the Linux kernel build
system.  Please take this patch series as the start of a discussion
about whether others feel like this type of speedup is worthwhile and
how to best accomplish it.  Specific things to note:

- I'm happy to paint the bikeshed any color that maintainers want.  If
  you'd like the cache named differently, in a slightly different
  format, or you want me to adjust the spacing / names of Makefile
  stuff then please just let me know.

- If this is totally the wrong approach and you have a better idea
  then let me know.  If you want something that's super complicated to
  explain then feel free to post a replacement patch and I'm happy to
  test.

- This patch definitely needs extra testing.  I've tested it on a very
  limited build environment and it seems to be working fine, but I
  could believe that with some weird compiler options or on certain
  architectures you might need some extra escaping here and there.

Changes in v2:
- Abstract at a different level (like shell-cached) per Masahiro Yamada
- Include ld-version, which I missed the first time

Douglas Anderson (2):
  kbuild: Add a cache for generated variables
  kbuild: Cache a few more calls to the compiler

 Documentation/kbuild/makefiles.txt | 21 +++++++++
 Makefile                           |  4 +-
 scripts/Kbuild.include             | 90 ++++++++++++++++++++++++++++++++------
 3 files changed, 99 insertions(+), 16 deletions(-)

-- 
2.14.2.920.gcf0c67979c-goog


^ permalink raw reply

* [PATCH v2 1/2] kbuild: Add a cache for generated variables
From: Douglas Anderson @ 2017-10-04 22:37 UTC (permalink / raw)
  To: yamada.masahiro, mmarek
  Cc: groeck, sjg, briannorris, Douglas Anderson, Marcin Nowakowski,
	Matthias Kaehlcke, Cao jin, Arnd Bergmann, Mark Charlebois,
	linux-kbuild, linux-doc, Jonathan Corbet, linux-kernel,
	James Hogan, Josh Poimboeuf, Ingo Molnar
In-Reply-To: <20171004223717.3010-1-dianders@chromium.org>

While timing a "no-op" build of the kernel (incrementally building the
kernel even though nothing changed) in the Chrome OS build system I
found that it was much slower than I expected.

Digging into things a bit, I found that quite a bit of the time was
spent invoking the C compiler even though we weren't actually building
anything.  Currently in the Chrome OS build system the C compiler is
called through a number of wrappers (one of which is written in
python!) and can take upwards of 100 ms to invoke even if we're not
doing anything difficult, so these invocations of the compiler were
taking a lot of time.  Worse the invocations couldn't seem to take
advantage of the multiple cores on my system.

Certainly it seems like we could make the compiler invocations in the
Chrome OS build system faster, but only to a point.  Inherently
invoking a program as big as a C compiler is a fairly heavy
operation.  Thus even if we can speed the compiler calls it made sense
to track down what was happening.

It turned out that all the compiler invocations were coming from
usages like this in the kernel's Makefile:

KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)

Due to the way cc-option and similar statements work the above
contains an implicit call to the C compiler.  ...and due to the fact
that we're storing the result in KBUILD_CFLAGS, a simply expanded
variable, the call will happen every time the Makefile is parsed, even
if there are no users of KBUILD_CFLAGS.

Rather than redoing this computation every time, it makes a lot of
sense to cache the result of all of the Makefile's compiler calls just
like we do when we compile a ".c" file to a ".o" file.  Conceptually
this is quite a simple idea.  ...and since the calls to invoke the
compiler and similar tools are centrally located in the Kbuild.include
file this doesn't even need to be super invasive.

Implementing the cache in a simple-to-use and efficient way is not
quite as simple as it first sounds, though.  To get maximum speed we
really want the cache in a format that make can natively understand
and make doesn't really have an ability to load/parse files. ...but
make _can_ import other Makefiles, so the solution is to store the
cache in Makefile format.  This requires coming up with a valid/unique
Makefile variable name for each value to be cached, but that's
solvable with some cleverness.

After this change, we'll automatically create a ".cache.mk" file that
will contain our cached variables.  We'll load this on each invocation
of make and will avoid recomputing anything that's already in our
cache.  The cache is stored in a format that it shouldn't need any
invalidation since anything that might change should affect the "key"
and any old cached value won't be used.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- Abstract at a different level (like shell-cached) per Masahiro Yamada
- Include ld-version, which I missed the first time

 Documentation/kbuild/makefiles.txt | 21 +++++++++
 scripts/Kbuild.include             | 90 ++++++++++++++++++++++++++++++++------
 2 files changed, 97 insertions(+), 14 deletions(-)

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index 329e740adea7..679e8483cb4f 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -581,6 +581,27 @@ more details, with real examples.
 		LDFLAGS_vmlinux += $(call ld-option, -X)
 
 
+--- 3.14 $(LD) support function cache
+
+One thing to realize about all the calls to the above support functions
+is that each use of them requires a full invocation of an external tool, like
+the C compiler, assembler, or linker.  If nothing else that invocation will
+cause a fork/exec/shared library link.  In some build environments, however, it
+could also involve traversing thorough one or more wrappers.  To put some
+numbers on it, I've measured compiler invocations of as fast as 4ms or
+as slow as 150ms.
+
+Many of the above support functions are used in places where they are
+evaluated on each invocation of Make before anything else can run.  Even on
+a simple command like "make help" we need to evaluate every single one of the
+variables.
+
+To avoid this slow overhead we can cache the result of these invocations.
+If we store this cache in a way that's easy for Make to use (like in Makefile
+format) then it will be very quick and we'll save a lot of time with each
+invocation of make.
+
+
 === 4 Host Program support
 
 Kbuild supports building executables on the host for use during the
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 9ffd3dda3889..c539c709a00c 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -8,6 +8,8 @@ squote  := '
 empty   :=
 space   := $(empty) $(empty)
 space_escape := _-_SPACE_-_
+right_paren := )
+left_paren := (
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
@@ -69,6 +71,56 @@ endef
 # gcc support functions
 # See documentation in Documentation/kbuild/makefiles.txt
 
+# Tools for caching Makefile variables that are "expensive" to compute.
+#
+# Here we want to help deal with variables that take a long time to compute
+# by making it easy to store these variables in a cache.
+#
+# The canonical example here is testing for compiler flags.  On a simple system
+# each call to the compiler takes 10 ms, but on a system with a compiler that's
+# called through various wrappers it can take upwards of 100 ms.  If we have
+# 100 calls to the compiler this can take 1 second (on a simple system) or 10
+# seconds (on a complicated system).
+#
+# The "cache" will be in Makefile syntax and can be directly included.
+# Any time we try to reference a variable that's not in the cache we'll
+# calculate it and store it in the cache for next time.
+
+# Include values from last time
+make-cache := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/,$(if $(obj),$(obj)/)).cache.mk
+-include $(make-cache)
+
+# Usage: $(call __sanitize-opt,Hello=Hola$(comma)Goodbye Adios)
+#
+# Convert all '$', ')', '(', '\', '=', ' ', ',', ':' to '_'
+__sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(subst :,_,$(1)))))))))
+
+# Usage:   $(call shell-cached,shell_command)
+# Example: $(call shell-cached,md5sum /usr/bin/gcc)
+#
+# If we've already seen a call to this exact shell command (even in a
+# previous invocation of make!) we'll return the value.  If not, we'll
+# compute it and store the result for future runs.
+#
+# This is a bit of voodoo, but basic explanation is that if the variable
+# was undefined then we'll evaluate the shell command and store the result
+# into the variable.  We'll then store that value in the cache and finally
+# output the value.
+#
+# NOTE: The $$(2) here isn't actually a parameter to __run-and-store.  We
+# happen to know that the caller will have their shell command in $(2) so the
+# result of "call"ing this will produce a reference to that $(2).  The reason
+# for this strangeness is to avoid an extra level of eval (and escaping) of
+# $(2).
+define __run-and-store
+ifeq ($(origin $(1)),undefined)
+  $$(eval $(1) := $$(shell $$(2)))
+  $$(shell echo '$(1) := $$($(1))' >> $(make-cache))
+endif
+endef
+__shell-cached = $(eval $(call __run-and-store,$(1)))$($(1))
+shell-cached = $(call __shell-cached,__cached_$(call __sanitize-opt,$(1)),$(1))
+
 # cc-cross-prefix
 # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-)
 # Return first prefix where a prefix$(CC) is found in PATH.
@@ -87,30 +139,40 @@ TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
 # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
 # Exit code chooses option. "$$TMP" serves as a temporary file and is
 # automatically cleaned up.
-try-run = $(shell set -e;		\
+__try-run = set -e;			\
 	TMP="$(TMPOUT).$$$$.tmp";	\
 	TMPO="$(TMPOUT).$$$$.o";	\
 	if ($(1)) >/dev/null 2>&1;	\
 	then echo "$(2)";		\
 	else echo "$(3)";		\
 	fi;				\
-	rm -f "$$TMP" "$$TMPO")
+	rm -f "$$TMP" "$$TMPO"
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" serves as a temporary file and is
+# automatically cleaned up.
+try-run = $(shell $(__try-run))
+
+# try-run-cached
+# This works like try-run, but the result is cached.
+try-run-cached = $(call shell-cached,$(__try-run))
 
 # as-option
 # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
 
-as-option = $(call try-run,\
+as-option = $(call try-run-cached,\
 	$(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
 
 # as-instr
 # Usage: cflags-y += $(call as-instr,instr,option1,option2)
 
-as-instr = $(call try-run,\
+as-instr = $(call try-run-cached,\
 	printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
 
 # __cc-option
 # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
-__cc-option = $(call try-run,\
+__cc-option = $(call try-run-cached,\
 	$(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
 
 # Do not attempt to build with gcc plugins during cc-option tests.
@@ -130,23 +192,23 @@ hostcc-option = $(call __cc-option, $(HOSTCC),\
 
 # cc-option-yn
 # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
-cc-option-yn = $(call try-run,\
+cc-option-yn = $(call try-run-cached,\
 	$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n)
 
 # cc-disable-warning
 # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
-cc-disable-warning = $(call try-run,\
+cc-disable-warning = $(call try-run-cached,\
 	$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
 
 # cc-name
 # Expands to either gcc or clang
-cc-name = $(shell $(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)
+cc-name = $(call shell-cached,$(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)
 
 # cc-version
-cc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
+cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
 
 # cc-fullversion
-cc-fullversion = $(shell $(CONFIG_SHELL) \
+cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
 	$(srctree)/scripts/gcc-version.sh -p $(CC))
 
 # cc-ifversion
@@ -159,22 +221,22 @@ cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo
 
 # cc-ldoption
 # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
-cc-ldoption = $(call try-run,\
+cc-ldoption = $(call try-run-cached,\
 	$(CC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
 
 # ld-option
 # Usage: LDFLAGS += $(call ld-option, -X)
-ld-option = $(call try-run,\
+ld-option = $(call try-run-cached,\
 	$(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
 
 # ar-option
 # Usage: KBUILD_ARFLAGS := $(call ar-option,D)
 # Important: no spaces around options
-ar-option = $(call try-run, $(AR) rc$(1) "$$TMP",$(1),$(2))
+ar-option = $(call try-run-cached, $(AR) rc$(1) "$$TMP",$(1),$(2))
 
 # ld-version
 # Note this is mainly for HJ Lu's 3 number binutil versions
-ld-version = $(shell $(LD) --version | $(srctree)/scripts/ld-version.sh)
+ld-version = $(call shell-cached,$(LD) --version | $(srctree)/scripts/ld-version.sh)
 
 # ld-ifversion
 # Usage:  $(call ld-ifversion, -ge, 22252, y)
-- 
2.14.2.920.gcf0c67979c-goog


^ permalink raw reply related

* [PATCH v2 2/2] kbuild: Cache a few more calls to the compiler
From: Douglas Anderson @ 2017-10-04 22:37 UTC (permalink / raw)
  To: yamada.masahiro, mmarek
  Cc: groeck, sjg, briannorris, Douglas Anderson, linux-kernel,
	linux-kbuild
In-Reply-To: <20171004223717.3010-1-dianders@chromium.org>

These are a few stragglers that I left out of the original patch to
cache calls to the C compiler ("kbuild: Add a cache for generated
variables") because they bleed out into the main Makefile and thus
uglify things a little bit.  The idea is the same here, though.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- Abstract at a different level (like shell-cached) per Masahiro Yamada

 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d1119941261c..d366bdc3da04 100644
--- a/Makefile
+++ b/Makefile
@@ -652,7 +652,7 @@ KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
 # check for 'asm goto'
-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
+ifeq ($(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
 	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
 	KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
 endif
@@ -788,7 +788,7 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 # arch Makefile may override CC so keep this after arch Makefile is included
-NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
+NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC) -print-file-name=include)
 CHECKFLAGS     += $(NOSTDINC_FLAGS)
 
 # warn about C99 declaration after statement
-- 
2.14.2.920.gcf0c67979c-goog


^ permalink raw reply related

* Re: [PATCH 1/7] xen/arm: domain_build: Clean-up insert_11_bank
From: Andrew Cooper @ 2017-10-04 22:39 UTC (permalink / raw)
  To: Julien Grall, xen-devel; +Cc: Stefano Stabellini
In-Reply-To: <20171004181526.9405-2-julien.grall@linaro.org>

On 04/10/2017 19:15, Julien Grall wrote:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 3723dc3f78..093ebf1a8e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -109,11 +109,11 @@ static bool insert_11_bank(struct domain *d,
>  
>      spfn = page_to_mfn(pg);
>      start = pfn_to_paddr(spfn);
> -    size = pfn_to_paddr((1 << order));
> +    size = pfn_to_paddr(1UL << order);
>  
>      D11PRINT("Allocated %#"PRIpaddr"-%#"PRIpaddr" (%ldMB/%ldMB, order %d)\n",
>               start, start + size,
> -             1UL << (order+PAGE_SHIFT-20),
> +             1UL << (order + PAGE_SHIFT -  20),

If you are looking to be picky, you've got a double space between the
minus and the 20.  I'm sure this would be trivial to fix on commit.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* Re: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
From: Josh Poimboeuf @ 2017-10-04 22:40 UTC (permalink / raw)
  To: lkp
In-Reply-To: <201710040644.GJE89219.FOHMFSFOVtOQLJ@I-love.SAKURA.ne.jp>

[-- Attachment #1: Type: text/plain, Size: 4603 bytes --]

On Wed, Oct 04, 2017 at 06:44:50AM +0900, Tetsuo Handa wrote:
> Josh Poimboeuf wrote:
> > On Tue, Oct 03, 2017 at 11:28:15AM -0500, Josh Poimboeuf wrote:
> > > There are two bugs:
> > > 
> > > 1) Somebody -- presumably lockdep -- is corrupting the stack.  Need the
> > >    lockdep people to look at that.
> > > 
> > > 2) The 32-bit FP unwinder isn't handling the corrupt stack very well,
> > >    It's blindly dereferencing untrusted data:
> > > 
> > > 	/* Is the next frame pointer an encoded pointer to pt_regs? */
> > > 	regs = decode_frame_pointer(next_bp);
> > > 	if (regs) {
> > > 		frame = (unsigned long *)regs;
> > > 		len = regs_size(regs);
> > > 		state->got_irq = true;
> > > 
> > >   On 32-bit, regs_size() dereferences the regs pointer before we know it
> > >   points to a valid stack.  I'll fix that, along with the other unwinder
> > >   improvements I discussed with Linus.
> > 
> > Tetsuo and/or Fengguang,
> > 
> > Would you mind testing with this patch?  It should at least prevent the
> > unwinder panic and should hopefully print a useful unwinder dump
> > instead.
> > 
> Here are two outputs.

Tetsuo, would you mind trying the following patch?


diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 8a13d468635a..50e0d2bc4528 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -176,7 +176,7 @@
 /*
  * This is a sneaky trick to help the unwinder find pt_regs on the stack.  The
  * frame pointer is replaced with an encoded pointer to pt_regs.  The encoding
- * is just setting the LSB, which makes it an invalid stack address and is also
+ * is just clearing the MSB, which makes it an invalid stack address and is also
  * a signal to the unwinder that it's a pt_regs pointer in disguise.
  *
  * NOTE: This macro must be used *after* SAVE_ALL because it corrupts the
@@ -185,7 +185,7 @@
 .macro ENCODE_FRAME_POINTER
 #ifdef CONFIG_FRAME_POINTER
 	mov %esp, %ebp
-	orl $0x1, %ebp
+	andl $0x7fffffff, %ebp
 #endif
 .endm
 
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..acefab447f00 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -44,7 +44,8 @@ static void unwind_dump(struct unwind_state *state)
 			state->stack_info.type, state->stack_info.next_sp,
 			state->stack_mask, state->graph_idx);
 
-	for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
+	for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp;
+	     sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
 		if (get_stack_info(sp, state->task, &stack_info, &visit_mask))
 			break;
 
@@ -77,6 +78,12 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
+#ifdef CONFIG_X86_32
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long))
+#else
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs))
+#endif
+
 static bool in_entry_code(unsigned long ip)
 {
 	char *addr = (char *)ip;
@@ -174,6 +181,7 @@ static bool is_last_task_frame(struct unwind_state *state)
  * This determines if the frame pointer actually contains an encoded pointer to
  * pt_regs on the stack.  See ENCODE_FRAME_POINTER.
  */
+#ifdef CONFIG_X86_64
 static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 {
 	unsigned long regs = (unsigned long)bp;
@@ -183,6 +191,17 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 
 	return (struct pt_regs *)(regs & ~0x1);
 }
+#else
+static struct pt_regs *decode_frame_pointer(unsigned long *bp)
+{
+	unsigned long regs = (unsigned long)bp;
+
+	if (regs & 0x80000000)
+		return NULL;
+
+	return (struct pt_regs *)(regs | 0x80000000);
+}
+#endif
 
 static bool update_stack_state(struct unwind_state *state,
 			       unsigned long *next_bp)
@@ -202,7 +221,7 @@ static bool update_stack_state(struct unwind_state *state,
 	regs = decode_frame_pointer(next_bp);
 	if (regs) {
 		frame = (unsigned long *)regs;
-		len = regs_size(regs);
+		len = KERNEL_REGS_SIZE;
 		state->got_irq = true;
 	} else {
 		frame = next_bp;
@@ -226,6 +245,14 @@ static bool update_stack_state(struct unwind_state *state,
 	    frame < prev_frame_end)
 		return false;
 
+	/*
+	 * On 32-bit with user mode regs, make sure the last two regs are safe
+	 * to access:
+	 */
+	if (IS_ENABLED(CONFIG_X86_32) && regs && user_mode(regs) &&
+	    !on_stack(info, frame, len + 2*sizeof(long)))
+		return false;
+
 	/* Move state to the next frame: */
 	if (regs) {
 		state->regs = regs;

^ permalink raw reply related

* Re: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
From: Josh Poimboeuf @ 2017-10-04 22:40 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: fengguang.wu, byungchul.park, mingo, peterz, linux-kernel, lkp,
	torvalds, bp, x86, hpa, tglx
In-Reply-To: <201710040644.GJE89219.FOHMFSFOVtOQLJ@I-love.SAKURA.ne.jp>

On Wed, Oct 04, 2017 at 06:44:50AM +0900, Tetsuo Handa wrote:
> Josh Poimboeuf wrote:
> > On Tue, Oct 03, 2017 at 11:28:15AM -0500, Josh Poimboeuf wrote:
> > > There are two bugs:
> > > 
> > > 1) Somebody -- presumably lockdep -- is corrupting the stack.  Need the
> > >    lockdep people to look at that.
> > > 
> > > 2) The 32-bit FP unwinder isn't handling the corrupt stack very well,
> > >    It's blindly dereferencing untrusted data:
> > > 
> > > 	/* Is the next frame pointer an encoded pointer to pt_regs? */
> > > 	regs = decode_frame_pointer(next_bp);
> > > 	if (regs) {
> > > 		frame = (unsigned long *)regs;
> > > 		len = regs_size(regs);
> > > 		state->got_irq = true;
> > > 
> > >   On 32-bit, regs_size() dereferences the regs pointer before we know it
> > >   points to a valid stack.  I'll fix that, along with the other unwinder
> > >   improvements I discussed with Linus.
> > 
> > Tetsuo and/or Fengguang,
> > 
> > Would you mind testing with this patch?  It should at least prevent the
> > unwinder panic and should hopefully print a useful unwinder dump
> > instead.
> > 
> Here are two outputs.

Tetsuo, would you mind trying the following patch?


diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 8a13d468635a..50e0d2bc4528 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -176,7 +176,7 @@
 /*
  * This is a sneaky trick to help the unwinder find pt_regs on the stack.  The
  * frame pointer is replaced with an encoded pointer to pt_regs.  The encoding
- * is just setting the LSB, which makes it an invalid stack address and is also
+ * is just clearing the MSB, which makes it an invalid stack address and is also
  * a signal to the unwinder that it's a pt_regs pointer in disguise.
  *
  * NOTE: This macro must be used *after* SAVE_ALL because it corrupts the
@@ -185,7 +185,7 @@
 .macro ENCODE_FRAME_POINTER
 #ifdef CONFIG_FRAME_POINTER
 	mov %esp, %ebp
-	orl $0x1, %ebp
+	andl $0x7fffffff, %ebp
 #endif
 .endm
 
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..acefab447f00 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -44,7 +44,8 @@ static void unwind_dump(struct unwind_state *state)
 			state->stack_info.type, state->stack_info.next_sp,
 			state->stack_mask, state->graph_idx);
 
-	for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
+	for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp;
+	     sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
 		if (get_stack_info(sp, state->task, &stack_info, &visit_mask))
 			break;
 
@@ -77,6 +78,12 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
+#ifdef CONFIG_X86_32
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long))
+#else
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs))
+#endif
+
 static bool in_entry_code(unsigned long ip)
 {
 	char *addr = (char *)ip;
@@ -174,6 +181,7 @@ static bool is_last_task_frame(struct unwind_state *state)
  * This determines if the frame pointer actually contains an encoded pointer to
  * pt_regs on the stack.  See ENCODE_FRAME_POINTER.
  */
+#ifdef CONFIG_X86_64
 static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 {
 	unsigned long regs = (unsigned long)bp;
@@ -183,6 +191,17 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 
 	return (struct pt_regs *)(regs & ~0x1);
 }
+#else
+static struct pt_regs *decode_frame_pointer(unsigned long *bp)
+{
+	unsigned long regs = (unsigned long)bp;
+
+	if (regs & 0x80000000)
+		return NULL;
+
+	return (struct pt_regs *)(regs | 0x80000000);
+}
+#endif
 
 static bool update_stack_state(struct unwind_state *state,
 			       unsigned long *next_bp)
@@ -202,7 +221,7 @@ static bool update_stack_state(struct unwind_state *state,
 	regs = decode_frame_pointer(next_bp);
 	if (regs) {
 		frame = (unsigned long *)regs;
-		len = regs_size(regs);
+		len = KERNEL_REGS_SIZE;
 		state->got_irq = true;
 	} else {
 		frame = next_bp;
@@ -226,6 +245,14 @@ static bool update_stack_state(struct unwind_state *state,
 	    frame < prev_frame_end)
 		return false;
 
+	/*
+	 * On 32-bit with user mode regs, make sure the last two regs are safe
+	 * to access:
+	 */
+	if (IS_ENABLED(CONFIG_X86_32) && regs && user_mode(regs) &&
+	    !on_stack(info, frame, len + 2*sizeof(long)))
+		return false;
+
 	/* Move state to the next frame: */
 	if (regs) {
 		state->regs = regs;

^ permalink raw reply related

* Re: [RFC] bpf: remove global verifier state
From: Alexei Starovoitov @ 2017-10-04 22:40 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Daniel Borkmann, Eric Dumazet, dsahern, netdev, oss-drivers,
	david.beckett
In-Reply-To: <20171004145744.27c08d4a@cakuba.netronome.com>

On Wed, Oct 04, 2017 at 02:57:44PM -0700, Jakub Kicinski wrote:
> On Wed, 04 Oct 2017 21:13:47 +0200, Daniel Borkmann wrote:
> > On 10/04/2017 05:43 AM, Alexei Starovoitov wrote:
> > > On Tue, Oct 03, 2017 at 08:24:06PM -0700, Eric Dumazet wrote:  
> > >> On Tue, 2017-10-03 at 19:52 -0700, Alexei Starovoitov wrote:
> > >>  
> > >>> yep. looks great.
> > >>> Please test it and submit officially :)
> > >>> The commit aafe6ae9cee3 ("bpf: dynamically allocate digest scratch buffer")
> > >>> fixed the other case where we were relying on the above mutex.
> > >>> The only other spot to be adjusted is to add spin_lock/mutex or DO_ONCE() to
> > >>> bpf_get_skb_set_tunnel_proto() to protect md_dst init.
> > >>> imo that would be it.
> > >>> Daniel, anything else comes to mind?  
> > 
> > Yes, this should be all. DO_ONCE() for the tunnel proto seems a
> > good choice.
> 
> Hm.  I actually did:
> 
> if (!dst) {
> 	tmp = alloc();
> 	if (!tmp)
> 		return;
> 	if (cmpxchg(&dst, NULL, tmp))
> 		free(tmp);
> }
> 
> I don't like how DO_ONCE() doesn't handle errors from the init
> function :(

yeah. good point.
Above looks good to me.

^ permalink raw reply

* Re: [PATCH 3/7] xen/x86: Use maddr_to_page and maddr_to_mfn to avoid open-coded >> PAGE_SHIFT
From: Andrew Cooper @ 2017-10-04 22:41 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Elena Ufimtseva, George Dunlap, Tim Deegan, Jan Beulich
In-Reply-To: <20171004181526.9405-4-julien.grall@linaro.org>

On 04/10/2017 19:15, Julien Grall wrote:
> The constructions _mfn(... > PAGE_SHIFT) and mfn_to_page(... >> PAGE_SHIFT)
> could respectively be replaced by maddr_to_mfn(...) and
> maddr_to_page(...).
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [PATCH 0/2] [v5] pinctrl: qcom: add support for sparse GPIOs
From: Timur Tabi @ 2017-10-04 22:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171004215023.GA457@codeaurora.org>

On 10/04/2017 04:50 PM, Stephen Boyd wrote:

>> At the time I wrote that patch, the ACPI tables exposed all of the
>> GPIOs, even the ones it didn't care about.  The new ACPI tables list
>> only specific GPIOs, and so we no longer need to blindly read the
>> direction of all GPIOs.
>>
> 
> Do you avoid this problem on new ACPI tables because only pins
> that are able to be read are exposed?

Yes.  A recent firmware update enabled the "XPU" block which is being 
programmed with a select subset of individual GPIOs.  On our silicon, 
each TLMM GPIO is in a separate 64k page, and so the XPU can block any 
individual GPIO.  Any attempt to touch those registers causes an XPU 
violation which takes the whole system down.

>>> This is basically a revert of commit 72d320006177 ("gpio: set up
>>> initial state from .get_direction()").
>>
>> I would be in favor of either reverting that patch, or moving the
>> code into gpiochip_add_pin_range().
> 
> If it's in gpiochip_add_pin_range() would we still read the
> hardware when creating the pin ranges?

I presume so.  The idea is that pinctrl-qdf2xxx/pinctrl-msm only submit 
pin ranges that are present in the ACPI tables.

 > I don't want to have to> describe pin ranges of "valid" pins that 
won't cause the system
> to blow up if we touch them, because those pins are never used by
> Linux so reading them is not useful.

Well, that's exactly what I'm trying to do with current patch set :-) 
It seems the most logical approach to me.  I don't understand the 
dislike for it.  What else are pin ranges for, other than to specify 
ranges of pins that can be accessed?

Another alternative was to enumerate all of the GPIOs starting from 0. 
So the first GPIO in ACPI would be gpio0, regardless of what gpio number 
it actually was.  E.g. GPIO 37 would appear as gpio0, GPIO 38 would 
appear as gpio1, and so on.  That also worked, but it meant that 
customers would need to figure out which GPIO that "gpio0" actually 
pointed to.  That was not acceptable, so I dropped it.

I'm at a loss on how else to do it.  I think a gpio_chip.available 
callback is far less elegant than define pin ranges.  There is no chance 
that unavailable GPIOs can be accessed because the physical addresses 
are not in the msm_pingroup array.  That is, groups[0].ctrl_reg == 0, 
not 0xFF02010000.


-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.