* Re: TTM placement & caching issue/questions
From: Thomas Hellstrom @ 2014-09-04 8:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jerome Glisse, linuxppc-dev, Michel Danzer, dri-devel
In-Reply-To: <1409817962.4246.51.camel@pasglop>
On 09/04/2014 10:06 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-09-04 at 09:44 +0200, Thomas Hellstrom wrote:
>
>>> This will, from what I can tell, try to use the same caching mode as the
>>> original object:
>>>
>>> if ((cur_placement & caching) != 0)
>>> result |= (cur_placement & caching);
>>>
>>> And cur_placement comes from bo->mem.placement which as far as I can
>>> tell is based on the placement array which the drivers set up.
>> This originates from the fact that when evicting GTT memory, on x86 it's
>> unnecessary and undesirable to switch caching mode when going to system.
> But that's what I don't quite understand. We have two different mappings
> here. The VRAM and the memory object. We wouldn't be "switching"... we
> are creating a temporary mapping for the memory object in order to do
> the memcpy, but we seem to be doing it by using the caching attributes
> of the VRAM object.... or am I missing something ? I don't see how that
> makes sense so I suppose I'm missing something here :-)
Well, the intention when TTM was written was that the driver writer
should be smart enough that when he wanted a move from unached VRAM to
system, he'd request cached system in the placement flags in the first
place. If TTM somehow overrides such a request, that's a bug in TTM.
If the move, for example, is a result of an eviction, then the driver
evict_flags() function should ideally look at the current placement and
decide about a suitable placement based on that: vram-to-system moves
should generally request cacheable memory if the next access is expected
by the CPU. Probably write-combined otherwise.
If the move is the result of a TTM swapout, TTM will automatically
select cachable system, and for most other moves, I think the driver
writer is in full control.
>
>> Last time I tested, (and it seems like Michel is on the same track),
>> writing with the CPU to write-combined memory was substantially faster
>> than writing to cached memory, with the additional side-effect that CPU
>> caches are left unpolluted.
> That's very strange indeed. It's certainly an x86 specific artifact,
> even if we were allowed by our hypervisor to map memory non-cachable
> (the HW somewhat can), we tend to have a higher throughput by going
> cachable, but that could be due to the way the PowerBus works (it's
> basically very biased toward cachable transactions).
>
>> I dislike the approach of rewriting placements. In some cases I think it
>> won't even work, because placements are declared 'static const'
>>
>> What I'd suggest is instead to intercept the driver response from
>> init_mem_type() and filter out undesired caching modes from
>> available_caching and default_caching,
> This was my original intent but Jerome seems to have different ideas
> (see his proposed patches). I'm happy to revive mine as well and post it
> as an alternative after I've tested it a bit more (tomorrow).
>
>> perhaps also looking at whether
>> the memory type is mappable or not. This should have the additional
>> benefit of working everywhere, and if a caching mode is selected that's
>> not available on the platform, you'll simply get an error. (I guess?)
> You mean that if not mappable we don't bother filtering ?
>
> The rule is really for me pretty simple:
>
> - If it's system memory (PL_SYSTEM/PL_TT), it MUST be cachable
>
> - If it's PCIe memory space (VRAM, registers, ...) it MUST be
> non-cachable.
Yes, something along these lines. I guess checking for VRAM or
TTM_MEMTYPE_FLAG_FIXED would perhaps do the trick
/Thomas
>
> Cheers,
> Ben.
>
>> /Thomas
>>
>>
>>> Cheers,
>>> Ben.
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/dri-devel&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=C9AHL1VngKBOxe2UrNP2eCZo6FLqdlr6Y90rpfE5rUs%3D%0A&s=73da0633bafc5d54bf116bc861d48d13c39cf8f41832adfb739709e98ec05553
>
^ permalink raw reply
* Re: bit fields && data tearing
From: Mikael Pettersson @ 2014-09-04 8:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jakub Jelinek, linux-arch, Tony Luck, linux-ia64, Peter Hurley,
Oleg Nesterov, linux-kernel, Paul Mackerras, Paul E. McKenney,
linuxppc-dev, Miroslav Franc, Richard Henderson
In-Reply-To: <1409785893.30640.118.camel@pasglop>
Benjamin Herrenschmidt writes:
> On Wed, 2014-09-03 at 18:51 -0400, Peter Hurley wrote:
>
> > Apologies for hijacking this thread but I need to extend this discussion
> > somewhat regarding what a compiler might do with adjacent fields in a structure.
> >
> > The tty subsystem defines a large aggregate structure, struct tty_struct.
> > Importantly, several different locks apply to different fields within that
> > structure; ie., a specific spinlock will be claimed before updating or accessing
> > certain fields while a different spinlock will be claimed before updating or
> > accessing certain _adjacent_ fields.
> >
> > What is necessary and sufficient to prevent accidental false-sharing?
> > The patch below was flagged as insufficient on ia64, and possibly ARM.
>
> We expect native aligned scalar types to be accessed atomically (the
> read/modify/write of a larger quantity that gcc does on some bitfield
> cases has been flagged as a gcc bug, but shouldn't happen on normal
> scalar types).
>
> I am not 100% certain of "bool" here, I assume it's treated as a normal
> scalar and thus atomic but if unsure, you can always use int.
Please use an aligned int or long. Some machines cannot do atomic
accesses on sub-int/long quantities, so 'bool' may cause unexpected
rmw cycles on adjacent fields.
/Mikael
>
> Another option is to use the atomic bitops and make these bits in a
> bitmask but that is probably unnecessary if you have locks already.
>
> Cheers,
> Ben.
>
>
> > Regards,
> > Peter Hurley
> >
> > --- >% ---
> > Subject: [PATCH 21/26] tty: Convert tty_struct bitfield to bools
> >
> > The stopped, hw_stopped, flow_stopped and packet bits are smp-unsafe
> > and interrupt-unsafe. For example,
> >
> > CPU 0 | CPU 1
> > |
> > tty->flow_stopped = 1 | tty->hw_stopped = 0
> >
> > One of these updates will be corrupted, as the bitwise operation
> > on the bitfield is non-atomic.
> >
> > Ensure each flag has a separate memory location, so concurrent
> > updates do not corrupt orthogonal states.
> >
> > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> > ---
> > include/linux/tty.h | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > index 1c3316a..7cf61cb 100644
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -261,7 +261,10 @@ struct tty_struct {
> > unsigned long flags;
> > int count;
> > struct winsize winsize; /* winsize_mutex */
> > - unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
> > + bool stopped;
> > + bool hw_stopped;
> > + bool flow_stopped;
> > + bool packet;
> > unsigned char ctrl_status; /* ctrl_lock */
> > unsigned int receive_room; /* Bytes free for queue */
> > int flow_change;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
^ permalink raw reply
* Re: bit fields && data tearing
From: Jakub Jelinek @ 2014-09-04 9:09 UTC (permalink / raw)
To: Mikael Pettersson
Cc: linux-arch, Tony Luck, linux-ia64, Peter Hurley, Oleg Nesterov,
linux-kernel, Paul Mackerras, Paul E. McKenney, linuxppc-dev,
Miroslav Franc, Richard Henderson
In-Reply-To: <21512.10628.412205.873477@gargle.gargle.HOWL>
On Thu, Sep 04, 2014 at 10:57:40AM +0200, Mikael Pettersson wrote:
> Benjamin Herrenschmidt writes:
> > On Wed, 2014-09-03 at 18:51 -0400, Peter Hurley wrote:
> >
> > > Apologies for hijacking this thread but I need to extend this discussion
> > > somewhat regarding what a compiler might do with adjacent fields in a structure.
> > >
> > > The tty subsystem defines a large aggregate structure, struct tty_struct.
> > > Importantly, several different locks apply to different fields within that
> > > structure; ie., a specific spinlock will be claimed before updating or accessing
> > > certain fields while a different spinlock will be claimed before updating or
> > > accessing certain _adjacent_ fields.
> > >
> > > What is necessary and sufficient to prevent accidental false-sharing?
> > > The patch below was flagged as insufficient on ia64, and possibly ARM.
> >
> > We expect native aligned scalar types to be accessed atomically (the
> > read/modify/write of a larger quantity that gcc does on some bitfield
> > cases has been flagged as a gcc bug, but shouldn't happen on normal
> > scalar types).
> >
> > I am not 100% certain of "bool" here, I assume it's treated as a normal
> > scalar and thus atomic but if unsure, you can always use int.
>
> Please use an aligned int or long. Some machines cannot do atomic
> accesses on sub-int/long quantities, so 'bool' may cause unexpected
> rmw cycles on adjacent fields.
Yeah, at least pre-EV56 Alpha performs rmw cycles on char/short accesses
and thus those are not atomic.
Jakub
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Daniel Vetter @ 2014-09-04 9:34 UTC (permalink / raw)
To: Thomas Hellstrom; +Cc: dri-devel, Michel Danzer, linuxppc-dev
In-Reply-To: <54081844.7000604@vmware.com>
On Thu, Sep 04, 2014 at 09:44:04AM +0200, Thomas Hellstrom wrote:
> Last time I tested, (and it seems like Michel is on the same track),
> writing with the CPU to write-combined memory was substantially faster
> than writing to cached memory, with the additional side-effect that CPU
> caches are left unpolluted.
>
> Moreover (although only tested on Intel's embedded chipsets), texturing
> from cpu-cache-coherent PCI memory was a real GPU performance hog
> compared to texturing from non-snooped memory. Hence, whenever a buffer
> could be classified as GPU-read-only (or almost at least), it should be
> placed in write-combined memory.
Just a quick comment since this explicitly referes to intel chips: On
desktop/laptop chips with the big shared l3/l4 caches it's the other way
round. Cached uploads are substantially faster than wc and not using
coherent access is a severe perf hit for texturing. I guess the hw guys
worked really hard to hide the snooping costs so that the gpu can benefit
from the massive bandwidth these caches can provide.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 9:43 UTC (permalink / raw)
To: Daniel Vetter; +Cc: linuxppc-dev, Thomas Hellstrom, dri-devel, Michel Danzer
In-Reply-To: <20140904093454.GG15520@phenom.ffwll.local>
On Thu, 2014-09-04 at 11:34 +0200, Daniel Vetter wrote:
> On Thu, Sep 04, 2014 at 09:44:04AM +0200, Thomas Hellstrom wrote:
> > Last time I tested, (and it seems like Michel is on the same track),
> > writing with the CPU to write-combined memory was substantially faster
> > than writing to cached memory, with the additional side-effect that CPU
> > caches are left unpolluted.
> >
> > Moreover (although only tested on Intel's embedded chipsets), texturing
> > from cpu-cache-coherent PCI memory was a real GPU performance hog
> > compared to texturing from non-snooped memory. Hence, whenever a buffer
> > could be classified as GPU-read-only (or almost at least), it should be
> > placed in write-combined memory.
>
> Just a quick comment since this explicitly referes to intel chips: On
> desktop/laptop chips with the big shared l3/l4 caches it's the other way
> round. Cached uploads are substantially faster than wc and not using
> coherent access is a severe perf hit for texturing. I guess the hw guys
> worked really hard to hide the snooping costs so that the gpu can benefit
> from the massive bandwidth these caches can provide.
This is similar to modern POWER chips as well. We have pretty big L3's
(though not technically shared they are in a separate quadrant and we
have a shared L4 in the memory buffer) and our fabric is generally
optimized for cachable/coherent access performance. In fact, we only
have so many credits for NC accesses on the bus...
What that tells me is that when setting up the desired cachability
attributes for the mapping of a memory object, we need to consider these
things here:
- The hard requirement of the HW (non-coherent GPUs require NC, AGP
does in some cases, etc...) which I think is basically already handled
using the placement attributes set by the GPU driver for the memory type
- The optimal attributes (and platform hard requirements) for fast
memory accesses to an object by the processor. From what I read here,
this can be NC+WC on older Intel, cachable on newer, etc...)
- The optimal attributes for fast GPU DMA accesses to the object in
system memory. Here too, this is fairly platform/chipset dependent.
Do we have flags in the DRM that tell us whether an object in memory is
more likely to be used by the GPU via DMA vs by the CPU via MMIO ? On
powerpc (except in the old AGP case), I wouldn't care about require
cachable in both case, but I can see the low latency crowd wanting the
former to be non-cachable while the dumb GPUs like AST who don't do DMA
would benefit greatly from the latter...
Cheers,
Ben.
^ permalink raw reply
* Re: bit fields && data tearing
From: Benjamin Herrenschmidt @ 2014-09-04 9:52 UTC (permalink / raw)
To: David Laight
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Peter Hurley, Oleg Nesterov,
linux-kernel@vger.kernel.org, Paul Mackerras, Paul E. McKenney,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17487172@AcuExch.aculab.com>
On Thu, 2014-09-04 at 08:43 +0000, David Laight wrote:
> From: Benjamin Herrenschmidt
> > On Wed, 2014-09-03 at 18:51 -0400, Peter Hurley wrote:
> >
> > > Apologies for hijacking this thread but I need to extend this discussion
> > > somewhat regarding what a compiler might do with adjacent fields in a structure.
> > >
> > > The tty subsystem defines a large aggregate structure, struct tty_struct.
> > > Importantly, several different locks apply to different fields within that
> > > structure; ie., a specific spinlock will be claimed before updating or accessing
> > > certain fields while a different spinlock will be claimed before updating or
> > > accessing certain _adjacent_ fields.
> > >
> > > What is necessary and sufficient to prevent accidental false-sharing?
> > > The patch below was flagged as insufficient on ia64, and possibly ARM.
> >
> > We expect native aligned scalar types to be accessed atomically (the
> > read/modify/write of a larger quantity that gcc does on some bitfield
> > cases has been flagged as a gcc bug, but shouldn't happen on normal
> > scalar types).
>
> That isn't true on all architectures for items smaller than a machine word.
> At least one has to do rmw for byte accesses.
Yeah correct, alpha and bytes right ? Is there any other ? That's why I
suggested int.
> David
>
> > I am not 100% certain of "bool" here, I assume it's treated as a normal
> > scalar and thus atomic but if unsure, you can always use int.
> >
> > Another option is to use the atomic bitops and make these bits in a
> > bitmask but that is probably unnecessary if you have locks already.
> >
> > Cheers,
> > Ben.
^ permalink raw reply
* Re: [PATCH] iommu/fsl: Fix warning resulting from adding PCI device twice
From: Joerg Roedel @ 2014-09-04 10:14 UTC (permalink / raw)
To: Varun Sethi
Cc: linuxppc-dev, iommu, alex.williamson, Emilian.Medve, linux-kernel
In-Reply-To: <1409810622-742-1-git-send-email-Varun.Sethi@freescale.com>
On Thu, Sep 04, 2014 at 11:33:42AM +0530, Varun Sethi wrote:
> + if (!iommu_group_get(dev))
> + ret = iommu_group_add_device(group, dev);
>
> iommu_group_put(group);
> return ret;
Doesn't this additional call to iommu_group_get take a reference to the
iommu_group that needs to be released?
Joerg
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Thomas Hellstrom @ 2014-09-04 10:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, dri-devel, Daniel Vetter
In-Reply-To: <1409823823.4246.61.camel@pasglop>
On 09/04/2014 11:43 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-09-04 at 11:34 +0200, Daniel Vetter wrote:
>> On Thu, Sep 04, 2014 at 09:44:04AM +0200, Thomas Hellstrom wrote:
>>> Last time I tested, (and it seems like Michel is on the same track),
>>> writing with the CPU to write-combined memory was substantially faster
>>> than writing to cached memory, with the additional side-effect that CPU
>>> caches are left unpolluted.
>>>
>>> Moreover (although only tested on Intel's embedded chipsets), texturing
>>> from cpu-cache-coherent PCI memory was a real GPU performance hog
>>> compared to texturing from non-snooped memory. Hence, whenever a buffer
>>> could be classified as GPU-read-only (or almost at least), it should be
>>> placed in write-combined memory.
>> Just a quick comment since this explicitly referes to intel chips: On
>> desktop/laptop chips with the big shared l3/l4 caches it's the other way
>> round. Cached uploads are substantially faster than wc and not using
>> coherent access is a severe perf hit for texturing. I guess the hw guys
>> worked really hard to hide the snooping costs so that the gpu can benefit
>> from the massive bandwidth these caches can provide.
> This is similar to modern POWER chips as well. We have pretty big L3's
> (though not technically shared they are in a separate quadrant and we
> have a shared L4 in the memory buffer) and our fabric is generally
> optimized for cachable/coherent access performance. In fact, we only
> have so many credits for NC accesses on the bus...
>
Thanks both of you for the update. I haven't dealt with real hardware
for a while..
/Thomas
^ permalink raw reply
* RE: [PATCH] iommu/fsl: Fix warning resulting from adding PCI device twice
From: Varun Sethi @ 2014-09-04 10:57 UTC (permalink / raw)
To: Joerg Roedel
Cc: linuxppc-dev@ozlabs.org, iommu@lists.linux-foundation.org,
alex.williamson@redhat.com, Emilian Medve,
linux-kernel@vger.kernel.org
In-Reply-To: <20140904101402.GM28786@8bytes.org>
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Thursday, September 04, 2014 3:44 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; alex.williamson@redhat.com; Medve
> Emilian-EMMEDVE1; linuxppc-dev@ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] iommu/fsl: Fix warning resulting from adding PCI dev=
ice
> twice
>=20
> On Thu, Sep 04, 2014 at 11:33:42AM +0530, Varun Sethi wrote:
> > + if (!iommu_group_get(dev))
> > + ret =3D iommu_group_add_device(group, dev);
> >
> > iommu_group_put(group);
> > return ret;
>=20
> Doesn't this additional call to iommu_group_get take a reference to the
> iommu_group that needs to be released?
Yes, you are right this would take a reference to the group that should be =
released. I need to add iommu_group_put for decrementing the reference coun=
t. Will post a new patch.
Thanks,
Varun
^ permalink raw reply
* [PATCH v2] iommu/fsl: Fix warning resulting from adding PCI device twice
From: Varun Sethi @ 2014-09-04 11:38 UTC (permalink / raw)
To: joro, iommu, alex.williamson, Emilian.Medve, linuxppc-dev,
linux-kernel
Cc: Varun Sethi
iommu_group_get_for_dev determines the iommu group for the PCI device and adds
the device to the group.
In the PAMU driver we were again adding the device to the same group without checking
if the device already had an iommu group. This resulted in the following warning.
sysfs: cannot create duplicate filename '/devices/ffe200000.pcie/pci0000:00/0000:00:00.0/iommu_group'
------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:31
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc3-00002-g7505cea-dirty #126
task: c0000001fe0a0000 ti: c0000001fe044000 task.ti: c0000001fe044000
NIP: c00000000018879c LR: c000000000188798 CTR: c00000000001ea50
REGS: c0000001fe047040 TRAP: 0700 Not tainted (3.17.0-rc3-00002-g7505cea-dirty)
MSR: 0000000080029000 <CE,EE,ME> CR: 24ad8e22 XER: 20000000
SOFTE: 1
GPR00: c000000000188798 c0000001fe0472c0 c0000000009a52e0 0000000000000065
GPR04: 0000000000000001 0000000000000000 3a30303a00000000 0000000027000000
GPR08: 2f696f6d00000000 c0000000008d3830 c0000000009b3938 c0000000009bb3d0
GPR12: 0000000028ad8e24 c00000000fff4000 c00000000000205c 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000008a4c70
GPR24: c0000000007e9010 c0000001fe0140a8 ffffffffffffffef 0000000000000001
GPR28: c0000001fe22ebb8 c0000000007e9010 c00000000090bf10 c0000001fe220000
NIP [c00000000018879c] .sysfs_warn_dup+0x74/0xa4
LR [c000000000188798] .sysfs_warn_dup+0x70/0xa4
Call Trace:
[c0000001fe0472c0] [c000000000188798] .sysfs_warn_dup+0x70/0xa4 (unreliable)
[c0000001fe047350] [c000000000188d34] .sysfs_do_create_link_sd.clone.2+0x168/0x174
[c0000001fe047400] [c0000000004b3cf8] .iommu_group_add_device+0x78/0x244
[c0000001fe0474b0] [c0000000004b6964] .fsl_pamu_add_device+0x88/0x1a8
[c0000001fe047570] [c0000000004b3960] .iommu_bus_notifier+0xdc/0x15c
[c0000001fe047600] [c000000000059848] .notifier_call_chain+0x8c/0xe8
[c0000001fe0476a0] [c000000000059d04] .__blocking_notifier_call_chain+0x58/0x84
[c0000001fe047750] [c00000000036619c] .device_add+0x464/0x5c8
[c0000001fe047820] [c000000000300ebc] .pci_device_add+0x14c/0x17c
[c0000001fe0478c0] [c000000000300fbc] .pci_scan_single_device+0xd0/0xf4
[c0000001fe047970] [c00000000030104c] .pci_scan_slot+0x6c/0x18c
[c0000001fe047a10] [c00000000030226c] .pci_scan_child_bus+0x40/0x114
[c0000001fe047ac0] [c000000000021974] .pcibios_scan_phb+0x240/0x2c8
[c0000001fe047b70] [c00000000085a970] .pcibios_init+0x64/0xc8
[c0000001fe047c00] [c000000000001884] .do_one_initcall+0xbc/0x224
[c0000001fe047d00] [c000000000852d50] .kernel_init_freeable+0x14c/0x21c
[c0000001fe047db0] [c000000000002078] .kernel_init+0x1c/0xfa4
[c0000001fe047e30] [c000000000000884] .ret_from_kernel_thread+0x58/0xd4
Instruction dump:
7c7f1b79 4182001c 7fe4fb78 7f83e378 38a01000 4bffc905 60000000 7c641b78
e87e8008 7fa5eb78 48482ff5 60000000 <0fe00000> 7fe3fb78 4bf7bd39 60000000
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
v2 changes
- directly check for the device iommu_group
drivers/iommu/fsl_pamu_domain.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 61d1daf..56feed7 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -984,7 +984,7 @@ static int fsl_pamu_add_device(struct device *dev)
struct iommu_group *group = ERR_PTR(-ENODEV);
struct pci_dev *pdev;
const u32 *prop;
- int ret, len;
+ int ret = 0, len;
/*
* For platform devices we allocate a separate group for
@@ -1007,7 +1007,13 @@ static int fsl_pamu_add_device(struct device *dev)
if (IS_ERR(group))
return PTR_ERR(group);
- ret = iommu_group_add_device(group, dev);
+ /*
+ * Check if device has already been added to an iommu group.
+ * Group could have already been created for a PCI device in
+ * the iommu_group_get_for_dev path.
+ */
+ if (!dev->iommu_group)
+ ret = iommu_group_add_device(group, dev);
iommu_group_put(group);
return ret;
--
1.7.9.5
^ permalink raw reply related
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-04 12:24 UTC (permalink / raw)
To: Jakub Jelinek, Mikael Pettersson
Cc: linux-arch, Tony Luck, linux-ia64, Oleg Nesterov, linux-kernel,
Paul Mackerras, Paul E. McKenney, linuxppc-dev, Miroslav Franc,
Richard Henderson
In-Reply-To: <20140904090952.GW17454@tucnak.redhat.com>
On 09/04/2014 05:09 AM, Jakub Jelinek wrote:
> On Thu, Sep 04, 2014 at 10:57:40AM +0200, Mikael Pettersson wrote:
>> Benjamin Herrenschmidt writes:
>> > On Wed, 2014-09-03 at 18:51 -0400, Peter Hurley wrote:
>> >
>> > > Apologies for hijacking this thread but I need to extend this discussion
>> > > somewhat regarding what a compiler might do with adjacent fields in a structure.
>> > >
>> > > The tty subsystem defines a large aggregate structure, struct tty_struct.
>> > > Importantly, several different locks apply to different fields within that
>> > > structure; ie., a specific spinlock will be claimed before updating or accessing
>> > > certain fields while a different spinlock will be claimed before updating or
>> > > accessing certain _adjacent_ fields.
>> > >
>> > > What is necessary and sufficient to prevent accidental false-sharing?
>> > > The patch below was flagged as insufficient on ia64, and possibly ARM.
>> >
>> > We expect native aligned scalar types to be accessed atomically (the
>> > read/modify/write of a larger quantity that gcc does on some bitfield
>> > cases has been flagged as a gcc bug, but shouldn't happen on normal
>> > scalar types).
>> >
>> > I am not 100% certain of "bool" here, I assume it's treated as a normal
>> > scalar and thus atomic but if unsure, you can always use int.
>>
>> Please use an aligned int or long. Some machines cannot do atomic
>> accesses on sub-int/long quantities, so 'bool' may cause unexpected
>> rmw cycles on adjacent fields.
>
> Yeah, at least pre-EV56 Alpha performs rmw cycles on char/short accesses
> and thus those are not atomic.
Ok, thanks.
And I just confirmed with the Alpha cross-compiler that the fields are
not 'padded out' if volatile either.
Do any arches consider this an 'optimization'? I ask because this kind of
accidental adjacency sharing may be common. Even RCU has a char field,
rcu_read_unlock_special, in the middle of the task_struct; luckily the
adjacent field is a list_head.
Besides updating the documentation, it may make sense to do something
arch-specific. Just bumping out storage on arches that don't need it
seems wasteful, as does generating bus locks on arches that don't need it.
Unfortunately, the code churn looks unavoidable.
Regards,
Peter Hurley
^ permalink raw reply
* Re: bit fields && data tearing
From: Jakub Jelinek @ 2014-09-04 12:29 UTC (permalink / raw)
To: Peter Hurley
Cc: linux-arch, Mikael Pettersson, Tony Luck, linux-ia64,
Oleg Nesterov, linux-kernel, Paul Mackerras, Paul E. McKenney,
linuxppc-dev, Miroslav Franc, Richard Henderson
In-Reply-To: <540859EC.5000407@hurleysoftware.com>
On Thu, Sep 04, 2014 at 08:24:12AM -0400, Peter Hurley wrote:
> And I just confirmed with the Alpha cross-compiler that the fields are
> not 'padded out' if volatile either.
They can't be, struct layout is part of the ABI.
Guess you can introduce say atomic_bool and similar typedefs which would be
bool or char on most arches and on alpha int.
Jakub
^ permalink raw reply
* [PATCH 1/2] sched: Add helper for task stack page overrun checking
From: Aaron Tomlin @ 2014-09-04 14:50 UTC (permalink / raw)
To: peterz
Cc: dzickus, jcastillo, riel, minchan, bmr, x86, oleg, rostedt,
linux-kernel, hannes, mingo, aneesh.kumar, atomlin, tglx,
linuxppc-dev, akpm, pzijlstr
In-Reply-To: <1409842224-11847-1-git-send-email-atomlin@redhat.com>
This facility is used in a few places so let's introduce
a helper function to improve readability.
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
arch/powerpc/mm/fault.c | 6 ++----
arch/x86/mm/fault.c | 5 +----
include/linux/sched.h | 3 +++
kernel/trace/trace_stack.c | 5 ++---
4 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 51ab9e7..5cffc7c 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -30,7 +30,6 @@
#include <linux/kprobes.h>
#include <linux/kdebug.h>
#include <linux/perf_event.h>
-#include <linux/magic.h>
#include <linux/ratelimit.h>
#include <linux/context_tracking.h>
@@ -508,7 +507,6 @@ bail:
void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
{
const struct exception_table_entry *entry;
- unsigned long *stackend;
/* Are we prepared to handle this fault? */
if ((entry = search_exception_tables(regs->nip)) != NULL) {
@@ -537,8 +535,8 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
printk(KERN_ALERT "Faulting instruction address: 0x%08lx\n",
regs->nip);
- stackend = end_of_stack(current);
- if (current != &init_task && *stackend != STACK_END_MAGIC)
+ if (current != &init_task &&
+ task_stack_end_corrupted(current))
printk(KERN_ALERT "Thread overran stack, or stack corrupted\n");
die("Kernel access of bad area", regs, sig);
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a241946..b5b9c09 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -3,7 +3,6 @@
* Copyright (C) 2001, 2002 Andi Kleen, SuSE Labs.
* Copyright (C) 2008-2009, Red Hat Inc., Ingo Molnar
*/
-#include <linux/magic.h> /* STACK_END_MAGIC */
#include <linux/sched.h> /* test_thread_flag(), ... */
#include <linux/kdebug.h> /* oops_begin/end, ... */
#include <linux/module.h> /* search_exception_table */
@@ -649,7 +648,6 @@ no_context(struct pt_regs *regs, unsigned long error_code,
unsigned long address, int signal, int si_code)
{
struct task_struct *tsk = current;
- unsigned long *stackend;
unsigned long flags;
int sig;
@@ -709,8 +707,7 @@ no_context(struct pt_regs *regs, unsigned long error_code,
show_fault_oops(regs, error_code, address);
- stackend = end_of_stack(tsk);
- if (tsk != &init_task && *stackend != STACK_END_MAGIC)
+ if (tsk != &init_task && task_stack_end_corrupted(tsk))
printk(KERN_EMERG "Thread overran stack, or stack corrupted\n");
tsk->thread.cr2 = address;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 5c2c885..36d93aa 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -57,6 +57,7 @@ struct sched_param {
#include <linux/llist.h>
#include <linux/uidgid.h>
#include <linux/gfp.h>
+#include <linux/magic.h>
#include <asm/processor.h>
@@ -2614,6 +2615,8 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
}
#endif
+#define task_stack_end_corrupted(task) \
+ (*(end_of_stack(task)) != STACK_END_MAGIC)
static inline int object_is_on_stack(void *obj)
{
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 8a4e5cb..06c7390 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -13,7 +13,6 @@
#include <linux/sysctl.h>
#include <linux/init.h>
#include <linux/fs.h>
-#include <linux/magic.h>
#include <asm/setup.h>
@@ -171,8 +170,8 @@ check_stack(unsigned long ip, unsigned long *stack)
i++;
}
- if ((current != &init_task &&
- *(end_of_stack(current)) != STACK_END_MAGIC)) {
+ if (current != &init_task &&
+ task_stack_end_corrupted(current)) {
print_max_stack();
BUG();
}
--
1.9.3
^ permalink raw reply related
* [PATCH 0/2] sched: Always check the integrity of the canary
From: Aaron Tomlin @ 2014-09-04 14:50 UTC (permalink / raw)
To: peterz
Cc: dzickus, jcastillo, riel, minchan, bmr, x86, oleg, rostedt,
linux-kernel, hannes, mingo, aneesh.kumar, atomlin, tglx,
linuxppc-dev, akpm, pzijlstr
Currently in the event of a stack overrun a call to schedule()
does not check for this type of corruption. This corruption is
often silent and can go unnoticed. However once the corrupted
region is examined at a later stage, the outcome is undefined
and often results in a sporadic page fault which cannot be
handled.
The first patch provides a helper to determine the integrity
of the canary. While the second patch checks for a stack
overrun and takes appropriate action since the damage is
already done, there is no point in continuing.
Aaron Tomlin (2):
sched: Add helper for task stack page overrun checking
sched: BUG when stack end location is over written
arch/powerpc/mm/fault.c | 6 ++----
arch/x86/mm/fault.c | 5 +----
include/linux/sched.h | 3 +++
kernel/sched/core.c | 3 +++
kernel/trace/trace_stack.c | 5 ++---
5 files changed, 11 insertions(+), 11 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH 2/2] sched: BUG when stack end location is over written
From: Aaron Tomlin @ 2014-09-04 14:50 UTC (permalink / raw)
To: peterz
Cc: dzickus, jcastillo, riel, minchan, bmr, x86, oleg, rostedt,
linux-kernel, hannes, mingo, aneesh.kumar, atomlin, tglx,
linuxppc-dev, akpm, pzijlstr
In-Reply-To: <1409842224-11847-1-git-send-email-atomlin@redhat.com>
Currently in the event of a stack overrun a call to schedule()
does not check for this type of corruption. This corruption is
often silent and can go unnoticed. However once the corrupted
region is examined at a later stage, the outcome is undefined
and often results in a sporadic page fault which cannot be
handled.
This patch checks for a stack overrun and takes appropriate
action since the damage is already done, there is no point
in continuing.
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
kernel/sched/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ec1a286..d6af6a0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2660,6 +2660,9 @@ static noinline void __schedule_bug(struct task_struct *prev)
*/
static inline void schedule_debug(struct task_struct *prev)
{
+ if (unlikely(prev != &init_task &&
+ task_stack_end_corrupted(prev)))
+ BUG();
/*
* Test if we are atomic. Since do_exit() needs to call into
* schedule() atomically, we ignore that path. Otherwise whine
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/2] sched: Add helper for task stack page overrun checking
From: Oleg Nesterov @ 2014-09-04 15:02 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, pzijlstr, riel, peterz, bmr, x86, linux-kernel, rostedt,
minchan, mingo, aneesh.kumar, hannes, tglx, linuxppc-dev, akpm,
jcastillo
In-Reply-To: <1409842224-11847-2-git-send-email-atomlin@redhat.com>
On 09/04, Aaron Tomlin wrote:
>
> +#define task_stack_end_corrupted(task) \
> + (*(end_of_stack(task)) != STACK_END_MAGIC)
and it is always used along with "tsk != init_task".
But why we exclude swapper/0? Can't we add
end_of_stack(current) = STACK_END_MAGIC;
somewhere at the start of start_kernel() ?
If not, perhaps this new helper should check "task != &init_task"
itself so that we can simplify its users?
Oleg.
>
> static inline int object_is_on_stack(void *obj)
> {
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 8a4e5cb..06c7390 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -13,7 +13,6 @@
> #include <linux/sysctl.h>
> #include <linux/init.h>
> #include <linux/fs.h>
> -#include <linux/magic.h>
>
> #include <asm/setup.h>
>
> @@ -171,8 +170,8 @@ check_stack(unsigned long ip, unsigned long *stack)
> i++;
> }
>
> - if ((current != &init_task &&
> - *(end_of_stack(current)) != STACK_END_MAGIC)) {
> + if (current != &init_task &&
> + task_stack_end_corrupted(current)) {
> print_max_stack();
> BUG();
> }
> --
> 1.9.3
>
^ permalink raw reply
* Re: [PATCH 1/2] sched: Add helper for task stack page overrun checking
From: Peter Zijlstra @ 2014-09-04 15:30 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, bmr, x86, oleg, rostedt, linux-kernel,
minchan, mingo, aneesh.kumar, hannes, tglx, linuxppc-dev, akpm,
pzijlstr
In-Reply-To: <1409842224-11847-2-git-send-email-atomlin@redhat.com>
On Thu, Sep 04, 2014 at 03:50:23PM +0100, Aaron Tomlin wrote:
> @@ -537,8 +535,8 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
> printk(KERN_ALERT "Faulting instruction address: 0x%08lx\n",
> regs->nip);
>
> - stackend = end_of_stack(current);
> - if (current != &init_task && *stackend != STACK_END_MAGIC)
> + if (current != &init_task &&
> + task_stack_end_corrupted(current))
superfluous linebreak.
> @@ -2614,6 +2615,8 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
> }
>
> #endif
Here otoh we could do with some extra whitespace
> @@ -171,8 +170,8 @@ check_stack(unsigned long ip, unsigned long *stack)
> i++;
> }
>
> - if ((current != &init_task &&
> - *(end_of_stack(current)) != STACK_END_MAGIC)) {
> + if (current != &init_task &&
> + task_stack_end_corrupted(current)) {
Again, superfluous linebreak.
^ permalink raw reply
* Re: [PATCH 2/2] sched: BUG when stack end location is over written
From: Peter Zijlstra @ 2014-09-04 15:32 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, bmr, x86, oleg, rostedt, linux-kernel,
minchan, mingo, aneesh.kumar, hannes, tglx, linuxppc-dev, akpm,
pzijlstr
In-Reply-To: <1409842224-11847-3-git-send-email-atomlin@redhat.com>
On Thu, Sep 04, 2014 at 03:50:24PM +0100, Aaron Tomlin wrote:
> Currently in the event of a stack overrun a call to schedule()
> does not check for this type of corruption. This corruption is
> often silent and can go unnoticed. However once the corrupted
> region is examined at a later stage, the outcome is undefined
> and often results in a sporadic page fault which cannot be
> handled.
>
> This patch checks for a stack overrun and takes appropriate
> action since the damage is already done, there is no point
> in continuing.
>
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
> kernel/sched/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index ec1a286..d6af6a0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2660,6 +2660,9 @@ static noinline void __schedule_bug(struct task_struct *prev)
> */
> static inline void schedule_debug(struct task_struct *prev)
> {
> + if (unlikely(prev != &init_task &&
> + task_stack_end_corrupted(prev)))
> + BUG();
superfluous linebreak, also we appear to have BUG_ON() for situations
just like these.
secondly, while I appreciate the 'feature' you're making schedule()
slower for everybody, what do you propose to do about that?
^ permalink raw reply
* Re: [PATCH 1/2] sched: Add helper for task stack page overrun checking
From: Aaron Tomlin @ 2014-09-04 15:52 UTC (permalink / raw)
To: Oleg Nesterov
Cc: dzickus, pzijlstr, riel, peterz, bmr, x86, linux-kernel, rostedt,
minchan, mingo, aneesh.kumar, hannes, tglx, linuxppc-dev, akpm,
jcastillo
In-Reply-To: <20140904150234.GA6480@redhat.com>
On Thu, Sep 04, 2014 at 05:02:34PM +0200, Oleg Nesterov wrote:
> On 09/04, Aaron Tomlin wrote:
> >
> > +#define task_stack_end_corrupted(task) \
> > + (*(end_of_stack(task)) != STACK_END_MAGIC)
>
> and it is always used along with "tsk != init_task".
>
> But why we exclude swapper/0? Can't we add
>
> end_of_stack(current) = STACK_END_MAGIC;
>
> somewhere at the start of start_kernel() ?
Good point. I can look into it.
> If not, perhaps this new helper should check "task != &init_task"
> itself so that we can simplify its users?
>
> Oleg.
>
> >
> > static inline int object_is_on_stack(void *obj)
> > {
> > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> > index 8a4e5cb..06c7390 100644
> > --- a/kernel/trace/trace_stack.c
> > +++ b/kernel/trace/trace_stack.c
> > @@ -13,7 +13,6 @@
> > #include <linux/sysctl.h>
> > #include <linux/init.h>
> > #include <linux/fs.h>
> > -#include <linux/magic.h>
> >
> > #include <asm/setup.h>
> >
> > @@ -171,8 +170,8 @@ check_stack(unsigned long ip, unsigned long *stack)
> > i++;
> > }
> >
> > - if ((current != &init_task &&
> > - *(end_of_stack(current)) != STACK_END_MAGIC)) {
> > + if (current != &init_task &&
> > + task_stack_end_corrupted(current)) {
> > print_max_stack();
> > BUG();
> > }
> > --
> > 1.9.3
> >
>
--
Aaron Tomlin
^ permalink raw reply
* Re: [PATCH 2/2] sched: BUG when stack end location is over written
From: Aaron Tomlin @ 2014-09-04 16:11 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dzickus, jcastillo, riel, bmr, x86, oleg, rostedt, linux-kernel,
minchan, mingo, aneesh.kumar, akpm, hannes, tglx, linuxppc-dev,
pzijlstr
In-Reply-To: <20140904153231.GE346@worktop.programming.kicks-ass.net>
On Thu, Sep 04, 2014 at 05:32:31PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 04, 2014 at 03:50:24PM +0100, Aaron Tomlin wrote:
> > Currently in the event of a stack overrun a call to schedule()
> > does not check for this type of corruption. This corruption is
> > often silent and can go unnoticed. However once the corrupted
> > region is examined at a later stage, the outcome is undefined
> > and often results in a sporadic page fault which cannot be
> > handled.
> >
> > This patch checks for a stack overrun and takes appropriate
> > action since the damage is already done, there is no point
> > in continuing.
> >
> > Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> > ---
> > kernel/sched/core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index ec1a286..d6af6a0 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -2660,6 +2660,9 @@ static noinline void __schedule_bug(struct task_struct *prev)
> > */
> > static inline void schedule_debug(struct task_struct *prev)
> > {
> > + if (unlikely(prev != &init_task &&
> > + task_stack_end_corrupted(prev)))
> > + BUG();
>
> superfluous linebreak, also we appear to have BUG_ON() for situations
> just like these.
>
> secondly, while I appreciate the 'feature' you're making schedule()
> slower for everybody, what do you propose to do about that?
Understood. I will wrap this with a suitable Kconfig option.
--
Aaron Tomlin
^ permalink raw reply
* Re: bit fields && data tearing
From: One Thousand Gnomes @ 2014-09-04 16:50 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, Mikael Pettersson, Tony Luck, linux-ia64,
Oleg Nesterov, linux-kernel, Paul Mackerras, linux-arch,
Paul E. McKenney, linuxppc-dev, Miroslav Franc, Richard Henderson
In-Reply-To: <540859EC.5000407@hurleysoftware.com>
> Besides updating the documentation, it may make sense to do something
> arch-specific. Just bumping out storage on arches that don't need it
> seems wasteful, as does generating bus locks on arches that don't need it.
> Unfortunately, the code churn looks unavoidable.
The arch specific is pretty much set_bit and friends. Bus locks on a
locally owned cache line should not be very expensive on anything vaguely
modern, while uniprocessor boxes usually only have to generate set_bit
as a single instruction so it is interrupt safe.
Alan
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-04 19:42 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Jakub Jelinek, Mikael Pettersson, Tony Luck, linux-ia64,
Oleg Nesterov, linux-kernel, Paul Mackerras, linux-arch,
Paul E. McKenney, linuxppc-dev, Miroslav Franc, Richard Henderson
In-Reply-To: <20140904175044.4697aee4@alan.etchedpixels.co.uk>
On 09/04/2014 12:50 PM, One Thousand Gnomes wrote:
>> Besides updating the documentation, it may make sense to do something
>> arch-specific. Just bumping out storage on arches that don't need it
>> seems wasteful, as does generating bus locks on arches that don't need it.
>> Unfortunately, the code churn looks unavoidable.
>
> The arch specific is pretty much set_bit and friends. Bus locks on a
> locally owned cache line should not be very expensive on anything vaguely
> modern, while uniprocessor boxes usually only have to generate set_bit
> as a single instruction so it is interrupt safe.
Or we could give up on the Alpha.
It's not just the non-atomic bytes; we could do away with the
read_barrier_depends() which hardly any code gets correctly anyway.
Regards,
Peter Hurley
^ permalink raw reply
* Re: [PATCH v2] iommu/fsl: Fix warning resulting from adding PCI device twice
From: Emil Medve @ 2014-09-04 20:56 UTC (permalink / raw)
To: Varun Sethi, joro, iommu, alex.williamson, linuxppc-dev,
linux-kernel
In-Reply-To: <1409830725-4469-1-git-send-email-Varun.Sethi@freescale.com>
On 09/04/2014 06:38 AM, Varun Sethi wrote:
> iommu_group_get_for_dev determines the iommu group for the PCI device and adds
> the device to the group.
>
> In the PAMU driver we were again adding the device to the same group without checking
> if the device already had an iommu group. This resulted in the following warning.
>
> sysfs: cannot create duplicate filename '/devices/ffe200000.pcie/pci0000:00/0000:00:00.0/iommu_group'
> ------------[ cut here ]------------
> WARNING: at fs/sysfs/dir.c:31
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc3-00002-g7505cea-dirty #126
> task: c0000001fe0a0000 ti: c0000001fe044000 task.ti: c0000001fe044000
> NIP: c00000000018879c LR: c000000000188798 CTR: c00000000001ea50
> REGS: c0000001fe047040 TRAP: 0700 Not tainted (3.17.0-rc3-00002-g7505cea-dirty)
> MSR: 0000000080029000 <CE,EE,ME> CR: 24ad8e22 XER: 20000000
> SOFTE: 1
> GPR00: c000000000188798 c0000001fe0472c0 c0000000009a52e0 0000000000000065
> GPR04: 0000000000000001 0000000000000000 3a30303a00000000 0000000027000000
> GPR08: 2f696f6d00000000 c0000000008d3830 c0000000009b3938 c0000000009bb3d0
> GPR12: 0000000028ad8e24 c00000000fff4000 c00000000000205c 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000008a4c70
> GPR24: c0000000007e9010 c0000001fe0140a8 ffffffffffffffef 0000000000000001
> GPR28: c0000001fe22ebb8 c0000000007e9010 c00000000090bf10 c0000001fe220000
> NIP [c00000000018879c] .sysfs_warn_dup+0x74/0xa4
> LR [c000000000188798] .sysfs_warn_dup+0x70/0xa4
> Call Trace:
> [c0000001fe0472c0] [c000000000188798] .sysfs_warn_dup+0x70/0xa4 (unreliable)
> [c0000001fe047350] [c000000000188d34] .sysfs_do_create_link_sd.clone.2+0x168/0x174
> [c0000001fe047400] [c0000000004b3cf8] .iommu_group_add_device+0x78/0x244
> [c0000001fe0474b0] [c0000000004b6964] .fsl_pamu_add_device+0x88/0x1a8
> [c0000001fe047570] [c0000000004b3960] .iommu_bus_notifier+0xdc/0x15c
> [c0000001fe047600] [c000000000059848] .notifier_call_chain+0x8c/0xe8
> [c0000001fe0476a0] [c000000000059d04] .__blocking_notifier_call_chain+0x58/0x84
> [c0000001fe047750] [c00000000036619c] .device_add+0x464/0x5c8
> [c0000001fe047820] [c000000000300ebc] .pci_device_add+0x14c/0x17c
> [c0000001fe0478c0] [c000000000300fbc] .pci_scan_single_device+0xd0/0xf4
> [c0000001fe047970] [c00000000030104c] .pci_scan_slot+0x6c/0x18c
> [c0000001fe047a10] [c00000000030226c] .pci_scan_child_bus+0x40/0x114
> [c0000001fe047ac0] [c000000000021974] .pcibios_scan_phb+0x240/0x2c8
> [c0000001fe047b70] [c00000000085a970] .pcibios_init+0x64/0xc8
> [c0000001fe047c00] [c000000000001884] .do_one_initcall+0xbc/0x224
> [c0000001fe047d00] [c000000000852d50] .kernel_init_freeable+0x14c/0x21c
> [c0000001fe047db0] [c000000000002078] .kernel_init+0x1c/0xfa4
> [c0000001fe047e30] [c000000000000884] .ret_from_kernel_thread+0x58/0xd4
> Instruction dump:
> 7c7f1b79 4182001c 7fe4fb78 7f83e378 38a01000 4bffc905 60000000 7c641b78
> e87e8008 7fa5eb78 48482ff5 60000000 <0fe00000> 7fe3fb78 4bf7bd39 60000000
>
>
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
Tested-by: Emil Medve <Emilian.Medve@Freescale.com>
^ permalink raw reply
* Re: [PATCH] IBM Akebono: Remove obsolete config select
From: Paul Bolle @ 2014-09-04 22:20 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1774195.a1B25gz7RT@mexican>
On Fri, 2014-06-20 at 10:10 +1000, Alistair Popple wrote:
> Hi Ben,
>
> It looks like we may have missed this trivial fix? Can you please apply it to
> your tree?
>
> Regards,
>
> Alistair
>
> On Fri, 13 Jun 2014 13:56:32 Paul Bolle wrote:
> > On Fri, 2014-05-02 at 18:06 +1000, Alistair Popple wrote:
> > > The original implementation of MMC support for Akebono introduced a
> > > new configuration symbol (MMC_SDHCI_OF_476GTR). This symbol has been
> > > dropped in favour of using the generic platform driver however the
> > > select for this symbol was mistakenly left in the platform
> > > configuration.
> > >
> > > This patch removes the obsolete symbol selection.
> > >
> > > Signed-off-by: Alistair Popple <alistair@popple.id.au>
> >
> > This patch hasn't yet entered linux-next nor Linus' tree. Is it queued
> > somewhere? If not, would a
> > Acked-by: Paul Bolle <pebolle@tiscali.nl>
> >
> > help to get this trivial patch queued for either of those trees?
> >
> >
> > Paul Bolle
> >
> > > ---
> > >
> > > arch/powerpc/platforms/44x/Kconfig | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/arch/powerpc/platforms/44x/Kconfig
> > > b/arch/powerpc/platforms/44x/Kconfig index 8beec7d..908bf11 100644
> > > --- a/arch/powerpc/platforms/44x/Kconfig
> > > +++ b/arch/powerpc/platforms/44x/Kconfig
> > > @@ -220,7 +220,6 @@ config AKEBONO
> > >
> > > select USB_EHCI_HCD_PLATFORM
> > > select MMC_SDHCI
> > > select MMC_SDHCI_PLTFM
> > >
> > > - select MMC_SDHCI_OF_476GTR
> > >
> > > select ATA
> > > select SATA_AHCI_PLATFORM
> > > help
This trivial cleanup is still not in linux-next nor in Linus' tree.
Could someone else please have a look at it?
Thanks,
Paul Bolle
^ permalink raw reply
* Re: [4/5] IBM Akebono: Add the Akebono platform
From: Paul Bolle @ 2014-09-04 22:29 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <10214508.Vq3Ynjy2Co@mexican>
Hi Alistair,
On Fri, 2014-05-02 at 10:35 +1000, Alistair Popple wrote:
> On Thu, 1 May 2014 11:27:27 Paul Bolle wrote:
> > On Thu, 2014-03-06 at 14:52 +1100, Alistair Popple wrote:
> > > This patch adds support for the IBM Akebono board.
> > > + select IBM_EMAC_RGMII_WOL
> >
> > The patch that added this symbol (and the related driver) was submitted
> > in https://lkml.org/lkml/2014/2/21/25 . It's not (yet) included in
> > linux-next. Is it queued somewhere else?
>
> To be honest I'm not sure. I will follow this up on the netdev list.
I noticed that this symbol (and its driver) are still not in v3.17-rc3
nor in next-20140904. Any update on this?
Thanks,
Paul Bolle
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox