LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add dual-fifo mode support of i.MX ssi
From: Nicolin Chen @ 2013-10-31  6:32 UTC (permalink / raw)
  To: s.hauer, timur, shawn.guo, broonie
  Cc: mark.rutland, devicetree, alsa-devel, pawel.moll, linux-doc,
	swarren, linux-kernel, rob.herring, galak, dmaengine,
	ijc+devicetree, linuxppc-dev, linux-arm-kernel

Changelog
v2:
 * Instead of adding rogue scripts to current SDMA driver based on firmware
 * V1, we define the new SDMA firmware as version 2 and bisect the PATCH-1
 * to two patches: The first is to add version check code to the SDMA driver;
 * And the second is to add SSI dual FIFO DMATYPE.
 *
 * Nothing changes for the last two patches.
v1:
 * SSI can reduce hardware overrun/underrun possibility when using dual
 * fifo mode. To support this mode, we need to first update sdma sciprt
 * list, and then enable dual fifo BIT in SSI driver, and last update DT
 * bindings of i.MX series.
 *
 * ! This series of patches has a direct dependency between them. When
 * ! applying them, we need to apply in one single branch. Otherwise,
 * ! it would break currect branches.

Nicolin Chen (4):
  dma: imx-sdma: Add sdma firmware version 2 support
  dma: imx-sdma: Add new dma type for ssi dual fifo script
  ASoC: fsl_ssi: Add dual fifo mode support
  ARM: dts: imx: use dual-fifo sdma script for ssi

 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |  1 +
 arch/arm/boot/dts/imx51.dtsi                       |  4 ++--
 arch/arm/boot/dts/imx53.dtsi                       |  4 ++--
 arch/arm/boot/dts/imx6qdl.dtsi                     | 12 +++++------
 arch/arm/boot/dts/imx6sl.dtsi                      | 12 +++++------
 drivers/dma/imx-sdma.c                             | 19 ++++++++++++++++-
 include/linux/platform_data/dma-imx-sdma.h         |  3 +++
 include/linux/platform_data/dma-imx.h              |  1 +
 sound/soc/fsl/fsl_ssi.c                            | 24 +++++++++++++++++++++-
 9 files changed, 62 insertions(+), 18 deletions(-)

-- 
1.8.4

^ permalink raw reply

* Re: [PATCH 2/2] of: move definition of of_find_next_cache_node into common code.
From: Benjamin Herrenschmidt @ 2013-10-31  5:20 UTC (permalink / raw)
  To: Sudeep KarkadaNagesha; +Cc: Grant Likely, devicetree, linuxppc-dev, Rob Herring
In-Reply-To: <1379501585-12532-3-git-send-email-Sudeep.KarkadaNagesha@arm.com>

On Wed, 2013-09-18 at 11:53 +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> Since the definition of_find_next_cache_node is architecture independent,
> the existing definition in powerpc can be moved to driver/of/base.c
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

I've seen no follow up on that, I'm happy to stick it in powerpc-next
with some other late stuff.

Cheers,
Ben.

> ---
>  arch/powerpc/include/asm/prom.h |  3 ---
>  arch/powerpc/kernel/prom.c      | 31 -------------------------------
>  drivers/of/base.c               | 31 +++++++++++++++++++++++++++++++
>  include/linux/of.h              |  2 ++
>  4 files changed, 33 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 7d0c7f3..bf09e5a 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -44,9 +44,6 @@ void of_parse_dma_window(struct device_node *dn, const __be32 *dma_window,
>  
>  extern void kdump_move_device_tree(void);
>  
> -/* cache lookup */
> -struct device_node *of_find_next_cache_node(struct device_node *np);
> -
>  #ifdef CONFIG_NUMA
>  extern int of_node_to_nid(struct device_node *device);
>  #else
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 09be275..4432fd8 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -761,37 +761,6 @@ void __init early_init_devtree(void *params)
>   *******/
>  
>  /**
> - *	of_find_next_cache_node - Find a node's subsidiary cache
> - *	@np:	node of type "cpu" or "cache"
> - *
> - *	Returns a node pointer with refcount incremented, use
> - *	of_node_put() on it when done.  Caller should hold a reference
> - *	to np.
> - */
> -struct device_node *of_find_next_cache_node(struct device_node *np)
> -{
> -	struct device_node *child;
> -	const phandle *handle;
> -
> -	handle = of_get_property(np, "l2-cache", NULL);
> -	if (!handle)
> -		handle = of_get_property(np, "next-level-cache", NULL);
> -
> -	if (handle)
> -		return of_find_node_by_phandle(be32_to_cpup(handle));
> -
> -	/* OF on pmac has nodes instead of properties named "l2-cache"
> -	 * beneath CPU nodes.
> -	 */
> -	if (!strcmp(np->type, "cpu"))
> -		for_each_child_of_node(np, child)
> -			if (!strcmp(child->type, "cache"))
> -				return child;
> -
> -	return NULL;
> -}
> -
> -/**
>   * of_get_ibm_chip_id - Returns the IBM "chip-id" of a device
>   * @np: device node of the device
>   *
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 865d3f6..b2cee3d 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1884,3 +1884,34 @@ int of_device_is_stdout_path(struct device_node *dn)
>  	return of_stdout == dn;
>  }
>  EXPORT_SYMBOL_GPL(of_device_is_stdout_path);
> +
> +/**
> + *	of_find_next_cache_node - Find a node's subsidiary cache
> + *	@np:	node of type "cpu" or "cache"
> + *
> + *	Returns a node pointer with refcount incremented, use
> + *	of_node_put() on it when done.  Caller should hold a reference
> + *	to np.
> + */
> +struct device_node *of_find_next_cache_node(const struct device_node *np)
> +{
> +	struct device_node *child;
> +	const phandle *handle;
> +
> +	handle = of_get_property(np, "l2-cache", NULL);
> +	if (!handle)
> +		handle = of_get_property(np, "next-level-cache", NULL);
> +
> +	if (handle)
> +		return of_find_node_by_phandle(be32_to_cpup(handle));
> +
> +	/* OF on pmac has nodes instead of properties named "l2-cache"
> +	 * beneath CPU nodes.
> +	 */
> +	if (!strcmp(np->type, "cpu"))
> +		for_each_child_of_node(np, child)
> +			if (!strcmp(child->type, "cache"))
> +				return child;
> +
> +	return NULL;
> +}
> diff --git a/include/linux/of.h b/include/linux/of.h
> index f95aee3..c08c07e 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -226,6 +226,8 @@ static inline int of_get_child_count(const struct device_node *np)
>  	return num;
>  }
>  
> +/* cache lookup */
> +extern struct device_node *of_find_next_cache_node(const struct device_node *);
>  extern struct device_node *of_find_node_with_property(
>  	struct device_node *from, const char *prop_name);
>  #define for_each_node_with_property(dn, prop_name) \

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Paul E. McKenney @ 2013-10-31  4:32 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, Peter Zijlstra, LKML,
	Oleg Nesterov, Linux PPC dev, Anton Blanchard,
	Frederic Weisbecker
In-Reply-To: <OFC6B7100E.31C6B190-ON42257C14.004841C6-42257C14.004A0EA6@il.ibm.com>

On Wed, Oct 30, 2013 at 03:28:54PM +0200, Victor Kaplansky wrote:
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote on 10/30/2013
> 11:27:25 AM:
> 
> > If you were to back up that insistence with a description of the
> orderings
> > you are relying on, why other orderings are not important, and how the
> > important orderings are enforced, I might be tempted to pay attention
> > to your opinion.
> >
> >                      Thanx, Paul
> 
> NP, though, I feel too embarrassed to explain things about memory barriers
> when
> one of the authors of Documentation/memory-barriers.txt is on cc: list ;-)
> 
> Disclaimer: it is anyway impossible to prove lack of *any* problem.

If you want to play the "omit memory barriers" game, then proving a
negative is in fact the task before you.

> Having said that, lets look into an example in
> Documentation/circular-buffers.txt:

And the correctness of this code has been called into question.  :-(
An embarrassingly long time ago -- I need to get this either proven
or fixed.

> > THE PRODUCER
> > ------------
> >
> > The producer will look something like this:
> >
> >       spin_lock(&producer_lock);
> >
> >       unsigned long head = buffer->head;
> >       unsigned long tail = ACCESS_ONCE(buffer->tail);
> >
> >       if (CIRC_SPACE(head, tail, buffer->size) >= 1) {
> >               /* insert one item into the buffer */
> >               struct item *item = buffer[head];
> >
> >               produce_item(item);
> >
> >               smp_wmb(); /* commit the item before incrementing the head
> */
> >
> >               buffer->head = (head + 1) & (buffer->size - 1);
> >
> >               /* wake_up() will make sure that the head is committed
> before
> >                * waking anyone up */
> >               wake_up(consumer);
> >       }
> >
> >       spin_unlock(&producer_lock);
> 
> We can see that authors of the document didn't put any memory barrier
> after "buffer->tail" read and before "produce_item(item)" and I think they
> have
> a good reason.
> 
> Lets consider an imaginary smp_mb() right before "produce_item(item);".
> Such a barrier will ensure that -
> 
>     - the memory read on "buffer->tail" is completed
> 	before store to memory pointed by "item" is committed.
> 
> However, the store to "buffer->tail" anyway cannot be completed before
> conditional
> branch implied by "if ()" is proven to execute body statement of the if().
> And the
> latter cannot be proven before read of "buffer->tail" is completed.
> 
> Lets see this other way. Lets imagine that somehow a store to the data
> pointed by "item"
> is completed before we read "buffer->tail". That would mean, that the store
> was completed
> speculatively. But speculative execution of conditional stores is
> prohibited by C/C++ standard,
> otherwise any conditional store at any random place of code could pollute
> shared memory.

Before C/C++11, the closest thing to such a prohibition is use of
volatile, for example, ACCESS_ONCE().  Even in C/C++11, you have to
use atomics to get anything resembing this prohibition.

If you just use normal variables, the compiler is within its rights
to transform something like the following:

	if (a)
		b = 1;
	else
		b = 42;

Into:

	b = 42;
	if (a)
		b = 1;

Many other similar transformations are permitted.  Some are used to all
vector instructions to be used -- the compiler can do a write with an
overly wide vector instruction, then clean up the clobbered variables
later, if it wishes.  Again, if the variables are not marked volatile,
or, in C/C++11, atomic.

> On the other hand, if compiler or processor can prove that condition in
> above if() is going
> to be true (or if speculative store writes the same value as it was before
> write), the
> speculative store *is* allowed. In this case we should not be bothered by
> the fact that
> memory pointed by "item" is written before a read from "buffer->tail" is
> completed.

The compilers don't always know as much as they might about the underlying
hardware's memory model.  Of course, if this code is architecture specific,
it can avoid DEC Alpha's fun and games, which could also violate your
assumptions in the above paragraph:

	http://www.openvms.compaq.com/wizard/wiz_2637.html

Anyway, proving or fixing the code in Documentation/circular-buffers.txt
has been on my list for too long, so I will take a closer look at it.

							Thanx, Paul

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Paul E. McKenney @ 2013-10-31  4:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker,
	Victor Kaplansky
In-Reply-To: <20131030155116.GO16117@laptop.programming.kicks-ass.net>

On Wed, Oct 30, 2013 at 04:51:16PM +0100, Peter Zijlstra wrote:
> On Wed, Oct 30, 2013 at 03:28:54PM +0200, Victor Kaplansky wrote:
> > one of the authors of Documentation/memory-barriers.txt is on cc: list ;-)
> > 
> > Disclaimer: it is anyway impossible to prove lack of *any* problem.
> > 
> > Having said that, lets look into an example in
> > Documentation/circular-buffers.txt:
> 
> > 
> > We can see that authors of the document didn't put any memory barrier
> 
> Note that both documents have the same author list ;-)
> 
> Anyway, I didn't know about the circular thing, I suppose I should use
> CIRC_SPACE() thing :-)

Interesting that we didn't seem to supply a definition...  ;-)

							Thanx, Paul

^ permalink raw reply

* [Suggestion] drivers: powercap: 'dev_attrs' has already removed from 'struct class'
From: Chen Gang @ 2013-10-31  2:18 UTC (permalink / raw)
  To: srinivas.pandruvada, jacob.jun.pan, rafael.j.wysocki@intel.com,
	linuxppc-dev@lists.ozlabs.org
  Cc: Greg KH

Hello Maintainers

It is removed by "bcc8edb driver core: remove dev_attrs from struct
class" in Oct 5 2013. But "75d2364 PowerCap: Add class driver" still
use it in Oct 11 2013.

The related error (for powerpc with allmodconfig):

  CC      drivers/powercap/powercap_sys.o
drivers/powercap/powercap_sys.c:484:2: error: unknown field ‘dev_attrs’ specified in initializer
drivers/powercap/powercap_sys.c:484:2: warning: initialization from incompatible pointer type [enabled by default]
drivers/powercap/powercap_sys.c:484:2: warning: (near initialization for ‘powercap_class.suspend’) [enabled by default]


Please give a check thanks.

Thanks.
-- 
Chen Gang

^ permalink raw reply

* Re: [PATCH] offb: make the screen properties endian safe
From: Benjamin Herrenschmidt @ 2013-10-31  2:08 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <1383149647-15500-1-git-send-email-clg@fr.ibm.com>

On Wed, 2013-10-30 at 17:14 +0100, Cédric Le Goater wrote:
> @@ -552,25 +552,25 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
>  	if (pp == NULL)
>  		pp = of_get_property(dp, "depth", &len);
>  	if (pp && len == sizeof(u32))
> -		depth = *pp;
> +		depth = be32_to_cpu(*pp);

This is usually written as

		depth = be32_to_cpup(pp);

It used to be that the latter generated better code but that might not
be the case anymore, however it's still a better alternative.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] ADB_PMU_LED_IDE selects LEDS_TRIGGER_IDE_DISK which has unmet direct dependencies
From: Benjamin Herrenschmidt @ 2013-10-31  1:37 UTC (permalink / raw)
  To: Christian Kujau; +Cc: Geert Uytterhoeven, linuxppc-dev
In-Reply-To: <alpine.DEB.2.11.1310292123190.31521@trent.utfs.org>

On Tue, 2013-10-29 at 21:25 -0700, Christian Kujau wrote:
> On Wed, 30 Oct 2013 at 10:13, Benjamin Herrenschmidt wrote:
> > You probably want to do that to the ADB_PMU_LED_IDE entry not the
> > ADB_PMU_LED one which doesn't have a dependency and isn't the one
> > selecting LEDS_TRIGGER_IDE_DISK :-)
> 
> Right you are, sorry for the mixup. Let me try again:

And this time without a changeset comment ? :-)

I've copied it from the previous version but next time, re-send the
complete commit.

Cheers,
Ben.

> 
>   Signed-off-by: Christian Kujau <lists@nerdbynature.de>
> 
> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> index 696238b..d26a312 100644
> --- a/drivers/macintosh/Kconfig
> +++ b/drivers/macintosh/Kconfig
> @@ -103,6 +103,7 @@ config ADB_PMU_LED_IDE
>         bool "Use front LED as IDE LED by default"
>         depends on ADB_PMU_LED
>         depends on LEDS_CLASS
> +       depends on IDE_GD_ATA
>         select LEDS_TRIGGERS
>         select LEDS_TRIGGER_IDE_DISK
>         help
> 
> C.
> -- 
> BOFH excuse #378:
> 
> Operators killed by year 2000 bug bite.

^ permalink raw reply

* Re: [PATCH v2] powerpc/mpc512x: remove unnecessary #if
From: Anatolij Gustschin @ 2013-10-30 22:12 UTC (permalink / raw)
  To: Brian Norris; +Cc: Gerhard Sittig, linuxppc-dev
In-Reply-To: <1381513058-14164-1-git-send-email-computersforpeace@gmail.com>

On Fri, 11 Oct 2013 10:37:38 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> Several functions are only ever referenced locally, so make them static.
> Of those functions, many of them are protected by an #if. However, the
> code which can compile fine in either case.
> 
> Now that (1) the unneeded code is marked 'static' and (2) the code is
> only used under a C 'if (IS_ENABLED(CONFIG_FB_FSL_DIU))', the compiler
> can automatically remove the unneeded code, and we don't need the #if or
> the empty stub functions.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> v2: left out a diff I was holding locally (to remove an #if/#endif
>     completely). Sorry for the noise.
> 
> Based off of Gerhard Sittig's patch:
>   powerpc/mpc512x: silence build warning upon disabled DIU
> 
> Compile-tested with CONFIG_FB_FSL_DIU=n
> 
>  arch/powerpc/platforms/512x/mpc512x_shared.c | 21 +++++++--------------
>  arch/powerpc/sysdev/fsl_soc.h                |  3 ---
>  2 files changed, 7 insertions(+), 17 deletions(-)

Applied, thanks!

Anatolij

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode
From: Tom Musta @ 2013-10-30 21:45 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linuxppc-dev, tmusta, Andreas Schwab
In-Reply-To: <CAMuHMdWMZNF_KOMyVES2UbvCyGEH1N=CFp-TTWuLMzm89SLs3A@mail.gmail.com>

On 10/30/2013 2:43 PM, Geert Uytterhoeven wrote:
> On Wed, Oct 30, 2013 at 8:35 PM, Tom Musta <tommusta@gmail.com> wrote:
>> On 10/30/2013 12:43 PM, Andreas Schwab wrote:
>>>
>>> Tom Musta <tommusta@gmail.com> writes:
>>>
>>>> +#ifdef __LITTLE_ENDIAN__
>>>> +       if (!regs->msr & MSR_LE)
>>>
>>>
>>> That won't work.
>>>
>>> Andreas.
>>>
>>
>> Please elaborate.
>
> You want to test for "!(regs & MSR_LE)".
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>

Thanks Adnreas and Geert.  I will fix and resubmit.

^ permalink raw reply

* [PATCH v4 2/7] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
From: Yann Droneaud @ 2013-10-30 19:47 UTC (permalink / raw)
  To: Jeremy Kerr, Arnd Bergmann, Benjamin Herrenschmidt,
	Paul Mackerras
  Cc: Yann Droneaud, cbe-oss-dev, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1383121137.git.ydroneaud@opteya.com>

Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().

Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.

In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.

This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.

The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.

Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Link: http://lkml.kernel.org/r/cover.1383121137.git.ydroneaud@opteya.com
---
 arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 87ba7cf..51effce 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -301,7 +301,7 @@ static int spufs_context_open(struct path *path)
 	int ret;
 	struct file *filp;
 
-	ret = get_unused_fd();
+	ret = get_unused_fd_flags(0);
 	if (ret < 0)
 		return ret;
 
@@ -518,7 +518,7 @@ static int spufs_gang_open(struct path *path)
 	int ret;
 	struct file *filp;
 
-	ret = get_unused_fd();
+	ret = get_unused_fd_flags(0);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v4 0/7] Getting rid of get_unused_fd()
From: Yann Droneaud @ 2013-10-30 19:47 UTC (permalink / raw)
  To: Tony Luck, Fenghua Yu, Al Viro, linux-ia64, Jeremy Kerr,
	Arnd Bergmann, Benjamin Herrenschmidt, Paul Mackerras,
	linuxppc-dev, cbe-oss-dev, linux-fsdevel, Eric Paris,
	Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Yann Droneaud, linux-kernel

Hi,

Please find the fourth revision of my patchset to remove get_unused_fd()
macro in order to encourage subsystems to use get_unused_fd_flags() or
anon_inode_getfd() with open flags set to O_CLOEXEC were appropriate.

The patchset convert all calls to get_unused_fd() to
get_unused_fd_flags(0) before removing get_unused_fd() macro.

Without get_unused_fd() macro, more subsystems are likely to use
anon_inode_getfd() and be teached to provide an API that let userspace
choose the opening flags of the file descriptor.

Not using O_CLOEXEC by default or not letting userspace provide the
"open" flags should be considered bad practice from security point
of view: in most case O_CLOEXEC must be used to not leak file descriptor
across exec().

Using O_CLOEXEC by default when flags are not provided by userspace
allows userspace to set, using fcntl(), without any risk of race, 
if the file descriptor is going to be inherited or not across exec().

Status:

In linux-next tag 20131029, they're currently:

- 32 calls to fd_install()
- 23 calls to get_unused_fd_flags()
- 11 calls to anon_inode_getfd()
-  7 calls to get_unused_fd()

Changes from patchset v3 [PATCHSETv3]:

- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
  DROPPED: applied upstream

Changes from patchset v2 [PATCHSETv2]:

- android/sw_sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
  DROPPED: applied upstream

- android/sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
  DROPPED: applied upstream

- vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
  DROPPED: applied upstream.
  Additionally subsystem maintainer applied another patch on top
  to set the flags to O_CLOEXEC.

- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
  NEW: propose to use O_CLOEXEC as default flag.

Changes from patchset v1 [PATCHSETv1]:

- explicitly added subsystem maintainers as mail recepients.

- infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
  DROPPED: subsystem maintainer applied another patch
           using get_unused_fd_flags(O_CLOEXEC) as suggested.

- android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
  MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested

- android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
  MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested

- xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
  DROPPED: applied asis by subsystem maintainer.

- sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
  DROPPED: applied asis by subsystem maintainer.

Links:

[PATCHSETv3]
  http://lkml.kernel.org/r/cover.1378460926.git.ydroneaud@opteya.com

[PATCHSETv2]
  http://lkml.kernel.org/r/cover.1376327678.git.ydroneaud@opteya.com

[PATCHSETv1]
  http://lkml.kernel.org/r/cover.1372777600.git.ydroneaud@opteya.com

Yann Droneaud (7):
  ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
  ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
  binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
  file: use get_unused_fd_flags(0) instead of get_unused_fd()
  fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
  events: use get_unused_fd_flags(0) instead of get_unused_fd()
  file: remove get_unused_fd()

 arch/ia64/kernel/perfmon.c                | 2 +-
 arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
 fs/binfmt_misc.c                          | 2 +-
 fs/file.c                                 | 2 +-
 fs/notify/fanotify/fanotify_user.c        | 2 +-
 include/linux/file.h                      | 1 -
 kernel/events/core.c                      | 2 +-
 7 files changed, 7 insertions(+), 8 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode
From: Geert Uytterhoeven @ 2013-10-30 19:43 UTC (permalink / raw)
  To: Tom Musta; +Cc: linuxppc-dev, tmusta, Andreas Schwab
In-Reply-To: <52715F7F.8020206@gmail.com>

On Wed, Oct 30, 2013 at 8:35 PM, Tom Musta <tommusta@gmail.com> wrote:
> On 10/30/2013 12:43 PM, Andreas Schwab wrote:
>>
>> Tom Musta <tommusta@gmail.com> writes:
>>
>>> +#ifdef __LITTLE_ENDIAN__
>>> +       if (!regs->msr & MSR_LE)
>>
>>
>> That won't work.
>>
>> Andreas.
>>
>
> Please elaborate.

You want to test for "!(regs & MSR_LE)".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode
From: Tom Musta @ 2013-10-30 19:35 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: tmusta, linuxppc-dev
In-Reply-To: <878uxairi8.fsf@igel.home>

On 10/30/2013 12:43 PM, Andreas Schwab wrote:
> Tom Musta <tommusta@gmail.com> writes:
>
>> +#ifdef __LITTLE_ENDIAN__
>> +	if (!regs->msr & MSR_LE)
>
> That won't work.
>
> Andreas.
>

Please elaborate.

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-30 19:11 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker, paulmck
In-Reply-To: <20131030182930.GP2490@laptop.programming.kicks-ass.net>

On Wed, Oct 30, 2013 at 07:29:30PM +0100, Peter Zijlstra wrote:
> +	page_shift = PAGE_SHIFT + page_order(rb);
> +
> +	handle->page = (offset >> page_shift) & (rb->nr_pages - 1);
> +
> +	offset &= page_shift - 1;

offset &= (1UL << page_shift) - 1;

Weird that it even appeared to work.. /me wonders if he even booted the
right kernel.

> +
> +	handle->addr = rb->data_pages[handle->page] + offset;
> +	handle->size = (1 << page_shift) - offset;

^ permalink raw reply

* Re: [PATCH 29/51] DMA-API: ata: pata_octeon_cf: convert to use dma_coerce_mask_and_coherent()
From: Geert Uytterhoeven @ 2013-10-30 19:08 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, linux-doc, Linux MMC List,
	Linux Fbdev development list, linux-nvme,
	linux-ide@vger.kernel.org, driverdevel, linux-samsung-soc, scsi,
	e1000-devel, Linux-Next, b43-dev, Linux Media Mailing List,
	devicetree, DRI Development, linux-tegra,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Solarflare linux maintainers, netdev@vger.kernel.org, USB list,
	linux-wireless, linux-crypto, Tejun Heo,
	uclinux-dist-devel@blackfin.uclinux.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <E1VMmAk-0007iq-IU@rmk-PC.arm.linux.org.uk>

On Thu, Sep 19, 2013 at 11:54 PM, Russell King
<rmk+kernel@arm.linux.org.uk> wrote:
> diff --git a/drivers/ata/pata_octeon_cf.c b/drivers/ata/pata_octeon_cf.c
> index c51bbb9..6231d43 100644
> --- a/drivers/ata/pata_octeon_cf.c
> +++ b/drivers/ata/pata_octeon_cf.c
> @@ -1014,8 +1014,9 @@ static int octeon_cf_probe(struct platform_device *pdev)
>         }
>         cf_port->c0 = ap->ioaddr.ctl_addr;
>
> -       pdev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
> -       pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> +       rv = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +       if (rv)
> +               return ret;

return rv;

http://kisskb.ellerman.id.au/kisskb/buildresult/9959184/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-30 18:29 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker, paulmck
In-Reply-To: <20131030155116.GO16117@laptop.programming.kicks-ass.net>

On Wed, Oct 30, 2013 at 04:51:16PM +0100, Peter Zijlstra wrote:
> On Wed, Oct 30, 2013 at 03:28:54PM +0200, Victor Kaplansky wrote:
> > one of the authors of Documentation/memory-barriers.txt is on cc: list ;-)
> > 
> > Disclaimer: it is anyway impossible to prove lack of *any* problem.
> > 
> > Having said that, lets look into an example in
> > Documentation/circular-buffers.txt:
> 
> > 
> > We can see that authors of the document didn't put any memory barrier
> 
> Note that both documents have the same author list ;-)
> 
> Anyway, I didn't know about the circular thing, I suppose I should use
> CIRC_SPACE() thing :-)

The below removes 80 bytes from ring_buffer.o of which 50 bytes are from
perf_output_begin(), it also removes 30 lines of code, so yay!

(x86_64 build)

And it appears to still work.. although I've not stressed the no-space
bits.

---
 kernel/events/ring_buffer.c | 74 ++++++++++++++-------------------------------
 1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 9c2ddfbf4525..e4a51fa10595 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -12,40 +12,10 @@
 #include <linux/perf_event.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
+#include <linux/circ_buf.h>
 
 #include "internal.h"
 
-static bool perf_output_space(struct ring_buffer *rb, unsigned long tail,
-			      unsigned long offset, unsigned long head)
-{
-	unsigned long sz = perf_data_size(rb);
-	unsigned long mask = sz - 1;
-
-	/*
-	 * check if user-writable
-	 * overwrite : over-write its own tail
-	 * !overwrite: buffer possibly drops events.
-	 */
-	if (rb->overwrite)
-		return true;
-
-	/*
-	 * verify that payload is not bigger than buffer
-	 * otherwise masking logic may fail to detect
-	 * the "not enough space" condition
-	 */
-	if ((head - offset) > sz)
-		return false;
-
-	offset = (offset - tail) & mask;
-	head   = (head   - tail) & mask;
-
-	if ((int)(head - offset) < 0)
-		return false;
-
-	return true;
-}
-
 static void perf_output_wakeup(struct perf_output_handle *handle)
 {
 	atomic_set(&handle->rb->poll, POLL_IN);
@@ -115,8 +85,7 @@ static void perf_output_put_handle(struct perf_output_handle *handle)
 	rb->user_page->data_head = head;
 
 	/*
-	 * Now check if we missed an update, rely on the (compiler)
-	 * barrier in atomic_dec_and_test() to re-read rb->head.
+	 * Now check if we missed an update.
 	 */
 	if (unlikely(head != local_read(&rb->head))) {
 		local_inc(&rb->nest);
@@ -135,7 +104,7 @@ int perf_output_begin(struct perf_output_handle *handle,
 {
 	struct ring_buffer *rb;
 	unsigned long tail, offset, head;
-	int have_lost;
+	int have_lost, page_shift;
 	struct perf_sample_data sample_data;
 	struct {
 		struct perf_event_header header;
@@ -161,7 +130,7 @@ int perf_output_begin(struct perf_output_handle *handle,
 		goto out;
 
 	have_lost = local_read(&rb->lost);
-	if (have_lost) {
+	if (unlikely(have_lost)) {
 		lost_event.header.size = sizeof(lost_event);
 		perf_event_header__init_id(&lost_event.header, &sample_data,
 					   event);
@@ -171,32 +140,33 @@ int perf_output_begin(struct perf_output_handle *handle,
 	perf_output_get_handle(handle);
 
 	do {
-		/*
-		 * Userspace could choose to issue a mb() before updating the
-		 * tail pointer. So that all reads will be completed before the
-		 * write is issued.
-		 *
-		 * See perf_output_put_handle().
-		 */
 		tail = ACCESS_ONCE(rb->user_page->data_tail);
-		smp_mb();
 		offset = head = local_read(&rb->head);
-		head += size;
-		if (unlikely(!perf_output_space(rb, tail, offset, head)))
+		if (!rb->overwrite &&
+		    unlikely(CIRC_SPACE(head, tail, perf_data_size(rb)) < size))
 			goto fail;
+		head += size;
 	} while (local_cmpxchg(&rb->head, offset, head) != offset);
 
+	/*
+	 * Userspace SHOULD issue an MB before writing the tail; see
+	 * perf_output_put_handle().
+	 */
+	smp_mb();
+
 	if (head - local_read(&rb->wakeup) > rb->watermark)
 		local_add(rb->watermark, &rb->wakeup);
 
-	handle->page = offset >> (PAGE_SHIFT + page_order(rb));
-	handle->page &= rb->nr_pages - 1;
-	handle->size = offset & ((PAGE_SIZE << page_order(rb)) - 1);
-	handle->addr = rb->data_pages[handle->page];
-	handle->addr += handle->size;
-	handle->size = (PAGE_SIZE << page_order(rb)) - handle->size;
+	page_shift = PAGE_SHIFT + page_order(rb);
+
+	handle->page = (offset >> page_shift) & (rb->nr_pages - 1);
+
+	offset &= page_shift - 1;
+
+	handle->addr = rb->data_pages[handle->page] + offset;
+	handle->size = (1 << page_shift) - offset;
 
-	if (have_lost) {
+	if (unlikely(have_lost)) {
 		lost_event.header.type = PERF_RECORD_LOST;
 		lost_event.header.misc = 0;
 		lost_event.id          = event->id;

^ permalink raw reply related

* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-30 17:44 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker,
	Paul E. McKenney
In-Reply-To: <OFCA1DB4AC.7D4F6EEE-ON42257C14.0056F77D-42257C14.005EB5EA@il.ibm.com>

On Wed, Oct 30, 2013 at 07:14:29PM +0200, Victor Kaplansky wrote:
> We need a complete rmb() here IMO. I think there is a fundamental
> difference between load and stores in this aspect. Load are allowed to
> be hoisted by compiler or executed speculatively by HW. To prevent
> load "*(ubuf->data + tail)" to be hoisted beyond "ubuf->head" load you
> would need something like this:

Indeed, we could compute and load ->data + tail the moment we've
completed the tail load but before we've completed the head load and
done the comparison.

So yes, full rmb() it is.

> void
> ubuf_read(void)
> {
>         u64 head, tail;
> 
>         tail = ubuf->tail;
>         head = ACCESS_ONCE(ubuf->head);
> 
>         /*
>          * Ensure we read the buffer boundaries before the actual buffer
>          * data...
>          */
> 
>         while (tail != head) {
> 		    smp_read_barrier_depends();         /* for Alpha */
>                 obj = *(ubuf->data + head - 128);
>                 /* process obj */
>                 tail += obj->size;
>                 tail %= ubuf->size;
>         }
> 
>         /*
>          * Ensure all data reads are complete before we issue the
>          * ubuf->tail update; once that update hits, kbuf_write() can
>          * observe and overwrite data.
>          */
>         smp_mb();               /* D, matches with A */
> 
>         ubuf->tail = tail;
> }
> 
> (note that "head" is part of address calculation of obj load now).

Right, explicit dependent loads; I was hoping the conditional in between
might be enough, but as argued above it is not. The above cannot work in
our case though, we must use tail to find the obj since we have variable
size objects.

> But, even in this demo example some "smp_read_barrier_depends()" before
> "obj = *(ubuf->data + head - 100);" is required for architectures
> like Alpha. Though, on more sane architectures "smp_read_barrier_depends()"
> will be translated to nothing.

Sure.. I know all about that.

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode
From: Andreas Schwab @ 2013-10-30 17:43 UTC (permalink / raw)
  To: Tom Musta; +Cc: tmusta, linuxppc-dev
In-Reply-To: <1382125235.2206.24.camel@tmusta-sc.rchland.ibm.com>

Tom Musta <tommusta@gmail.com> writes:

> +#ifdef __LITTLE_ENDIAN__
> +	if (!regs->msr & MSR_LE)

That won't work.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Victor Kaplansky @ 2013-10-30 17:14 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker,
	Paul E. McKenney
In-Reply-To: <20131030153931.GM16117@laptop.programming.kicks-ass.net>

Peter Zijlstra <peterz@infradead.org> wrote on 10/30/2013 05:39:31 PM:

> Although I suppose speculative reads are allowed -- they don't have the
> destructive behaviour speculative writes have -- and thus we could in
> fact get reorder issues.

I agree.

>
> But since it is still a dependent load in that we do that @tail vs @head
> comparison before doing other loads, wouldn't a read_barrier_depends()
> be sufficient? Or do we still need a complete rmb?

We need a complete rmb() here IMO. I think there is a fundamental
difference
between load and stores in this aspect. Load are allowed to be hoisted by
compiler or executed speculatively by HW. To prevent load "*(ubuf->data +
tail)"
to be hoisted beyond "ubuf->head" load you would need something like this:

void
ubuf_read(void)
{
        u64 head, tail;

        tail = ubuf->tail;
        head = ACCESS_ONCE(ubuf->head);

        /*
         * Ensure we read the buffer boundaries before the actual buffer
         * data...
         */

        while (tail != head) {
		    smp_read_barrier_depends();         /* for Alpha */
                obj = *(ubuf->data + head - 128);
                /* process obj */
                tail += obj->size;
                tail %= ubuf->size;
        }

        /*
         * Ensure all data reads are complete before we issue the
         * ubuf->tail update; once that update hits, kbuf_write() can
         * observe and overwrite data.
         */
        smp_mb();               /* D, matches with A */

        ubuf->tail = tail;
}

(note that "head" is part of address calculation of obj load now).

But, even in this demo example some "smp_read_barrier_depends()" before
"obj = *(ubuf->data + head - 100);" is required for architectures
like Alpha. Though, on more sane architectures "smp_read_barrier_depends()"
will be translated to nothing.


Regards,
-- Victor

^ permalink raw reply

* [PATCH] offb: make the screen properties endian safe
From: Cédric Le Goater @ 2013-10-30 16:14 UTC (permalink / raw)
  To: benh; +Cc: Cédric Le Goater, linuxppc-dev

The "screen" properties : depth, width, height, linebytes need
to be converted to the host endian order when read from the device
tree.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 drivers/video/offb.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index 0c4f343..c1cf591 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -536,7 +536,7 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
 	unsigned int flags, rsize, addr_prop = 0;
 	unsigned long max_size = 0;
 	u64 rstart, address = OF_BAD_ADDR;
-	const u32 *pp, *addrp, *up;
+	const __be32 *pp, *addrp, *up;
 	u64 asize;
 	int foreign_endian = 0;
 
@@ -552,25 +552,25 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
 	if (pp == NULL)
 		pp = of_get_property(dp, "depth", &len);
 	if (pp && len == sizeof(u32))
-		depth = *pp;
+		depth = be32_to_cpu(*pp);
 
 	pp = of_get_property(dp, "linux,bootx-width", &len);
 	if (pp == NULL)
 		pp = of_get_property(dp, "width", &len);
 	if (pp && len == sizeof(u32))
-		width = *pp;
+		width = be32_to_cpu(*pp);
 
 	pp = of_get_property(dp, "linux,bootx-height", &len);
 	if (pp == NULL)
 		pp = of_get_property(dp, "height", &len);
 	if (pp && len == sizeof(u32))
-		height = *pp;
+		height = be32_to_cpu(*pp);
 
 	pp = of_get_property(dp, "linux,bootx-linebytes", &len);
 	if (pp == NULL)
 		pp = of_get_property(dp, "linebytes", &len);
 	if (pp && len == sizeof(u32) && (*pp != 0xffffffffu))
-		pitch = *pp;
+		pitch = be32_to_cpu(*pp);
 	else
 		pitch = width * ((depth + 7) / 8);
 
-- 
1.7.10.4

^ permalink raw reply related

* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-30 15:51 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker, paulmck
In-Reply-To: <OFC6B7100E.31C6B190-ON42257C14.004841C6-42257C14.004A0EA6@il.ibm.com>

On Wed, Oct 30, 2013 at 03:28:54PM +0200, Victor Kaplansky wrote:
> one of the authors of Documentation/memory-barriers.txt is on cc: list ;-)
> 
> Disclaimer: it is anyway impossible to prove lack of *any* problem.
> 
> Having said that, lets look into an example in
> Documentation/circular-buffers.txt:

> 
> We can see that authors of the document didn't put any memory barrier

Note that both documents have the same author list ;-)

Anyway, I didn't know about the circular thing, I suppose I should use
CIRC_SPACE() thing :-)

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-30 15:39 UTC (permalink / raw)
  To: Victor Kaplansky
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker,
	Paul E. McKenney
In-Reply-To: <OFF6C78E48.E477DB1E-ON42257C14.0050F9B3-42257C14.0051AC6D@il.ibm.com>

On Wed, Oct 30, 2013 at 04:52:05PM +0200, Victor Kaplansky wrote:
> Peter Zijlstra <peterz@infradead.org> wrote on 10/30/2013 01:25:26 PM:
> 
> > Also, I'm not entirely sure on C, that too seems like a dependency, we
> > simply cannot read the buffer @tail before we've read the tail itself,
> > now can we? Similarly we cannot compare tail to head without having the
> > head read completed.
> 
> No, this one we cannot omit, because our problem on consumer side is not
> with @tail, which is written exclusively by consumer, but with @head.

Ah indeed, my argument was flawed in that @head is the important part.
But we still do a comparison of @tail against @head before we do further
reads.

Although I suppose speculative reads are allowed -- they don't have the
destructive behaviour speculative writes have -- and thus we could in
fact get reorder issues.

But since it is still a dependent load in that we do that @tail vs @head
comparison before doing other loads, wouldn't a read_barrier_depends()
be sufficient? Or do we still need a complete rmb?

> BTW, it is why you also don't need ACCESS_ONCE() around @tail, but only
> around
> @head read.

Agreed, the ACCESS_ONCE() around tail is superfluous since we're the one
updating tail, so there's no problem with the value changing
unexpectedly.

^ permalink raw reply

* Re: perf events ring buffer memory barrier on powerpc
From: Victor Kaplansky @ 2013-10-30 14:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Neuling, Mathieu Desnoyers, LKML, Oleg Nesterov,
	Linux PPC dev, Anton Blanchard, Frederic Weisbecker,
	Paul E. McKenney
In-Reply-To: <20131030112526.GI16117@laptop.programming.kicks-ass.net>

Peter Zijlstra <peterz@infradead.org> wrote on 10/30/2013 01:25:26 PM:

> Also, I'm not entirely sure on C, that too seems like a dependency, we
> simply cannot read the buffer @tail before we've read the tail itself,
> now can we? Similarly we cannot compare tail to head without having the
> head read completed.

No, this one we cannot omit, because our problem on consumer side is not
with @tail, which is written exclusively by consumer, but with @head.

BTW, it is why you also don't need ACCESS_ONCE() around @tail, but only
around
@head read.

-- Victor

^ permalink raw reply

* [RFC PATCH v5 12/12] powerpc/powernv: Machine check exception handling.
From: Mahesh J Salgaonkar @ 2013-10-30 14:36 UTC (permalink / raw)
  To: linuxppc-dev, Paul Mackerras, Benjamin Herrenschmidt
  Cc: Jeremy Kerr, Anton Blanchard
In-Reply-To: <20131030143219.26643.24782.stgit@mars>

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

Add basic error handling in machine check exception handler.

- If MSR_RI isn't set, we can not recover.
- Check if disposition set to OpalMCE_DISPOSITION_RECOVERED.
- Check if address at fault is inside kernel address space, if not then send
  SIGBUS to process if we hit exception when in userspace.
- If address at fault is not provided then and if we get a synchronous machine
  check while in userspace then kill the task.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/mce.h        |    1 +
 arch/powerpc/kernel/mce.c             |   27 +++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c |   43 ++++++++++++++++++++++++++++++++-
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 3276b40..a2b8c7b 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -193,5 +193,6 @@ extern void release_mce_event(void);
 extern void machine_check_queue_event(void);
 extern void machine_check_process_queued_event(void);
 extern void machine_check_print_event_info(struct machine_check_event *evt);
+extern uint64_t get_mce_fault_addr(struct machine_check_event *evt);
 
 #endif /* __ASM_PPC64_MCE_H__ */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 1c6d157..c0c52ec 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -316,3 +316,30 @@ void machine_check_print_event_info(struct machine_check_event *evt)
 		break;
 	}
 }
+
+uint64_t get_mce_fault_addr(struct machine_check_event *evt)
+{
+	switch (evt->error_type) {
+	case MCE_ERROR_TYPE_UE:
+		if (evt->u.ue_error.effective_address_provided)
+			return evt->u.ue_error.effective_address;
+		break;
+	case MCE_ERROR_TYPE_SLB:
+		if (evt->u.slb_error.effective_address_provided)
+			return evt->u.slb_error.effective_address;
+		break;
+	case MCE_ERROR_TYPE_ERAT:
+		if (evt->u.erat_error.effective_address_provided)
+			return evt->u.erat_error.effective_address;
+		break;
+	case MCE_ERROR_TYPE_TLB:
+		if (evt->u.tlb_error.effective_address_provided)
+			return evt->u.tlb_error.effective_address;
+		break;
+	default:
+	case MCE_ERROR_TYPE_UNKNOWN:
+		break;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(get_mce_fault_addr);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index d17e1e3..4a727d3 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/notifier.h>
 #include <linux/slab.h>
+#include <linux/sched.h>
 #include <asm/opal.h>
 #include <asm/firmware.h>
 #include <asm/mce.h>
@@ -240,6 +241,44 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	return written;
 }
 
+static int opal_recover_mce(struct pt_regs *regs,
+					struct machine_check_event *evt)
+{
+	int recovered = 0;
+	uint64_t ea = get_mce_fault_addr(evt);
+
+	if (!(regs->msr & MSR_RI)) {
+		/* If MSR_RI isn't set, we cannot recover */
+		recovered = 0;
+	} else if (evt->disposition == MCE_DISPOSITION_RECOVERED) {
+		/* Platform corrected itself */
+		recovered = 1;
+	} else if (ea && !is_kernel_addr(ea)) {
+		/*
+		 * Faulting address is not in kernel text. We should be fine.
+		 * We need to find which process uses this address.
+		 * For now, kill the task if we have received exception when
+		 * in userspace.
+		 *
+		 * TODO: Queue up this address for hwpoisioning later.
+		 */
+		if (user_mode(regs) && !is_global_init(current)) {
+			_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+			recovered = 1;
+		} else
+			recovered = 0;
+	} else if (user_mode(regs) && !is_global_init(current) &&
+		evt->severity == MCE_SEV_ERROR_SYNC) {
+		/*
+		 * If we have received a synchronous error when in userspace
+		 * kill the task.
+		 */
+		_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+		recovered = 1;
+	}
+	return recovered;
+}
+
 int opal_machine_check(struct pt_regs *regs)
 {
 	struct machine_check_event evt;
@@ -255,7 +294,9 @@ int opal_machine_check(struct pt_regs *regs)
 	}
 	machine_check_print_event_info(&evt);
 
-	return evt.severity == MCE_SEV_FATAL ? 0 : 1;
+	if (opal_recover_mce(regs, &evt))
+		return 1;
+	return 0;
 }
 
 static irqreturn_t opal_interrupt(int irq, void *data)

^ permalink raw reply related

* [RFC PATCH v5 11/12] powerpc/powernv: Remove machine check handling in OPAL.
From: Mahesh J Salgaonkar @ 2013-10-30 14:35 UTC (permalink / raw)
  To: linuxppc-dev, Paul Mackerras, Benjamin Herrenschmidt
  Cc: Jeremy Kerr, Anton Blanchard
In-Reply-To: <20131030143219.26643.24782.stgit@mars>

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

Now that we are ready to handle machine check directly in linux, do not
register with firmware to handle machine check exception.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 219ce65..d17e1e3 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -83,14 +83,10 @@ static int __init opal_register_exception_handlers(void)
 	if (!(powerpc_firmware_features & FW_FEATURE_OPAL))
 		return -ENODEV;
 
-	/* Hookup some exception handlers. We use the fwnmi area at 0x7000
-	 * to provide the glue space to OPAL
+	/* Hookup some exception handlers except machine check. We use the
+	 * fwnmi area at 0x7000 to provide the glue space to OPAL
 	 */
 	glue = 0x7000;
-	opal_register_exception_handler(OPAL_MACHINE_CHECK_HANDLER,
-					__pa(opal_mc_secondary_handler[0]),
-					glue);
-	glue += 128;
 	opal_register_exception_handler(OPAL_HYPERVISOR_MAINTENANCE_HANDLER,
 					0, glue);
 	glue += 128;

^ permalink raw reply related


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