* Re: linux-next oops in windfarm startup
From: Hugh Dickins @ 2012-05-08 1:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1336435741.2540.184.camel@pasglop>
On Tue, 8 May 2012, Benjamin Herrenschmidt wrote:
> On Mon, 2012-05-07 at 08:57 -0700, Hugh Dickins wrote:
> > Hi Ben,
> >
> > 3.4.0-rc5-next-20120504 and 3.4.0-rc6-next-20120507
> > crash while booting the PowerMac G5: I have to revert
> > your commit e326b30fda9985a2e7fda6fb9212b86bf025c39
> > powerpc/pmac: Convert windfarm_smu_sat to new i2c probing
> > to get a working system.
>
> Argh, stoopid me ... and I thought I had tested it .... just
> on the wrong machine. Oh well try this and let me know:
>
> powerpc/windfarm: Fix crash on SMU based machine after i2c conversion
>
> We no longer get the device node in platform_data but instead
> where it belongs in struct device, so get it from there instead
> of blowing up.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Yep, that fixes it for me: thanks a lot!
Hugh
> ---
>
> diff --git a/drivers/macintosh/windfarm_smu_sat.c b/drivers/macintosh/windfarm_smu_sat.c
> index e2989ce..426e810 100644
> --- a/drivers/macintosh/windfarm_smu_sat.c
> +++ b/drivers/macintosh/windfarm_smu_sat.c
> @@ -204,7 +204,7 @@ static struct wf_sensor_ops wf_sat_ops = {
> static int wf_sat_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - struct device_node *dev = client->dev.platform_data;
> + struct device_node *dev = client->dev.of_node;
> struct wf_sat *sat;
> struct wf_sat_sensor *sens;
> const u32 *reg;
^ permalink raw reply
* Re: linux-next oops in windfarm startup
From: Benjamin Herrenschmidt @ 2012-05-08 0:09 UTC (permalink / raw)
To: Hugh Dickins; +Cc: linuxppc-dev
In-Reply-To: <alpine.LSU.2.00.1205070851020.1416@eggly.anvils>
On Mon, 2012-05-07 at 08:57 -0700, Hugh Dickins wrote:
> Hi Ben,
>
> 3.4.0-rc5-next-20120504 and 3.4.0-rc6-next-20120507
> crash while booting the PowerMac G5: I have to revert
> your commit e326b30fda9985a2e7fda6fb9212b86bf025c39
> powerpc/pmac: Convert windfarm_smu_sat to new i2c probing
> to get a working system.
Argh, stoopid me ... and I thought I had tested it .... just
on the wrong machine. Oh well try this and let me know:
powerpc/windfarm: Fix crash on SMU based machine after i2c conversion
We no longer get the device node in platform_data but instead
where it belongs in struct device, so get it from there instead
of blowing up.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
diff --git a/drivers/macintosh/windfarm_smu_sat.c b/drivers/macintosh/windfarm_smu_sat.c
index e2989ce..426e810 100644
--- a/drivers/macintosh/windfarm_smu_sat.c
+++ b/drivers/macintosh/windfarm_smu_sat.c
@@ -204,7 +204,7 @@ static struct wf_sensor_ops wf_sat_ops = {
static int wf_sat_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct device_node *dev = client->dev.platform_data;
+ struct device_node *dev = client->dev.of_node;
struct wf_sat *sat;
struct wf_sat_sensor *sens;
const u32 *reg;
^ permalink raw reply related
* Re: linux-next ppc64: RCU mods cause __might_sleep BUGs
From: Hugh Dickins @ 2012-05-07 21:38 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linuxppc-dev, Christoph Lameter, linux-kernel, Paul E. McKenney
In-Reply-To: <20120507185017.GA21152@linux.vnet.ibm.com>
On Mon, 7 May 2012, Paul E. McKenney wrote:
> On Mon, May 07, 2012 at 09:21:54AM -0700, Hugh Dickins wrote:
> >
> > In 70 hours I got six isolated messages like the below (but from
> > different __might_sleep callsites) - where before I'd have flurries
> > of hundreds(?) and freeze within the hour.
> >
> > And the "rcu_nesting" debug line I'd added to the message was different:
> > where before it was showing ffffffff on some tasks and 1 on others i.e.
> > increment or decrement had been applied to the wrong task, these messages
> > now all showed 0s throughout i.e. by the time the message was printed,
> > there was no longer any justification for the message.
> >
> > As if a memory barrier were missing somewhere, perhaps.
>
> These fields should be updated only by the corresponding CPU, so
> if memory barriers are needed, it seems to me that the cross-CPU
> access is the bug, not the lack of a memory barrier.
Yes: the code you added appeared to be using local CPU accesses only
(very much intentionally), and the context switch should already have
provided all the memory barriers needed there.
>
> Ah... Is preemption disabled across the access to RCU's nesting level
> when printing out the message? If not, a preeemption at that point
> could result in the value printed being inaccurate.
Preemption was enabled in the cases I saw. So you're pointing out that
#define rcu_preempt_depth() (__this_cpu_read(rcu_read_lock_nesting))
should have been
#define rcu_preempt_depth() (this_cpu_read(rcu_read_lock_nesting))
to avoid the danger of spurious __might_sleep() warnings.
Yes, I believe you've got it - thanks.
Hugh
^ permalink raw reply
* [PATCH 17/21] powerpc/PCI: register busn_res for root buses
From: Yinghai Lu @ 2012-05-07 20:30 UTC (permalink / raw)
To: Bjorn Helgaas, Benjamin Herrenschmidt, Tony Luck, David Miller,
x86
Cc: linux-arch, Greg Kroah-Hartman, linuxppc-dev, linux-kernel,
Dominik Brodowski, Yinghai Lu, linux-pci, Paul Mackerras,
Andrew Morton, Linus Torvalds
In-Reply-To: <1336422606-4244-1-git-send-email-yinghai@kernel.org>
Add the host bridge bus number aperture to the resource list.
Like the MMIO and I/O port apertures, this is used when assigning
resources to hot-added devices or in the case of conflicts.
[bhelgaas: write changelog]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/pci-bridge.h | 1 +
arch/powerpc/kernel/pci-common.c | 17 +++++++++++------
arch/powerpc/kernel/pci_of_scan.c | 2 +-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index ac39e6a..8cccbee 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -30,6 +30,7 @@ struct pci_controller {
int first_busno;
int last_busno;
int self_busno;
+ struct resource busn;
void __iomem *io_base_virt;
#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 3532b53..7320f36 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1637,6 +1637,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
/* Wire up PHB bus resources */
pcibios_setup_phb_resources(hose, &resources);
+ hose->busn.start = hose->first_busno;
+ hose->busn.end = hose->last_busno;
+ hose->busn.flags = IORESOURCE_BUS;
+ pci_add_resource(&resources, &hose->busn);
+
/* Create an empty bus for the toplevel */
bus = pci_create_root_bus(hose->parent, hose->first_busno,
hose->ops, hose, &resources);
@@ -1646,7 +1651,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
pci_free_resource_list(&resources);
return;
}
- bus->busn_res.start = hose->first_busno;
hose->bus = bus;
/* Get probe mode and perform scan */
@@ -1654,13 +1658,14 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
if (node && ppc_md.pci_probe_mode)
mode = ppc_md.pci_probe_mode(bus);
pr_debug(" probe mode: %d\n", mode);
- if (mode == PCI_PROBE_DEVTREE) {
- bus->busn_res.end = hose->last_busno;
+ if (mode == PCI_PROBE_DEVTREE)
of_scan_bus(node, bus);
- }
- if (mode == PCI_PROBE_NORMAL)
- hose->last_busno = bus->busn_res.end = pci_scan_child_bus(bus);
+ if (mode == PCI_PROBE_NORMAL) {
+ pci_bus_update_busn_res_end(bus, 255);
+ hose->last_busno = pci_scan_child_bus(bus);
+ pci_bus_update_busn_res_end(bus, hose->last_busno);
+ }
/* Platform gets a chance to do some global fixups before
* we proceed to resource allocation
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index a36281a..ae5ea5e 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -240,7 +240,7 @@ void __devinit of_scan_pci_bridge(struct pci_dev *dev)
}
bus->primary = dev->bus->number;
- bus->busn_res.end = busrange[1];
+ pci_bus_insert_busn_res(bus, busrange[0], busrange[1]);
bus->bridge_ctl = 0;
/* parse ranges property */
--
1.7.7
^ permalink raw reply related
* Re: linux-next ppc64: RCU mods cause __might_sleep BUGs
From: Paul E. McKenney @ 2012-05-07 18:50 UTC (permalink / raw)
To: Hugh Dickins
Cc: linuxppc-dev, Christoph Lameter, linux-kernel, Paul E. McKenney
In-Reply-To: <alpine.LSU.2.00.1205070858450.1416@eggly.anvils>
On Mon, May 07, 2012 at 09:21:54AM -0700, Hugh Dickins wrote:
> On Wed, 2 May 2012, Hugh Dickins wrote:
> > On Wed, 2 May 2012, Paul E. McKenney wrote:
> > >
> > > In any case, I must confess that I feel quite silly about my series
> > > of patches. I have reverted them aside from a couple that did useful
> > > optimizations, and they should show up in -next shortly.
> >
> > A wee bit sad, but thank you - it was an experiment worth trying,
> > and perhaps there will be reason to come back to it future.
>
> The revert indeed showed up in next-20120504: thanks, no problem now.
>
> But although it's just history, and not worth anyone's time to
> investigate, I shouldn't let this thread die without an epilogue.
>
> Although the patch I posted (this_cpu_inc in __rcu_read_lock,
> preempt_disable and enable in __rcu_read_unlock) ran well until
> I killed the test after 70 hours, it did not _entirely_ eliminate
> the sleeping function BUG messages.
>
> In 70 hours I got six isolated messages like the below (but from
> different __might_sleep callsites) - where before I'd have flurries
> of hundreds(?) and freeze within the hour.
>
> And the "rcu_nesting" debug line I'd added to the message was different:
> where before it was showing ffffffff on some tasks and 1 on others i.e.
> increment or decrement had been applied to the wrong task, these messages
> now all showed 0s throughout i.e. by the time the message was printed,
> there was no longer any justification for the message.
>
> As if a memory barrier were missing somewhere, perhaps.
These fields should be updated only by the corresponding CPU, so
if memory barriers are needed, it seems to me that the cross-CPU
access is the bug, not the lack of a memory barrier.
Ah... Is preemption disabled across the access to RCU's nesting level
when printing out the message? If not, a preeemption at that point
could result in the value printed being inaccurate.
Thanx, Paul
> BUG: sleeping function called from invalid context at arch/powerpc/mm/fault.c:305
> cpu=2 preempt_count=0 preempt_offset=0 rcu_nesting=0 nesting_save=0
> in_atomic(): 0, irqs_disabled(): 0, pid: 12266, name: cc1
> Call Trace:
> [c000000003affac0] [c00000000000f36c] .show_stack+0x6c/0x16c (unreliable)
> [c000000003affb70] [c000000000078788] .__might_sleep+0x150/0x170
> [c000000003affc00] [c0000000000255f4] .do_page_fault+0x288/0x664
> [c000000003affe30] [c000000000005868] handle_page_fault+0x10/0x30
>
> Hugh
> --
> 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
* spufs: possible circular locking dependency detected
From: masterzorag @ 2012-05-07 17:03 UTC (permalink / raw)
To: linuxppc-dev
I'm running my test program, it uses all available spus to compute via
OpenCL
kernel 3.4-rc1 on a ps3
even on testing spu directly, it crashes
======================================================
[ INFO: possible circular locking dependency detected ]
3.4.0-rc1 #2 Not tainted
-------------------------------------------------------
test/964 is trying to acquire lock:
(&mm->mmap_sem){++++++}, at: [<d0000000005ff430>]
.spufs_ps_fault+0x160/0x210 [spufs]
but task is already holding lock:
(&ctx->state_mutex){+.+...}, at: [<d0000000005ff404>]
.spufs_ps_fault+0x134/0x210 [spufs]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&ctx->state_mutex){+.+...}:
[<c00000000033974c>] .mutex_lock_interruptible_nested+0x70/0x4a0
[<d000000000600d2c>] .spufs_mem_mmap_fault+0xb0/0x160 [spufs]
[<c0000000000c23f0>] .__do_fault+0x108/0x554
[<c0000000000254d8>] .do_page_fault+0x3c0/0x5b0
[<c0000000000058e8>] handle_page_fault+0x10/0x30
-> #0 (&mm->mmap_sem){++++++}:
[<c000000000084cc8>] .lock_acquire+0x98/0xcc
[<c00000000033ab58>] .down_read+0x34/0x78
[<d0000000005ff430>] .spufs_ps_fault+0x160/0x210 [spufs]
[<c0000000000c23f0>] .__do_fault+0x108/0x554
[<c0000000000254d8>] .do_page_fault+0x3c0/0x5b0
[<c0000000000058e8>] handle_page_fault+0x10/0x30
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&ctx->state_mutex);
lock(&mm->mmap_sem);
lock(&ctx->state_mutex);
lock(&mm->mmap_sem);
*** DEADLOCK ***
1 lock held by test/964:
#0: (&ctx->state_mutex){+.+...}, at: [<d0000000005ff404>]
.spufs_ps_fault+0x134/0x210 [spufs]
stack backtrace:
Call Trace:
[c000000006f6b790] [c000000000010278] .show_stack+0x6c/0x16c (unreliable)
[c000000006f6b840] [c000000000082974] .print_circular_bug+0x2f8/0x330
[c000000006f6b8f0] [c0000000000844f0] .__lock_acquire+0x131c/0x1a5c
[c000000006f6ba40] [c000000000084cc8] .lock_acquire+0x98/0xcc
[c000000006f6bb10] [c00000000033ab58] .down_read+0x34/0x78
[c000000006f6bb90] [d0000000005ff430] .spufs_ps_fault+0x160/0x210 [spufs]
[c000000006f6bc70] [c0000000000c23f0] .__do_fault+0x108/0x554
[c000000006f6bd70] [c0000000000254d8] .do_page_fault+0x3c0/0x5b0
[c000000006f6be30] [c0000000000058e8] handle_page_fault+0x10/0x30
^ permalink raw reply
* Re: linux-next ppc64: RCU mods cause __might_sleep BUGs
From: Hugh Dickins @ 2012-05-07 16:21 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linuxppc-dev, Christoph Lameter, linux-kernel, Paul E. McKenney
In-Reply-To: <alpine.LSU.2.00.1205021721350.24869@eggly.anvils>
On Wed, 2 May 2012, Hugh Dickins wrote:
> On Wed, 2 May 2012, Paul E. McKenney wrote:
> >
> > In any case, I must confess that I feel quite silly about my series
> > of patches. I have reverted them aside from a couple that did useful
> > optimizations, and they should show up in -next shortly.
>
> A wee bit sad, but thank you - it was an experiment worth trying,
> and perhaps there will be reason to come back to it future.
The revert indeed showed up in next-20120504: thanks, no problem now.
But although it's just history, and not worth anyone's time to
investigate, I shouldn't let this thread die without an epilogue.
Although the patch I posted (this_cpu_inc in __rcu_read_lock,
preempt_disable and enable in __rcu_read_unlock) ran well until
I killed the test after 70 hours, it did not _entirely_ eliminate
the sleeping function BUG messages.
In 70 hours I got six isolated messages like the below (but from
different __might_sleep callsites) - where before I'd have flurries
of hundreds(?) and freeze within the hour.
And the "rcu_nesting" debug line I'd added to the message was different:
where before it was showing ffffffff on some tasks and 1 on others i.e.
increment or decrement had been applied to the wrong task, these messages
now all showed 0s throughout i.e. by the time the message was printed,
there was no longer any justification for the message.
As if a memory barrier were missing somewhere, perhaps.
BUG: sleeping function called from invalid context at arch/powerpc/mm/fault.c:305
cpu=2 preempt_count=0 preempt_offset=0 rcu_nesting=0 nesting_save=0
in_atomic(): 0, irqs_disabled(): 0, pid: 12266, name: cc1
Call Trace:
[c000000003affac0] [c00000000000f36c] .show_stack+0x6c/0x16c (unreliable)
[c000000003affb70] [c000000000078788] .__might_sleep+0x150/0x170
[c000000003affc00] [c0000000000255f4] .do_page_fault+0x288/0x664
[c000000003affe30] [c000000000005868] handle_page_fault+0x10/0x30
Hugh
^ permalink raw reply
* Re: linux-next oops in windfarm startup
From: Benjamin Herrenschmidt @ 2012-05-07 16:19 UTC (permalink / raw)
To: Hugh Dickins; +Cc: linuxppc-dev
In-Reply-To: <alpine.LSU.2.00.1205070851020.1416@eggly.anvils>
On Mon, 2012-05-07 at 08:57 -0700, Hugh Dickins wrote:
> Hi Ben,
>
> 3.4.0-rc5-next-20120504 and 3.4.0-rc6-next-20120507
> crash while booting the PowerMac G5: I have to revert
> your commit e326b30fda9985a2e7fda6fb9212b86bf025c39
> powerpc/pmac: Convert windfarm_smu_sat to new i2c probing
> to get a working system.
Can you shoot me (privately) a tarball of /proc/device-tree on your
system ?
Cheers,
Ben.
> Actual oops message scrolled off screen, and I didn't note the registers.
> NIP [c000000000376964] of_get_next_child+0x68/0x70
> LR [c000000000376928] of_get_next_child+0x2c/0x70
> Call Trace:
> of_get_next_child
> wf_sat_probe
> i2c_device_probe
> driver_probe_device
> bus_for_each_drv
> device_attach
> i2c_new_device
> i2c_powermac_probe
> platform_drv_probe
> driver_probe_device
> __driver_attach
> bus_for_each_dev
> driver_attach
> bus_add_driver
> driver_register
> platform_driver_register
> i2c_powermac_driver_init
> do_one_initcall
> kernel_init
> kernel_thread
> Instruction dump:
> ebff0040 e87e8008 480b86e9 60000000 38210090 7fe3fb78 e8010010 eba1ffe8
> ebc1fff0 ebe1fff8 7c0803a6 4e800020 <ebfd0038> 4bffffd0 7c0802a6 fbe1fff8
>
> Hugh
^ permalink raw reply
* linux-next oops in windfarm startup
From: Hugh Dickins @ 2012-05-07 15:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
Hi Ben,
3.4.0-rc5-next-20120504 and 3.4.0-rc6-next-20120507
crash while booting the PowerMac G5: I have to revert
your commit e326b30fda9985a2e7fda6fb9212b86bf025c39
powerpc/pmac: Convert windfarm_smu_sat to new i2c probing
to get a working system.
Actual oops message scrolled off screen, and I didn't note the registers.
NIP [c000000000376964] of_get_next_child+0x68/0x70
LR [c000000000376928] of_get_next_child+0x2c/0x70
Call Trace:
of_get_next_child
wf_sat_probe
i2c_device_probe
driver_probe_device
bus_for_each_drv
device_attach
i2c_new_device
i2c_powermac_probe
platform_drv_probe
driver_probe_device
__driver_attach
bus_for_each_dev
driver_attach
bus_add_driver
driver_register
platform_driver_register
i2c_powermac_driver_init
do_one_initcall
kernel_init
kernel_thread
Instruction dump:
ebff0040 e87e8008 480b86e9 60000000 38210090 7fe3fb78 e8010010 eba1ffe8
ebc1fff0 ebe1fff8 7c0803a6 4e800020 <ebfd0038> 4bffffd0 7c0802a6 fbe1fff8
Hugh
^ permalink raw reply
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers, second version
From: Mauro Carvalho Chehab @ 2012-05-07 15:37 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Egor Martovetsky, Niklas Söderlund, Tim Small, Arvind R.,
Chris Metcalf, Olof Johansson, Doug Thompson,
Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <20120507133204.GC11462@aftab.osrc.amd.com>
Em 07-05-2012 10:32, Borislav Petkov escreveu:
> breaking thread because it grew too big.
>
> On Fri, May 04, 2012 at 07:48:42AM -0300, Mauro Carvalho Chehab wrote:
>
> [ … ]
>
>> + memset(&pos, 0, sizeof(pos));
>> + row = 0;
>> + chn = 0;
>> + debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
>> + per_rank ? "ranks" : "dimms");
>> + for (i = 0; i < tot_dimms; i++) {
>> + chan = &csi[row].channels[chn];
>> + dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
>> + pos[0], pos[1], pos[2]);
>> + dimm->mci = mci;
>> +
>> + debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
>> + i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
>> + pos[0], pos[1], pos[2], row, chn);
>> +
>> + /* Copy DIMM location */
>> + for (j = 0; j < n_layers; j++)
>> + dimm->location[j] = pos[j];
>> +
>> + /* Link it to the csrows old API data */
>> + chan->dimm = dimm;
>> + dimm->csrow = row;
>> + dimm->cschannel = chn;
>> +
>> + /* Increment csrow location */
>> + for (j = n_layers - 1; j >= 0; j--)
>> + if (layers[j].is_virt_csrow)
>> + break;
>
> This looks fishy: the for-loop above iterates over layers[j] to break on
> the first ->is_virt_csrow.
That loop is dead code. It was related to the "rev_order" handling.
I'll drop it.
>
>> + row++;
>
> And row here gets incremented unconditionally, independent from the loop
> above. And you're not using any results from the loop: j gets reset
> below in the next loop.
>
> What's going on?
>
>> + if (row == tot_csrows) {
>> + row = 0;
>> + chn++;
>> + }
>> +
>> + /* Increment dimm location */
>> + for (j = n_layers - 1; j >= 0; j--) {
>> + pos[j]++;
>> + if (pos[j] < layers[j].size)
>> + break;
>> + pos[j] = 0;
>> }
>> }
>>
>> @@ -263,6 +373,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
>> */
>> return mci;
>> }
>> +EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
>> +
>> +/**
>> + * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
>> + * @mc_num: Memory controller number
>> + * @n_layers: Number of layers at the MC hierarchy
>> + * layers: Describes each layer as seen by the Memory Controller
>> + * @size_pvt: Size of private storage needed
>> + *
>> + *
>> + * FIXME: drivers handle multi-rank memories in different ways: some
>> + * drivers map multi-ranked DIMMs as one DIMM while others
>> + * as several DIMMs.
>> + *
>> + * Everything is kmalloc'ed as one big chunk - more efficient.
>> + * It can only be used if all structures have the same lifetime - otherwise
>> + * you have to allocate and initialize your own structures.
>> + *
>> + * Use edac_mc_free() to free mc structures allocated by this function.
>> + *
>> + * Returns:
>> + * On failure: NULL
>> + * On success: struct mem_ctl_info pointer
>> + */
>> +
>
> [ … ]
>
>> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
>> + struct mem_ctl_info *mci,
>> + const unsigned long page_frame_number,
>> + const unsigned long offset_in_page,
>> + const unsigned long syndrome,
>> + const int layer0,
>> + const int layer1,
>> + const int layer2,
>> + const char *msg,
>> + const char *other_detail,
>> + const void *mcelog)
>> {
>> - int len = EDAC_MC_LABEL_LEN * 4;
>> - char labels[len + 1];
>> - char *pos = labels;
>> - int chan;
>> - int chars;
>> - char *label = NULL;
>> + /* FIXME: too much for stack: move it to some pre-alocated area */
>
> I'm assuming all those new FIXMEs are going to be addressed soonish :)
That's the idea ;) Some are fixed just after the drivers conversion.
This stack FIXME I'll solve together with some other cleanups I'm intending
to write after merging the existing chansesets (as there are still a lot
of patches on this series). The stack usage here is no too much to cause
any problems, but still I don't want to have large stuff like that at stack.
Regards,
Mauro
Patch without the dead code enclosed.
--
edac: Change internal representation to work with layers
From: Mauro Carvalho Chehab <mchehab@redhat.com>
Change the EDAC internal representation to work with non-csrow
based memory controllers.
There are lots of those memory controllers nowadays, and more
are coming. So, the EDAC internal representation needs to be
changed, in order to work with those memory controllers, while
preserving backward compatibility with the old ones.
The edac core was written with the idea that memory controllers
are able to directly access csrows.
This is not true for FB-DIMM and RAMBUS memory controllers.
Also, some recent advanced memory controllers don't present a per-csrows
view. Instead, they view memories as DIMMs, instead of ranks.
So, change the allocation and error report routines to allow
them to work with all types of architectures.
This will allow the removal of several hacks with FB-DIMM and RAMBUS
memory controllers.
Also, several tests were done on different platforms using different
x86 drivers.
TODO: a multi-rank DIMMs are currently represented by multiple DIMM
entries in struct dimm_info. That means that changing a label for one
rank won't change the same label for the other ranks at the same DIMM.
This bug is present since the beginning of the EDAC, so it is not a big
deal. However, on several drivers, it is possible to fix this issue, but
it should be a per-driver fix, as the csrow => DIMM arrangement may not
be equal for all. So, don't try to fix it here yet.
I tried to make this patch as short as possible, preceding it with
several other patches that simplified the logic here. Yet, as the
internal API changes, all drivers need changes. The changes are
generally bigger in the drivers for FB-DIMMs.
Cc: Aristeu Rozanski <arozansk@redhat.com>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Borislav Petkov <borislav.petkov@amd.com>
Cc: Mark Gross <mark.gross@intel.com>
Cc: Jason Uhlenkott <juhlenko@akamai.com>
Cc: Tim Small <tim@buttersideup.com>
Cc: Ranganathan Desikan <ravi@jetztechnologies.com>
Cc: "Arvind R." <arvino55@gmail.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Egor Martovetsky <egor@pasemi.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joe Perches <joe@perches.com>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Hitoshi Mitake <h.mitake@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Niklas Söderlund" <niklas.soderlund@ericsson.com>
Cc: Shaohui Xie <Shaohui.Xie@freescale.com>
Cc: Josh Boyer <jwboyer@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
v 20: remove a dead code.
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index e48ab31..1286c5e 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -447,8 +447,12 @@ static inline void pci_write_bits32(struct pci_dev *pdev, int offset,
#endif /* CONFIG_PCI */
-extern struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index);
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int edac_index);
+struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt);
extern int edac_mc_add_mc(struct mem_ctl_info *mci);
extern void edac_mc_free(struct mem_ctl_info *mci);
extern struct mem_ctl_info *edac_mc_find(int idx);
@@ -467,24 +471,78 @@ extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
* reporting logic and function interface - reduces conditional
* statement clutter and extra function arguments.
*/
-extern void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page,
- unsigned long syndrome, int row, int channel,
- const char *msg);
-extern void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row,
- const char *msg);
-extern void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel0, unsigned int channel1,
- char *msg);
-extern void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel, char *msg);
+
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog);
+
+static inline void edac_mc_handle_ce(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page,
+ unsigned long syndrome, int row, int channel,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ page_frame_number, offset_in_page, syndrome,
+ row, channel, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page, int row,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ page_frame_number, offset_in_page, 0,
+ row, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel0,
+ unsigned int channel1,
+ char *msg)
+{
+ /*
+ *FIXME: The error can also be at channel1 (e. g. at the second
+ * channel of the same branch). The fix is to push
+ * edac_mc_handle_error() call into each driver
+ */
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel0, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel, char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel, -1, msg, NULL, NULL);
+}
/*
* edac_device APIs
@@ -496,6 +554,7 @@ extern void edac_device_handle_ue(struct edac_device_ctl_info *edac_dev,
extern void edac_device_handle_ce(struct edac_device_ctl_info *edac_dev,
int inst_nr, int block_nr, const char *msg);
extern int edac_device_alloc_index(void);
+extern const char *edac_layer_name[];
/*
* edac_pci APIs
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6ec967a..510514f 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -44,9 +44,25 @@ static void edac_mc_dump_channel(struct rank_info *chan)
debugf4("\tchannel = %p\n", chan);
debugf4("\tchannel->chan_idx = %d\n", chan->chan_idx);
debugf4("\tchannel->csrow = %p\n\n", chan->csrow);
- debugf4("\tdimm->ce_count = %d\n", chan->dimm->ce_count);
- debugf4("\tdimm->label = '%s'\n", chan->dimm->label);
- debugf4("\tdimm->nr_pages = 0x%x\n", chan->dimm->nr_pages);
+ debugf4("\tchannel->dimm = %p\n", chan->dimm);
+}
+
+static void edac_mc_dump_dimm(struct dimm_info *dimm)
+{
+ int i;
+
+ debugf4("\tdimm = %p\n", dimm);
+ debugf4("\tdimm->label = '%s'\n", dimm->label);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
+ debugf4("\tdimm location ");
+ for (i = 0; i < dimm->mci->n_layers; i++) {
+ printk(KERN_CONT "%d", dimm->location[i]);
+ if (i < dimm->mci->n_layers - 1)
+ printk(KERN_CONT ".");
+ }
+ printk(KERN_CONT "\n");
+ debugf4("\tdimm->grain = %d\n", dimm->grain);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
}
static void edac_mc_dump_csrow(struct csrow_info *csrow)
@@ -70,6 +86,8 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci)
debugf4("\tmci->edac_check = %p\n", mci->edac_check);
debugf3("\tmci->nr_csrows = %d, csrows = %p\n",
mci->nr_csrows, mci->csrows);
+ debugf3("\tmci->nr_dimms = %d, dimms = %p\n",
+ mci->tot_dimms, mci->dimms);
debugf3("\tdev = %p\n", mci->dev);
debugf3("\tmod_name:ctl_name = %s:%s\n", mci->mod_name, mci->ctl_name);
debugf3("\tpvt_info = %p\n\n", mci->pvt_info);
@@ -157,10 +175,12 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
}
/**
- * edac_mc_alloc: Allocate a struct mem_ctl_info structure
- * @size_pvt: size of private storage needed
- * @nr_csrows: Number of CWROWS needed for this MC
- * @nr_chans: Number of channels for the MC
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of MC hierarchy layers
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: size of private storage needed
+ *
*
* Everything is kmalloc'ed as one big chunk - more efficient.
* Only can be used if all structures have the same lifetime - otherwise
@@ -168,22 +188,49 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
*
* Use edac_mc_free() to free mc structures allocated by this function.
*
+ * NOTE: drivers handle multi-rank memories in different ways: in some
+ * drivers, one multi-rank memory stick is mapped as one entry, while, in
+ * others, a single multi-rank memory stick would be mapped into several
+ * entries. Currently, this function will allocate multiple struct dimm_info
+ * on such scenarios, as grouping the multiple ranks require drivers change.
+ *
* Returns:
* NULL allocation failed
* struct mem_ctl_info pointer
*/
-struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index)
+struct mem_ctl_info *new_edac_mc_alloc(unsigned mc_num,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt)
{
- void *ptr = NULL;
struct mem_ctl_info *mci;
- struct csrow_info *csi, *csrow;
+ struct edac_mc_layer *layer;
+ struct csrow_info *csi, *csr;
struct rank_info *chi, *chp, *chan;
struct dimm_info *dimm;
- void *pvt;
- unsigned size;
- int row, chn;
- int err;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+ unsigned pos[EDAC_MAX_LAYERS];
+ void *pvt, *ptr = NULL;
+ unsigned size, tot_dimms = 1, count = 1;
+ unsigned tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
+ int i, j, err, row, chn;
+ bool per_rank = false;
+
+ BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
+ /*
+ * Calculate the total amount of dimms and csrows/cschannels while
+ * in the old API emulation mode
+ */
+ for (i = 0; i < n_layers; i++) {
+ tot_dimms *= layers[i].size;
+ if (layers[i].is_virt_csrow)
+ tot_csrows *= layers[i].size;
+ else
+ tot_channels *= layers[i].size;
+
+ if (layers[i].type == EDAC_MC_LAYER_CHIP_SELECT)
+ per_rank = true;
+ }
/* Figure out the offsets of the various items from the start of an mc
* structure. We want the alignment of each item to be at least as
@@ -191,12 +238,27 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
* hardcode everything into a single struct.
*/
mci = edac_align_ptr(&ptr, sizeof(*mci), 1);
- csi = edac_align_ptr(&ptr, sizeof(*csi), nr_csrows);
- chi = edac_align_ptr(&ptr, sizeof(*chi), nr_csrows * nr_chans);
- dimm = edac_align_ptr(&ptr, sizeof(*dimm), nr_csrows * nr_chans);
+ layer = edac_align_ptr(&ptr, sizeof(*layer), n_layers);
+ csi = edac_align_ptr(&ptr, sizeof(*csi), tot_csrows);
+ chi = edac_align_ptr(&ptr, sizeof(*chi), tot_csrows * tot_channels);
+ dimm = edac_align_ptr(&ptr, sizeof(*dimm), tot_dimms);
+ for (i = 0; i < n_layers; i++) {
+ count *= layers[i].size;
+ debugf4("%s: errcount layer %d size %d\n", __func__, i, count);
+ ce_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ ue_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ tot_errcount += 2 * count;
+ }
+
+ debugf4("%s: allocating %d error counters\n", __func__, tot_errcount);
pvt = edac_align_ptr(&ptr, sz_pvt, 1);
size = ((unsigned long)pvt) + sz_pvt;
+ debugf1("%s(): allocating %u bytes for mci data (%d %s, %d csrows/channels)\n",
+ __func__, size,
+ tot_dimms,
+ per_rank ? "ranks" : "dimms",
+ tot_csrows * tot_channels);
mci = kzalloc(size, GFP_KERNEL);
if (mci == NULL)
return NULL;
@@ -204,42 +266,87 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
/* Adjust pointers so they point within the memory we just allocated
* rather than an imaginary chunk of memory located at address 0.
*/
+ layer = (struct edac_mc_layer *)(((char *)mci) + ((unsigned long)layer));
csi = (struct csrow_info *)(((char *)mci) + ((unsigned long)csi));
chi = (struct rank_info *)(((char *)mci) + ((unsigned long)chi));
dimm = (struct dimm_info *)(((char *)mci) + ((unsigned long)dimm));
+ for (i = 0; i < n_layers; i++) {
+ mci->ce_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ce_per_layer[i]));
+ mci->ue_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ue_per_layer[i]));
+ }
pvt = sz_pvt ? (((char *)mci) + ((unsigned long)pvt)) : NULL;
/* setup index and various internal pointers */
- mci->mc_idx = edac_index;
+ mci->mc_idx = mc_num;
mci->csrows = csi;
mci->dimms = dimm;
+ mci->tot_dimms = tot_dimms;
mci->pvt_info = pvt;
- mci->nr_csrows = nr_csrows;
+ mci->n_layers = n_layers;
+ mci->layers = layer;
+ memcpy(mci->layers, layers, sizeof(*layer) * n_layers);
+ mci->nr_csrows = tot_csrows;
+ mci->num_cschannel = tot_channels;
+ mci->mem_is_per_rank = per_rank;
/*
- * For now, assumes that a per-csrow arrangement for dimms.
- * This will be latter changed.
+ * Fill the csrow struct
*/
- dimm = mci->dimms;
-
- for (row = 0; row < nr_csrows; row++) {
- csrow = &csi[row];
- csrow->csrow_idx = row;
- csrow->mci = mci;
- csrow->nr_channels = nr_chans;
- chp = &chi[row * nr_chans];
- csrow->channels = chp;
-
- for (chn = 0; chn < nr_chans; chn++) {
+ for (row = 0; row < tot_csrows; row++) {
+ csr = &csi[row];
+ csr->csrow_idx = row;
+ csr->mci = mci;
+ csr->nr_channels = tot_channels;
+ chp = &chi[row * tot_channels];
+ csr->channels = chp;
+
+ for (chn = 0; chn < tot_channels; chn++) {
chan = &chp[chn];
chan->chan_idx = chn;
- chan->csrow = csrow;
+ chan->csrow = csr;
+ }
+ }
- mci->csrows[row].channels[chn].dimm = dimm;
- dimm->csrow = row;
- dimm->csrow_channel = chn;
- dimm++;
- mci->nr_dimms++;
+ /*
+ * Fill the dimm struct
+ */
+ memset(&pos, 0, sizeof(pos));
+ row = 0;
+ chn = 0;
+ debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
+ per_rank ? "ranks" : "dimms");
+ for (i = 0; i < tot_dimms; i++) {
+ chan = &csi[row].channels[chn];
+ dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
+ pos[0], pos[1], pos[2]);
+ dimm->mci = mci;
+
+ debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
+ i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
+ pos[0], pos[1], pos[2], row, chn);
+
+ /* Copy DIMM location */
+ for (j = 0; j < n_layers; j++)
+ dimm->location[j] = pos[j];
+
+ /* Link it to the csrows old API data */
+ chan->dimm = dimm;
+ dimm->csrow = row;
+ dimm->cschannel = chn;
+
+ /* Increment csrow location */
+ row++;
+ if (row == tot_csrows) {
+ row = 0;
+ chn++;
+ }
+
+ /* Increment dimm location */
+ for (j = n_layers - 1; j >= 0; j--) {
+ pos[j]++;
+ if (pos[j] < layers[j].size)
+ break;
+ pos[j] = 0;
}
}
@@ -263,6 +370,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
*/
return mci;
}
+EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
+
+/**
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of layers at the MC hierarchy
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: Size of private storage needed
+ *
+ *
+ * FIXME: drivers handle multi-rank memories in different ways: some
+ * drivers map multi-ranked DIMMs as one DIMM while others
+ * as several DIMMs.
+ *
+ * Everything is kmalloc'ed as one big chunk - more efficient.
+ * It can only be used if all structures have the same lifetime - otherwise
+ * you have to allocate and initialize your own structures.
+ *
+ * Use edac_mc_free() to free mc structures allocated by this function.
+ *
+ * Returns:
+ * On failure: NULL
+ * On success: struct mem_ctl_info pointer
+ */
+
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int mc_num)
+{
+ unsigned n_layers = 2;
+ struct edac_mc_layer layers[n_layers];
+
+ layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+ layers[0].size = nr_csrows;
+ layers[0].is_virt_csrow = true;
+ layers[1].type = EDAC_MC_LAYER_CHANNEL;
+ layers[1].size = nr_chans;
+ layers[1].is_virt_csrow = false;
+
+ return new_edac_mc_alloc(mc_num, ARRAY_SIZE(layers), layers, sz_pvt);
+}
EXPORT_SYMBOL_GPL(edac_mc_alloc);
/**
@@ -528,7 +675,6 @@ EXPORT_SYMBOL(edac_mc_find);
* edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
* create sysfs entries associated with mci structure
* @mci: pointer to the mci structure to be added to the list
- * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
*
* Return:
* 0 Success
@@ -555,6 +701,8 @@ int edac_mc_add_mc(struct mem_ctl_info *mci)
edac_mc_dump_channel(&mci->csrows[i].
channels[j]);
}
+ for (i = 0; i < mci->tot_dimms; i++)
+ edac_mc_dump_dimm(&mci->dimms[i]);
}
#endif
mutex_lock(&mem_ctls_mutex);
@@ -712,261 +860,289 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
}
EXPORT_SYMBOL_GPL(edac_mc_find_csrow_by_page);
-/* FIXME - setable log (warning/emerg) levels */
-/* FIXME - integrate with evlog: http://evlog.sourceforge.net/ */
-void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, unsigned long syndrome,
- int row, int channel, const char *msg)
+const char *edac_layer_name[] = {
+ [EDAC_MC_LAYER_BRANCH] = "branch",
+ [EDAC_MC_LAYER_CHANNEL] = "channel",
+ [EDAC_MC_LAYER_SLOT] = "slot",
+ [EDAC_MC_LAYER_CHIP_SELECT] = "csrow",
+};
+EXPORT_SYMBOL_GPL(edac_layer_name);
+
+static void edac_inc_ce_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
{
- unsigned long remapped_page;
- char *label = NULL;
- u32 grain;
+ int i, index = 0;
- debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
+ mci->ce_count++;
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- if (channel >= mci->csrows[row].nr_channels || channel < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range "
- "(%d >= %d)\n", channel,
- mci->csrows[row].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ce_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_inc_ue_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
+{
+ int i, index = 0;
+
+ mci->ue_count++;
+
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- label = mci->csrows[row].channels[channel].dimm->label;
- grain = mci->csrows[row].channels[channel].dimm->grain;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ue_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_ce_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ u32 grain)
+{
+ unsigned long remapped_page;
if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
edac_mc_printk(mci, KERN_WARNING,
- "CE page 0x%lx, offset 0x%lx, grain %d, syndrome "
- "0x%lx, row %d, channel %d, label \"%s\": %s\n",
- page_frame_number, offset_in_page,
- grain, syndrome, row, channel,
- label, msg);
-
- mci->ce_count++;
- mci->csrows[row].ce_count++;
- mci->csrows[row].channels[channel].dimm->ce_count++;
- mci->csrows[row].channels[channel].ce_count++;
+ "CE %s on %s (%s%s %s)\n",
+ msg, label, location,
+ detail, other_detail);
+ edac_inc_ce_error(mci, enable_per_layer_report, pos);
if (mci->scrub_mode & SCRUB_SW_SRC) {
/*
- * Some MC's can remap memory so that it is still available
- * at a different address when PCI devices map into memory.
- * MC's that can't do this lose the memory where PCI devices
- * are mapped. This mapping is MC dependent and so we call
- * back into the MC driver for it to map the MC page to
- * a physical (CPU) page which can then be mapped to a virtual
- * page - which can then be scrubbed.
- */
+ * Some memory controllers (called MCs below) can remap
+ * memory so that it is still available at a different
+ * address when PCI devices map into memory.
+ * MC's that can't do this, lose the memory where PCI
+ * devices are mapped. This mapping is MC-dependent
+ * and so we call back into the MC driver for it to
+ * map the MC page to a physical (CPU) page which can
+ * then be mapped to a virtual page - which can then
+ * be scrubbed.
+ */
remapped_page = mci->ctl_page_to_phys ?
mci->ctl_page_to_phys(mci, page_frame_number) :
page_frame_number;
- edac_mc_scrub_block(remapped_page, offset_in_page, grain);
+ edac_mc_scrub_block(remapped_page,
+ offset_in_page, grain);
}
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce);
-void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
+static void edac_ue_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report)
{
- if (edac_mc_get_log_ce())
+ if (edac_mc_get_log_ue())
edac_mc_printk(mci, KERN_WARNING,
- "CE - no information available: %s\n", msg);
+ "UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
- mci->ce_noinfo_count++;
- mci->ce_count++;
+ if (edac_mc_get_panic_on_ue())
+ panic("UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
+
+ edac_inc_ue_error(mci, enable_per_layer_report, pos);
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce_no_info);
-void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row, const char *msg)
+#define OTHER_LABEL " or "
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog)
{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chan;
- int chars;
- char *label = NULL;
+ /* FIXME: too much for stack: move it to some pre-alocated area */
+ char detail[80], location[80];
+ char label[(EDAC_MC_LABEL_LEN + 1 + sizeof(OTHER_LABEL)) * mci->tot_dimms];
+ char *p;
+ int row = -1, chan = -1;
+ int pos[EDAC_MAX_LAYERS] = { layer0, layer1, layer2 };
+ int i;
u32 grain;
+ bool enable_per_layer_report = false;
debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
-
- grain = mci->csrows[row].channels[0].dimm->grain;
- label = mci->csrows[row].channels[0].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- for (chan = 1; (chan < mci->csrows[row].nr_channels) && (len > 0);
- chan++) {
- label = mci->csrows[row].channels[chan].dimm->label;
- chars = snprintf(pos, len + 1, ":%s", label);
- len -= chars;
- pos += chars;
+ /*
+ * Check if the event report is consistent and if the memory
+ * location is known. If it is known, enable_per_layer_report will be
+ * true, the DIMM(s) label info will be filled and the per-layer
+ * error counters will be incremented.
+ */
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] >= (int)mci->layers[i].size) {
+ if (type == HW_EVENT_ERR_CORRECTED)
+ p = "CE";
+ else
+ p = "UE";
+
+ edac_mc_printk(mci, KERN_ERR,
+ "INTERNAL ERROR: %s value is out of range (%d >= %d)\n",
+ edac_layer_name[mci->layers[i].type],
+ pos[i], mci->layers[i].size);
+ /*
+ * Instead of just returning it, let's use what's
+ * known about the error. The increment routines and
+ * the DIMM filter logic will do the right thing by
+ * pointing the likely damaged DIMMs.
+ */
+ pos[i] = -1;
+ }
+ if (pos[i] >= 0)
+ enable_per_layer_report = true;
}
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE page 0x%lx, offset 0x%lx, grain %d, row %d, "
- "labels \"%s\": %s\n", page_frame_number,
- offset_in_page, grain, row, labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: UE page 0x%lx, offset 0x%lx, grain %d, "
- "row %d, labels \"%s\": %s\n", mci->mc_idx,
- page_frame_number, offset_in_page,
- grain, row, labels, msg);
-
- mci->ue_count++;
- mci->csrows[row].ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue);
+ /*
+ * Get the dimm label/grain that applies to the match criteria.
+ * As the error algorithm may not be able to point to just one memory
+ * stick, the logic here will get all possible labels that could
+ * pottentially be affected by the error.
+ * On FB-DIMM memory controllers, for uncorrected errors, it is common
+ * to have only the MC channel and the MC dimm (also called "branch")
+ * but the channel is not known, as the memory is arranged in pairs,
+ * where each memory belongs to a separate channel within the same
+ * branch.
+ */
+ grain = 0;
+ p = label;
+ *p = '\0';
+ for (i = 0; i < mci->tot_dimms; i++) {
+ struct dimm_info *dimm = &mci->dimms[i];
-void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
-{
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: Uncorrected Error", mci->mc_idx);
+ if (layer0 >= 0 && layer0 != dimm->location[0])
+ continue;
+ if (layer1 >= 0 && layer1 != dimm->location[1])
+ continue;
+ if (layer2 >= 0 && layer2 != dimm->location[2])
+ continue;
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_WARNING,
- "UE - no information available: %s\n", msg);
- mci->ue_noinfo_count++;
- mci->ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue_no_info);
+ /* get the max grain, over the error match range */
+ if (dimm->grain > grain)
+ grain = dimm->grain;
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process UE events
- */
-void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
- unsigned int csrow,
- unsigned int channela,
- unsigned int channelb, char *msg)
-{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chars;
- char *label;
-
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
-
- if (channela >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-a out of range "
- "(%d >= %d)\n",
- channela, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ /*
+ * If the error is memory-controller wide, there's no need to
+ * seek for the affected DIMMs because the whole
+ * channel/memory controller/... may be affected.
+ * Also, don't show errors for empty DIMM slots.
+ */
+ if (enable_per_layer_report && dimm->nr_pages) {
+ if (p != label) {
+ strcpy(p, OTHER_LABEL);
+ p += strlen(OTHER_LABEL);
+ }
+ strcpy(p, dimm->label);
+ p += strlen(p);
+ *p = '\0';
+
+ /*
+ * get csrow/channel of the DIMM, in order to allow
+ * incrementing the compat API counters
+ */
+ debugf4("%s: %s csrows map: (%d,%d)\n",
+ __func__,
+ mci->mem_is_per_rank ? "rank" : "dimm",
+ dimm->csrow, dimm->cschannel);
+
+ if (row == -1)
+ row = dimm->csrow;
+ else if (row >= 0 && row != dimm->csrow)
+ row = -2;
+
+ if (chan == -1)
+ chan = dimm->cschannel;
+ else if (chan >= 0 && chan != dimm->cschannel)
+ chan = -2;
+ }
}
- if (channelb >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-b out of range "
- "(%d >= %d)\n",
- channelb, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ if (!enable_per_layer_report) {
+ strcpy(label, "any memory");
+ } else {
+ debugf4("%s: csrow/channel to increment: (%d,%d)\n",
+ __func__, row, chan);
+ if (p == label)
+ strcpy(label, "unknown memory");
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ if (row >= 0) {
+ mci->csrows[row].ce_count++;
+ if (chan >= 0)
+ mci->csrows[row].channels[chan].ce_count++;
+ }
+ } else
+ if (row >= 0)
+ mci->csrows[row].ue_count++;
}
- mci->ue_count++;
- mci->csrows[csrow].ue_count++;
-
- /* Generate the DIMM labels from the specified channels */
- label = mci->csrows[csrow].channels[channela].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- chars = snprintf(pos, len + 1, "-%s",
- mci->csrows[csrow].channels[channelb].dimm->label);
-
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela, channelb,
- labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela,
- channelb, labels, msg);
-}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ue);
-
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process CE events
- */
-void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
- unsigned int csrow, unsigned int channel, char *msg)
-{
- char *label = NULL;
+ /* Fill the RAM location data */
+ p = location;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ continue;
- /* Ensure boundary values */
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
- }
- if (channel >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range (%d >= %d)\n",
- channel, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
+ p += sprintf(p, "%s %d ",
+ edac_layer_name[mci->layers[i].type],
+ pos[i]);
}
- label = mci->csrows[csrow].channels[channel].dimm->label;
+ /* Memory type dependent details about the error */
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d syndrome 0x%lx",
+ page_frame_number, offset_in_page,
+ grain, syndrome);
- if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
- edac_mc_printk(mci, KERN_WARNING,
- "CE row %d, channel %d, label \"%s\": %s\n",
- csrow, channel, label, msg);
+ edac_ce_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report,
+ page_frame_number, offset_in_page, grain);
+ } else {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d",
+ page_frame_number, offset_in_page, grain);
- mci->ce_count++;
- mci->csrows[csrow].ce_count++;
- mci->csrows[csrow].channels[channel].dimm->ce_count++;
- mci->csrows[csrow].channels[channel].ce_count++;
+ edac_ue_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report);
+ }
}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ce);
+EXPORT_SYMBOL_GPL(edac_mc_handle_error);
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 3b8798d..c8f507d 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -412,18 +412,20 @@ struct edac_mc_layer {
/* FIXME: add the proper per-location error counts */
struct dimm_info {
char label[EDAC_MC_LABEL_LEN + 1]; /* DIMM label on motherboard */
- unsigned memory_controller;
- unsigned csrow;
- unsigned csrow_channel;
+
+ /* Memory location data */
+ unsigned location[EDAC_MAX_LAYERS];
+
+ struct mem_ctl_info *mci; /* the parent */
u32 grain; /* granularity of reported error in bytes */
enum dev_type dtype; /* memory device type */
enum mem_type mtype; /* memory dimm type */
enum edac_type edac_mode; /* EDAC mode for this dimm */
- u32 nr_pages; /* number of pages in csrow */
+ u32 nr_pages; /* number of pages on this dimm */
- u32 ce_count; /* Correctable Errors for this dimm */
+ unsigned csrow, cschannel; /* Points to the old API data */
};
/**
@@ -443,9 +445,10 @@ struct dimm_info {
*/
struct rank_info {
int chan_idx;
- u32 ce_count;
struct csrow_info *csrow;
struct dimm_info *dimm;
+
+ u32 ce_count; /* Correctable Errors for this csrow */
};
struct csrow_info {
@@ -541,13 +544,18 @@ struct mem_ctl_info {
unsigned long (*ctl_page_to_phys) (struct mem_ctl_info * mci,
unsigned long page);
int mc_idx;
- int nr_csrows;
struct csrow_info *csrows;
+ unsigned nr_csrows, num_cschannel;
+
+ /* Memory Controller hierarchy */
+ unsigned n_layers;
+ struct edac_mc_layer *layers;
+ bool mem_is_per_rank;
/*
* DIMM info. Will eventually remove the entire csrows_info some day
*/
- unsigned nr_dimms;
+ unsigned tot_dimms;
struct dimm_info *dimms;
/*
@@ -562,12 +570,16 @@ struct mem_ctl_info {
const char *dev_name;
char proc_name[MC_PROC_NAME_MAX_LEN + 1];
void *pvt_info;
- u32 ue_noinfo_count; /* Uncorrectable Errors w/o info */
- u32 ce_noinfo_count; /* Correctable Errors w/o info */
- u32 ue_count; /* Total Uncorrectable Errors for this MC */
- u32 ce_count; /* Total Correctable Errors for this MC */
unsigned long start_time; /* mci load start time (in jiffies) */
+ /*
+ * drivers shouldn't access those fields directly, as the core
+ * already handles that.
+ */
+ u32 ce_noinfo_count, ue_noinfo_count;
+ u32 ue_count, ce_count;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+
struct completion complete;
/* edac sysfs device control */
@@ -580,7 +592,7 @@ struct mem_ctl_info {
* by the low level driver.
*
* Set by the low level driver to provide attributes at the
- * controller level, same level as 'ue_count' and 'ce_count' above.
+ * controller level.
* An array of structures, NULL terminated
*
* If attributes are desired, then set to array of attributes
^ permalink raw reply related
* [PATCH] powerpc: fix compile fail in hugetlb cmdline parsing
From: Paul Gortmaker @ 2012-05-07 14:32 UTC (permalink / raw)
To: linuxppc-dev
Cc: Jim Cromie, Greg Kroah-Hartman, Jason Baron, Rusty Russell,
Paul Gortmaker, linux-next
Commit 9fb48c744ba6a4bf58b666f4e6fdac3008ea1bd4
"params: add 3rd arg to option handler callback signature"
added an extra arg to the function, but didn't catch all the use
cases needing it, causing this compile fail in mpc85xx_defconfig:
arch/powerpc/mm/hugetlbpage.c:316:4: error: passing argument 7 of
'parse_args' from incompatible pointer type [-Werror]
include/linux/moduleparam.h:317:12: note: expected
'int (*)(char *, char *, const char *)' but argument is of type
'int (*)(char *, char *)'
This function has no need to printk out the "doing" value, so
just add the arg as an "unused".
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Jim Cromie <jim.cromie@gmail.com>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Becky Bruce <beckyb@kernel.crashing.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index fb05b12..1a6de0a 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -271,7 +271,8 @@ int alloc_bootmem_huge_page(struct hstate *hstate)
unsigned long gpage_npages[MMU_PAGE_COUNT];
-static int __init do_gpage_early_setup(char *param, char *val)
+static int __init do_gpage_early_setup(char *param, char *val,
+ const char *unused)
{
static phys_addr_t size;
unsigned long npages;
--
1.7.9.1
^ permalink raw reply related
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers, second version
From: Borislav Petkov @ 2012-05-07 13:32 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Borislav Petkov, Egor Martovetsky, Niklas Söderlund,
Tim Small, Arvind R., Chris Metcalf, Olof Johansson,
Doug Thompson, Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
breaking thread because it grew too big.
On Fri, May 04, 2012 at 07:48:42AM -0300, Mauro Carvalho Chehab wrote:
[ … ]
> + memset(&pos, 0, sizeof(pos));
> + row = 0;
> + chn = 0;
> + debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
> + per_rank ? "ranks" : "dimms");
> + for (i = 0; i < tot_dimms; i++) {
> + chan = &csi[row].channels[chn];
> + dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
> + pos[0], pos[1], pos[2]);
> + dimm->mci = mci;
> +
> + debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
> + i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
> + pos[0], pos[1], pos[2], row, chn);
> +
> + /* Copy DIMM location */
> + for (j = 0; j < n_layers; j++)
> + dimm->location[j] = pos[j];
> +
> + /* Link it to the csrows old API data */
> + chan->dimm = dimm;
> + dimm->csrow = row;
> + dimm->cschannel = chn;
> +
> + /* Increment csrow location */
> + for (j = n_layers - 1; j >= 0; j--)
> + if (layers[j].is_virt_csrow)
> + break;
This looks fishy: the for-loop above iterates over layers[j] to break on
the first ->is_virt_csrow.
> + row++;
And row here gets incremented unconditionally, independent from the loop
above. And you're not using any results from the loop: j gets reset
below in the next loop.
What's going on?
> + if (row == tot_csrows) {
> + row = 0;
> + chn++;
> + }
> +
> + /* Increment dimm location */
> + for (j = n_layers - 1; j >= 0; j--) {
> + pos[j]++;
> + if (pos[j] < layers[j].size)
> + break;
> + pos[j] = 0;
> }
> }
>
> @@ -263,6 +373,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
> */
> return mci;
> }
> +EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
> +
> +/**
> + * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
> + * @mc_num: Memory controller number
> + * @n_layers: Number of layers at the MC hierarchy
> + * layers: Describes each layer as seen by the Memory Controller
> + * @size_pvt: Size of private storage needed
> + *
> + *
> + * FIXME: drivers handle multi-rank memories in different ways: some
> + * drivers map multi-ranked DIMMs as one DIMM while others
> + * as several DIMMs.
> + *
> + * Everything is kmalloc'ed as one big chunk - more efficient.
> + * It can only be used if all structures have the same lifetime - otherwise
> + * you have to allocate and initialize your own structures.
> + *
> + * Use edac_mc_free() to free mc structures allocated by this function.
> + *
> + * Returns:
> + * On failure: NULL
> + * On success: struct mem_ctl_info pointer
> + */
> +
[ … ]
> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
> + struct mem_ctl_info *mci,
> + const unsigned long page_frame_number,
> + const unsigned long offset_in_page,
> + const unsigned long syndrome,
> + const int layer0,
> + const int layer1,
> + const int layer2,
> + const char *msg,
> + const char *other_detail,
> + const void *mcelog)
> {
> - int len = EDAC_MC_LABEL_LEN * 4;
> - char labels[len + 1];
> - char *pos = labels;
> - int chan;
> - int chars;
> - char *label = NULL;
> + /* FIXME: too much for stack: move it to some pre-alocated area */
I'm assuming all those new FIXMEs are going to be addressed soonish :)
Rest looks ok,
thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply
* [PATCH] powerpc/44x: Support OCM(On Chip Memory) for APM821xx SoC and Bluestone board
From: Vinh Nguyen Huu Tuong @ 2012-05-07 3:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Josh Boyer, Matt Porter,
Grant Likely, Rob Herring, Duc Dang, David S. Miller, Kumar Gala,
Li Yang, Ashish Kalra, Anatolij Gustschin, Liu Gang, linuxppc-dev,
linux-kernel, devicetree-discuss
Cc: Vinh Nguyen Huu Tuong
This patch consists of:
- Add driver for OCM component
- Export OCM Information at /sys/class/ocm/ocminfo
Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
---
arch/powerpc/boot/dts/bluestone.dts | 8 +
arch/powerpc/include/asm/ppc4xx_ocm.h | 47 ++++
arch/powerpc/platforms/44x/Kconfig | 8 +
arch/powerpc/sysdev/Makefile | 1 +
arch/powerpc/sysdev/ppc4xx_ocm.c | 420 +++++++++++++++++++++++++++++++++
5 files changed, 484 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/ppc4xx_ocm.h
create mode 100644 arch/powerpc/sysdev/ppc4xx_ocm.c
diff --git a/arch/powerpc/boot/dts/bluestone.dts b/arch/powerpc/boot/dts/bluestone.dts
index 7bda373..2687c11 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -107,6 +107,14 @@
interrupt-parent = <&UIC0>;
};
+ OCM1: ocm@400040000 {
+ compatible = "ibm,ocm";
+ status = "ok";
+ cell-index = <1>;
+ /* configured in U-Boot */
+ reg = <4 0x00040000 0x8000>; /* 32K */
+ };
+
SDR0: sdr {
compatible = "ibm,sdr-apm821xx";
dcr-reg = <0x00e 0x002>;
diff --git a/arch/powerpc/include/asm/ppc4xx_ocm.h b/arch/powerpc/include/asm/ppc4xx_ocm.h
new file mode 100644
index 0000000..ff7f386
--- /dev/null
+++ b/arch/powerpc/include/asm/ppc4xx_ocm.h
@@ -0,0 +1,47 @@
+/*
+ * PowerPC 4xx OCM memory allocation support
+ *
+ * (C) Copyright 2009, Applied Micro Circuits Corporation
+ * Victor Gallardo (vgallardo@amcc.com)
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ASM_POWERPC_PPC4xx_OCM_H__
+#define __ASM_POWERPC_PPC4xx_OCM_H__
+
+#include <linux/types.h>
+
+#define OCM_NON_CACHED 0
+#define OCM_CACHED 1
+
+#if defined(CONFIG_PPC4xx_OCM)
+
+void *ocm_alloc(phys_addr_t *phys, int size, int align,
+ int flags, const char *owner);
+void ocm_free(const void *virt);
+
+#else
+
+#define ocm_alloc(phys, size, align, flags, owner) NULL
+#define ocm_free(addr) ((void)0)
+
+#endif /* CONFIG_PPC4xx_OCM */
+
+#endif /* __ASM_POWERPC_PPC4xx_OCM_H__ */
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 2e4e64a..6b1a64e 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -250,6 +250,14 @@ config PPC4xx_GPIO
help
Enable gpiolib support for ppc440 based boards
+config PPC4xx_OCM
+ bool "PPC4xx On Chip Memory (OCM) support"
+ depends on 4xx
+ select PPC_LIB_RHEAP
+ help
+ Enable OCM support for PowerPC 4xx platforms with on chip memory,
+ OCM provides the fast place for memory access to improve performance.
+
# 44x specific CPU modules, selected based on the board above.
config 440EP
bool
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 1bd7ecb..6f768e2 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_PPC_INDIRECT_PCI) += indirect_pci.o
obj-$(CONFIG_PPC_I8259) += i8259.o
obj-$(CONFIG_IPIC) += ipic.o
obj-$(CONFIG_4xx) += uic.o
+obj-$(CONFIG_PPC4xx_OCM) += ppc4xx_ocm.o
obj-$(CONFIG_4xx_SOC) += ppc4xx_soc.o
obj-$(CONFIG_XILINX_VIRTEX) += xilinx_intc.o
obj-$(CONFIG_XILINX_PCI) += xilinx_pci.o
diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c b/arch/powerpc/sysdev/ppc4xx_ocm.c
new file mode 100644
index 0000000..ba3e450
--- /dev/null
+++ b/arch/powerpc/sysdev/ppc4xx_ocm.c
@@ -0,0 +1,420 @@
+/*
+ * PowerPC 4xx OCM memory allocation support
+ *
+ * (C) Copyright 2009, Applied Micro Circuits Corporation
+ * Victor Gallardo (vgallardo@amcc.com)
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <linux/spinlock.h>
+#include <linux/dma-mapping.h>
+#include <linux/list.h>
+#include <asm/uaccess.h>
+#include <asm/prom.h>
+#include <asm/dcr.h>
+#include <asm/rheap.h>
+#include <asm/mmu.h>
+#include <asm/ppc4xx_ocm.h>
+#include <linux/export.h>
+
+#define OCM_DISABLED 0
+#define OCM_ENABLED 1
+
+struct ocm_block {
+ struct list_head list;
+ void __iomem *addr;
+ int size;
+ const char *owner;
+};
+
+/* non-cached or cached region */
+struct ocm_region {
+ phys_addr_t phys;
+ void __iomem *virt;
+
+ int memtotal;
+ int memfree;
+
+ rh_info_t *rh;
+ struct list_head list;
+};
+
+struct ocm_info {
+ int index;
+ int status;
+ int ready;
+
+ phys_addr_t phys;
+
+ int alignment;
+ int memtotal;
+ int cache_size;
+
+ struct ocm_region nc; /* non-cached region */
+ struct ocm_region c; /* cached region */
+};
+
+static struct ocm_info *ocm_nodes;
+static int ocm_count;
+
+static struct ocm_info *ocm_get_node(unsigned int index)
+{
+ if (index >= ocm_count) {
+ printk(KERN_ERR "OCM: invalid index");
+ return NULL;
+ }
+
+ return &ocm_nodes[index];
+}
+
+void *ocm_alloc(phys_addr_t *phys, int size, int align,
+ int flags, const char *owner)
+{
+ void __iomem *addr = NULL;
+ unsigned long offset;
+ struct ocm_info *ocm;
+ struct ocm_region *ocm_reg;
+ struct ocm_block *ocm_blk;
+ int i;
+
+ for (i = 0; i < ocm_count; i++) {
+ ocm = ocm_get_node(i);
+
+ if (!ocm || !ocm->ready)
+ continue;
+
+ if (flags == OCM_NON_CACHED)
+ ocm_reg = &ocm->nc;
+ else
+ ocm_reg = &ocm->c;
+
+ if (!ocm_reg->virt)
+ continue;
+
+ if (align < ocm->alignment)
+ align = ocm->alignment;
+
+ offset = rh_alloc_align(ocm_reg->rh, size, align, NULL);
+
+ if (IS_ERR_VALUE(offset))
+ continue;
+
+ ocm_blk = kzalloc(sizeof(struct ocm_block *), GFP_KERNEL);
+ if (!ocm_blk) {
+ printk(KERN_ERR "OCM: could not allocate ocm block");
+ rh_free(ocm_reg->rh, offset);
+ break;
+ }
+
+ *phys = ocm_reg->phys + offset;
+ addr = ocm_reg->virt + offset;
+ size = ALIGN(size, align);
+
+ ocm_blk->addr = addr;
+ ocm_blk->size = size;
+ ocm_blk->owner = owner;
+ list_add_tail(&ocm_blk->list, &ocm_reg->list);
+
+ ocm_reg->memfree -= size;
+
+ break;
+ }
+
+ return addr;
+}
+
+static int ocm_free_region(struct ocm_region *ocm_reg, const void *addr)
+{
+ struct ocm_block *blk, *tmp;
+ unsigned long offset;
+
+ if (!ocm_reg->virt)
+ return 0;
+
+ list_for_each_entry_safe(blk, tmp, &ocm_reg->list, list) {
+ if (blk->addr == addr) {
+ offset = addr - ocm_reg->virt;
+ ocm_reg->memfree += blk->size;
+ rh_free(ocm_reg->rh, offset);
+ list_del(&blk->list);
+ kfree(blk);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+void ocm_free(const void *addr)
+{
+ int i;
+
+ if (!addr)
+ return;
+
+ for (i = 0; i < ocm_count; i++) {
+ struct ocm_info *ocm = ocm_get_node(i);
+
+ if (!ocm || !ocm->ready)
+ continue;
+
+ if (ocm_free_region(&ocm->nc, addr) ||
+ ocm_free_region(&ocm->c, addr))
+ return;
+ }
+}
+
+static void __init ocm_init_node(int count, struct device_node *node)
+{
+ struct ocm_info *ocm;
+
+ const unsigned int *cell_index;
+ const unsigned int *cache_size;
+ int len;
+
+ struct resource rsrc;
+ int ioflags;
+
+ ocm = ocm_get_node(count);
+
+ cell_index = of_get_property(node, "cell-index", &len);
+ if (!cell_index) {
+ printk(KERN_ERR "OCM: missing cell-index property");
+ return;
+ }
+ ocm->index = *cell_index;
+
+ if (of_device_is_available(node))
+ ocm->status = OCM_ENABLED;
+
+ cache_size = of_get_property(node, "cached-region-size", &len);
+ if (cache_size)
+ ocm->cache_size = *cache_size;
+
+ if (of_address_to_resource(node, 0, &rsrc)) {
+ printk(KERN_ERR "OCM%d: could not get resource address\n",
+ ocm->index);
+ return;
+ }
+
+ ocm->phys = rsrc.start;
+ ocm->memtotal = (rsrc.end - rsrc.start + 1);
+
+ printk(KERN_INFO "OCM%d: %d Bytes (%s)\n",
+ ocm->index, ocm->memtotal,
+ (ocm->status == OCM_DISABLED) ? "disabled" : "enabled");
+
+ if (ocm->status == OCM_DISABLED)
+ return;
+
+ /* request region */
+
+ if (!request_mem_region(ocm->phys, ocm->memtotal, "ppc4xx_ocm")) {
+ printk(KERN_ERR "OCM%d: could not request region\n",
+ ocm->index);
+ return;
+ }
+
+ /* Configure non-cached and cached regions */
+
+ ocm->nc.phys = ocm->phys;
+ ocm->nc.memtotal = ocm->memtotal - ocm->cache_size;
+ ocm->nc.memfree = ocm->nc.memtotal;
+
+ ocm->c.phys = ocm->phys + ocm->nc.memtotal;
+ ocm->c.memtotal = ocm->cache_size;
+ ocm->c.memfree = ocm->c.memtotal;
+
+ if (ocm->nc.memtotal == 0)
+ ocm->nc.phys = 0;
+
+ if (ocm->c.memtotal == 0)
+ ocm->c.phys = 0;
+
+ printk(KERN_INFO "OCM%d: %d Bytes (non-cached)\n",
+ ocm->index, ocm->nc.memtotal);
+
+ printk(KERN_INFO "OCM%d: %d Bytes (cached)\n",
+ ocm->index, ocm->c.memtotal);
+
+ /* ioremap the non-cached region */
+ if (ocm->nc.memtotal) {
+ ioflags = _PAGE_NO_CACHE | _PAGE_GUARDED | _PAGE_EXEC;
+ ocm->nc.virt = __ioremap(ocm->nc.phys, ocm->nc.memtotal,
+ ioflags);
+
+ if (!ocm->nc.virt) {
+ printk(KERN_ERR
+ "OCM%d: failed to ioremap non-cached memory\n",
+ ocm->index);
+ ocm->nc.memfree = 0;
+ return;
+ }
+ }
+
+ /* ioremap the cached region */
+
+ if (ocm->c.memtotal) {
+ ioflags = _PAGE_EXEC;
+ ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
+ ioflags);
+
+ if (!ocm->c.virt) {
+ printk(KERN_ERR
+ "OCM%d: failed to ioremap cached memory\n",
+ ocm->index);
+ ocm->c.memfree = 0;
+ return;
+ }
+ }
+
+ /* Create Remote Heaps */
+
+ ocm->alignment = 4; /* default 4 byte alignment */
+
+ if (ocm->nc.virt) {
+ ocm->nc.rh = rh_create(ocm->alignment);
+ rh_attach_region(ocm->nc.rh, 0, ocm->nc.memtotal);
+ }
+
+ if (ocm->c.virt) {
+ ocm->c.rh = rh_create(ocm->alignment);
+ rh_attach_region(ocm->c.rh, 0, ocm->c.memtotal);
+ }
+
+ INIT_LIST_HEAD(&ocm->nc.list);
+ INIT_LIST_HEAD(&ocm->c.list);
+
+ ocm->ready = 1;
+
+ return;
+}
+
+static ssize_t ocm_sysfs_show(struct class *class,
+ struct class_attribute *attr, char *buf)
+{
+ struct ocm_block *blk, *tmp;
+ unsigned int count, i;
+
+ count = 0;
+ for (i = 0; i < ocm_count; i++) {
+ struct ocm_info *ocm = ocm_get_node(i);
+
+ if (!ocm || !ocm->ready)
+ continue;
+
+ count += sprintf(buf + count, "OCM : %d\n",
+ ocm->index);
+ count += sprintf(buf + count, "PhysAddr : 0x%llx\n",
+ ocm->phys);
+ count += sprintf(buf + count, "MemTotal : %d Bytes\n",
+ ocm->memtotal);
+ count += sprintf(buf + count, "MemTotal(NC) : %d Bytes\n",
+ ocm->nc.memtotal);
+ count += sprintf(buf + count, "MemTotal(C) : %d Bytes\n",
+ ocm->c.memtotal);
+
+ count += sprintf(buf + count, "\n");
+
+ count += sprintf(buf + count, "NC.PhysAddr : 0x%llx\n",
+ ocm->nc.phys);
+ count += sprintf(buf + count, "NC.VirtAddr : 0x%p\n",
+ ocm->nc.virt);
+ count += sprintf(buf + count, "NC.MemTotal : %d Bytes\n",
+ ocm->nc.memtotal);
+ count += sprintf(buf + count, "NC.MemFree : %d Bytes\n",
+ ocm->nc.memfree);
+
+ list_for_each_entry_safe(blk, tmp, &ocm->nc.list, list) {
+ count += sprintf(buf + count, "NC.MemUsed : %d Bytes (%s)\n",
+ blk->size, blk->owner);
+ }
+
+ count += sprintf(buf + count, "\n");
+
+ count += sprintf(buf + count, "C.PhysAddr : 0x%llx\n",
+ ocm->c.phys);
+ count += sprintf(buf + count, "C.VirtAddr : 0x%p\n",
+ ocm->c.virt);
+ count += sprintf(buf + count, "C.MemTotal : %d Bytes\n",
+ ocm->c.memtotal);
+ count += sprintf(buf + count, "C.MemFree : %d Bytes\n",
+ ocm->c.memfree);
+
+ list_for_each_entry_safe(blk, tmp, &ocm->c.list, list) {
+ count += sprintf(buf + count, "C.MemUsed : %d Bytes (%s)\n",
+ blk->size, blk->owner);
+ }
+
+ count += sprintf(buf + count, "\n");
+ }
+
+ return count;
+}
+
+static struct class *mem_class;
+
+static CLASS_ATTR(ocminfo, S_IRUGO, ocm_sysfs_show, NULL);
+
+static int ocm_sysfs_init(void)
+{
+ mem_class = class_create(THIS_MODULE, "ocm");
+ if (IS_ERR(mem_class))
+ return PTR_ERR(mem_class);
+
+ return class_create_file(mem_class, &class_attr_ocminfo);
+}
+
+static int __init ocm_init(void)
+{
+ struct device_node *np;
+ int count;
+
+ count = 0;
+ for_each_compatible_node(np, NULL, "ibm,ocm")
+ count++;
+
+ if (!count)
+ return 0;
+
+ ocm_nodes = kzalloc((count * sizeof(struct ocm_info)), GFP_KERNEL);
+ if (!ocm_nodes) {
+ printk(KERN_ERR "OCM: failed to allocate OCM nodes!\n");
+ return -ENOMEM;
+ }
+
+ ocm_count = count;
+ count = 0;
+
+ for_each_compatible_node(np, NULL, "ibm,ocm") {
+ ocm_init_node(count, np);
+ count++;
+ }
+
+ ocm_sysfs_init();
+
+ return 0;
+}
+
+arch_initcall(ocm_init);
--
1.7.2.5
^ permalink raw reply related
* Re: kilauea compilation breaks with v3.3 kernel and ELDK 4.2
From: Robert Berger @ 2012-05-06 14:37 UTC (permalink / raw)
To: Tony Breeds
Cc: linuxppc-dev, Wolfgang Denk, "Frank E. Svendsbøe"
In-Reply-To: <20120402062826.GC2129@thor.bakeyournoodle.com>
On 04/02/2012 09:28 AM, Tony Breeds wrote:
> On Mon, Apr 02, 2012 at 12:01:55PM +1000, Benjamin Herrenschmidt wrote:
>
>> Ok, I've asked Tony to have a look at splitting the build decision
>> in arch/powerpc/boot along the same lines as the CPU families... ie only
>> wrappers for platforms potentially supported by the built kernel. That
>> should fix it.
>
> Please try this patch. Only lightly tested here. I haven't "split up"
> src-wlib yet as I wanted to verify I'm on the right track.
I can confirm that it fixes the kilauea problem. Builds and runs so far.
Are you going to push this upstream?
Regards,
Robert
..."Software is like sex, it's better when it's free"
My public pgp key is available at:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x90320BF1
^ permalink raw reply
* [PATCH] cpu: Document clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-05-05 1:47 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: linaro-kernel, Mike Frysinger, user-mode-linux-devel, linux-sh,
Richard Weinberger, linuxppc-dev, Oleg Nesterov, linux-kernel,
linux-mm, Paul Mundt, John Stultz, patches, KOSAKI Motohiro,
Russell King, uclinux-dist-devel, linux-arm-kernel
In-Reply-To: <1335869133.13683.125.camel@twins>
This patch adds more comments on clear_tasks_mm_cpumask, plus adds
a runtime check: the function is only suitable for offlined CPUs,
and if called inappropriately, the kernel should scream aloud.
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
On Tue, May 01, 2012 at 12:45:33PM +0200, Peter Zijlstra wrote:
> On Thu, 2012-04-26 at 16:59 -0700, Andrew Morton wrote:
> > > +void clear_tasks_mm_cpumask(int cpu)
> >
> > The operation of this function was presumably obvious to you at the
> > time you wrote it, but that isn't true of other people at later times.
> >
> > Please document it?
[...]
> > If someone tries to use this function for a different purpose, or
> > copies-and-modifies it for a different purpose, we just shot them in
> > the foot.
> >
> > They'd be pretty dumb to do that without reading the local comment,
> > but still...
>
> Methinks something simple like:
>
> WARN_ON(cpu_online(cpu));
>
> Ought to cure that worry, no? :-)
Yeah, this is all good ideas, thanks.
How about the following patch?
kernel/cpu.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index ecdf499..1bfa26f 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -13,6 +13,7 @@
#include <linux/oom.h>
#include <linux/rcupdate.h>
#include <linux/export.h>
+#include <linux/bug.h>
#include <linux/kthread.h>
#include <linux/stop_machine.h>
#include <linux/mutex.h>
@@ -173,6 +174,18 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_cpu_notifier);
+/**
+ * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
+ * @cpu: a CPU id
+ *
+ * This function walks up all processes, finds a valid mm struct for
+ * each one and then clears a corresponding bit in mm's cpumask. While
+ * this all sounds trivial, there are various non-obvious corner cases,
+ * which this function tries to solve in a safe manner.
+ *
+ * Also note that the function uses a somewhat relaxed locking scheme,
+ * so it may be called only for an already offlined CPU.
+ */
void clear_tasks_mm_cpumask(int cpu)
{
struct task_struct *p;
@@ -184,10 +197,15 @@ void clear_tasks_mm_cpumask(int cpu)
* Thus, we may use rcu_read_lock() here, instead of grabbing
* full-fledged tasklist_lock.
*/
+ WARN_ON(cpu_online(cpu));
rcu_read_lock();
for_each_process(p) {
struct task_struct *t;
+ /*
+ * Main thread might exit, but other threads may still have
+ * a valid mm. Find one.
+ */
t = find_lock_task_mm(p);
if (!t)
continue;
--
1.7.9.2
^ permalink raw reply related
* Re: [PATCH 1/9] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Anton Vorontsov @ 2012-05-05 1:47 UTC (permalink / raw)
To: Andrew Morton
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, linuxppc-dev,
Oleg Nesterov, linux-kernel, linux-mm, Paul Mundt, John Stultz,
patches, KOSAKI Motohiro, Russell King, uclinux-dist-devel,
linux-arm-kernel
In-Reply-To: <20120426165911.00cebd31.akpm@linux-foundation.org>
On Thu, Apr 26, 2012 at 04:59:11PM -0700, Andrew Morton wrote:
[...]
> > so its not like new tasks will ever get this cpu set in
> > + * their mm mask. -- Peter Zijlstra
> > + * Thus, we may use rcu_read_lock() here, instead of grabbing
> > + * full-fledged tasklist_lock.
> > + */
> > + rcu_read_lock();
> > + for_each_process(p) {
> > + struct task_struct *t;
> > +
> > + t = find_lock_task_mm(p);
> > + if (!t)
> > + continue;
> > + cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
> > + task_unlock(t);
> > + }
> > + rcu_read_unlock();
> > +}
>
> It is good that this code exists under CONFIG_HOTPLUG_CPU. Did you
> check that everything works correctly with CONFIG_HOTPLUG_CPU=n?
Yeah, only the code under CONFIG_HOTPLUG_CPU calls the function, so
it should be all fine.
Thanks!
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers, second version
From: Mauro Carvalho Chehab @ 2012-05-04 10:48 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Egor Martovetsky, Niklas Söderlund, Tim Small, Arvind R.,
Chris Metcalf, Olof Johansson, Doug Thompson,
Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <20120504101636.GB18459@aftab.osrc.amd.com>
Em 04-05-2012 07:16, Borislav Petkov escreveu:
> On Thu, May 03, 2012 at 11:16:54AM -0300, Mauro Carvalho Chehab wrote:
>> edac: Change internal representation to work with layers
...
>> /**
>> - * edac_mc_alloc: Allocate a struct mem_ctl_info structure
>> - * @size_pvt: size of private storage needed
>> - * @nr_csrows: Number of CWROWS needed for this MC
>> - * @nr_chans: Number of channels for the MC
>> + * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
>> + * @mc_num: Memory controller number
>> + * @n_layers: Number of MC hierarchy layers
>> + * layers: Describes each layer as seen by the Memory Controller
>> + * @size_pvt: size of private storage needed
>> + *
>> + *
>> + * Non-csrow based drivers (like FB-DIMM and RAMBUS ones) will likely report
>> + * such DIMMS properly, but the CSROWS-based ones will likely do the wrong
>
> DIMMs csrow-based
>
>> + * thing, as two chip select values are used for dual-rank memories (and 4, for
>> + * quad-rank ones). I suspect that this issue could be solved inside the EDAC
>> + * core for SDRAM memories, but it requires further study at JEDEC JESD 21C.
>
> The paragraph above is still in, let me repeat my last note:
>
> "This last paragraph sounds innacurately, especially the "likely"
> adverbs make it even more confusing. The gist of what you're saying is
> already present in the commit message anyway, so drop it here pls."
There are two similar comments one for edac_mc_alloc and another for new_edac_mc_alloc.
It seems that I fixed just one of them.
>
>> + *
>> + * In summary, solving this issue is not easy, as it requires a lot of testing.
>
> As before:
>
> "Also superfluous and has nothing to do with edac_mc_alloc()."
>
> Pls remove it.
Dropped both paragraphs.
>
>> * Everything is kmalloc'ed as one big chunk - more efficient.
>> * Only can be used if all structures have the same lifetime - otherwise
>> @@ -168,22 +196,49 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
>> *
>> * Use edac_mc_free() to free mc structures allocated by this function.
>> *
>> + * NOTE: drivers handle multi-rank memories in different ways: in some
>> + * drivers, one multi-rank memory stick is mapped as one entry, while, in
>> + * others, a single multi-rank memory stick would be mapped into several
>> + * entries. Currently, this function will allocate multiple struct dimm_info
>> + * on such scenarios, as grouping the multiple ranks require drivers change.
>> + *
>> * Returns:
>> * NULL allocation failed
>> * struct mem_ctl_info pointer
>
> Ok, this patch still doesn't contain all the changes I requested for
> although you said you did them. Is this not the latest version? I'll
> wait for you to sort it out before I continue reviewing...
>
> Thanks.
>
Patch enclosed (and updated at Infradead).
Regards,
Mauro
-
edac: Change internal representation to work with layers
Change the EDAC internal representation to work with non-csrow
based memory controllers.
There are lots of those memory controllers nowadays, and more
are coming. So, the EDAC internal representation needs to be
changed, in order to work with those memory controllers, while
preserving backward compatibility with the old ones.
The edac core was written with the idea that memory controllers
are able to directly access csrows.
This is not true for FB-DIMM and RAMBUS memory controllers.
Also, some recent advanced memory controllers don't present a per-csrows
view. Instead, they view memories as DIMMs, instead of ranks.
So, change the allocation and error report routines to allow
them to work with all types of architectures.
This will allow the removal of several hacks with FB-DIMM and RAMBUS
memory controllers.
Also, several tests were done on different platforms using different
x86 drivers.
TODO: a multi-rank DIMMs are currently represented by multiple DIMM
entries in struct dimm_info. That means that changing a label for one
rank won't change the same label for the other ranks at the same DIMM.
This bug is present since the beginning of the EDAC, so it is not a big
deal. However, on several drivers, it is possible to fix this issue, but
it should be a per-driver fix, as the csrow => DIMM arrangement may not
be equal for all. So, don't try to fix it here yet.
I tried to make this patch as short as possible, preceding it with
several other patches that simplified the logic here. Yet, as the
internal API changes, all drivers need changes. The changes are
generally bigger in the drivers for FB-DIMMs.
Cc: Aristeu Rozanski <arozansk@redhat.com>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Borislav Petkov <borislav.petkov@amd.com>
Cc: Mark Gross <mark.gross@intel.com>
Cc: Jason Uhlenkott <juhlenko@akamai.com>
Cc: Tim Small <tim@buttersideup.com>
Cc: Ranganathan Desikan <ravi@jetztechnologies.com>
Cc: "Arvind R." <arvino55@gmail.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Egor Martovetsky <egor@pasemi.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joe Perches <joe@perches.com>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Hitoshi Mitake <h.mitake@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Niklas Söderlund" <niklas.soderlund@ericsson.com>
Cc: Shaohui Xie <Shaohui.Xie@freescale.com>
Cc: Josh Boyer <jwboyer@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
v19: dropped 2 comment paragraphs, as above.
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index e48ab31..1286c5e 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -447,8 +447,12 @@ static inline void pci_write_bits32(struct pci_dev *pdev, int offset,
#endif /* CONFIG_PCI */
-extern struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index);
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int edac_index);
+struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt);
extern int edac_mc_add_mc(struct mem_ctl_info *mci);
extern void edac_mc_free(struct mem_ctl_info *mci);
extern struct mem_ctl_info *edac_mc_find(int idx);
@@ -467,24 +471,78 @@ extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
* reporting logic and function interface - reduces conditional
* statement clutter and extra function arguments.
*/
-extern void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page,
- unsigned long syndrome, int row, int channel,
- const char *msg);
-extern void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row,
- const char *msg);
-extern void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel0, unsigned int channel1,
- char *msg);
-extern void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel, char *msg);
+
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog);
+
+static inline void edac_mc_handle_ce(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page,
+ unsigned long syndrome, int row, int channel,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ page_frame_number, offset_in_page, syndrome,
+ row, channel, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page, int row,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ page_frame_number, offset_in_page, 0,
+ row, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel0,
+ unsigned int channel1,
+ char *msg)
+{
+ /*
+ *FIXME: The error can also be at channel1 (e. g. at the second
+ * channel of the same branch). The fix is to push
+ * edac_mc_handle_error() call into each driver
+ */
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel0, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel, char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel, -1, msg, NULL, NULL);
+}
/*
* edac_device APIs
@@ -496,6 +554,7 @@ extern void edac_device_handle_ue(struct edac_device_ctl_info *edac_dev,
extern void edac_device_handle_ce(struct edac_device_ctl_info *edac_dev,
int inst_nr, int block_nr, const char *msg);
extern int edac_device_alloc_index(void);
+extern const char *edac_layer_name[];
/*
* edac_pci APIs
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6ec967a..4365079 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -44,9 +44,25 @@ static void edac_mc_dump_channel(struct rank_info *chan)
debugf4("\tchannel = %p\n", chan);
debugf4("\tchannel->chan_idx = %d\n", chan->chan_idx);
debugf4("\tchannel->csrow = %p\n\n", chan->csrow);
- debugf4("\tdimm->ce_count = %d\n", chan->dimm->ce_count);
- debugf4("\tdimm->label = '%s'\n", chan->dimm->label);
- debugf4("\tdimm->nr_pages = 0x%x\n", chan->dimm->nr_pages);
+ debugf4("\tchannel->dimm = %p\n", chan->dimm);
+}
+
+static void edac_mc_dump_dimm(struct dimm_info *dimm)
+{
+ int i;
+
+ debugf4("\tdimm = %p\n", dimm);
+ debugf4("\tdimm->label = '%s'\n", dimm->label);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
+ debugf4("\tdimm location ");
+ for (i = 0; i < dimm->mci->n_layers; i++) {
+ printk(KERN_CONT "%d", dimm->location[i]);
+ if (i < dimm->mci->n_layers - 1)
+ printk(KERN_CONT ".");
+ }
+ printk(KERN_CONT "\n");
+ debugf4("\tdimm->grain = %d\n", dimm->grain);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
}
static void edac_mc_dump_csrow(struct csrow_info *csrow)
@@ -70,6 +86,8 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci)
debugf4("\tmci->edac_check = %p\n", mci->edac_check);
debugf3("\tmci->nr_csrows = %d, csrows = %p\n",
mci->nr_csrows, mci->csrows);
+ debugf3("\tmci->nr_dimms = %d, dimms = %p\n",
+ mci->tot_dimms, mci->dimms);
debugf3("\tdev = %p\n", mci->dev);
debugf3("\tmod_name:ctl_name = %s:%s\n", mci->mod_name, mci->ctl_name);
debugf3("\tpvt_info = %p\n\n", mci->pvt_info);
@@ -157,10 +175,12 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
}
/**
- * edac_mc_alloc: Allocate a struct mem_ctl_info structure
- * @size_pvt: size of private storage needed
- * @nr_csrows: Number of CWROWS needed for this MC
- * @nr_chans: Number of channels for the MC
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of MC hierarchy layers
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: size of private storage needed
+ *
*
* Everything is kmalloc'ed as one big chunk - more efficient.
* Only can be used if all structures have the same lifetime - otherwise
@@ -168,22 +188,49 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
*
* Use edac_mc_free() to free mc structures allocated by this function.
*
+ * NOTE: drivers handle multi-rank memories in different ways: in some
+ * drivers, one multi-rank memory stick is mapped as one entry, while, in
+ * others, a single multi-rank memory stick would be mapped into several
+ * entries. Currently, this function will allocate multiple struct dimm_info
+ * on such scenarios, as grouping the multiple ranks require drivers change.
+ *
* Returns:
* NULL allocation failed
* struct mem_ctl_info pointer
*/
-struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index)
+struct mem_ctl_info *new_edac_mc_alloc(unsigned mc_num,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt)
{
- void *ptr = NULL;
struct mem_ctl_info *mci;
- struct csrow_info *csi, *csrow;
+ struct edac_mc_layer *layer;
+ struct csrow_info *csi, *csr;
struct rank_info *chi, *chp, *chan;
struct dimm_info *dimm;
- void *pvt;
- unsigned size;
- int row, chn;
- int err;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+ unsigned pos[EDAC_MAX_LAYERS];
+ void *pvt, *ptr = NULL;
+ unsigned size, tot_dimms = 1, count = 1;
+ unsigned tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
+ int i, j, err, row, chn;
+ bool per_rank = false;
+
+ BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
+ /*
+ * Calculate the total amount of dimms and csrows/cschannels while
+ * in the old API emulation mode
+ */
+ for (i = 0; i < n_layers; i++) {
+ tot_dimms *= layers[i].size;
+ if (layers[i].is_virt_csrow)
+ tot_csrows *= layers[i].size;
+ else
+ tot_channels *= layers[i].size;
+
+ if (layers[i].type == EDAC_MC_LAYER_CHIP_SELECT)
+ per_rank = true;
+ }
/* Figure out the offsets of the various items from the start of an mc
* structure. We want the alignment of each item to be at least as
@@ -191,12 +238,27 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
* hardcode everything into a single struct.
*/
mci = edac_align_ptr(&ptr, sizeof(*mci), 1);
- csi = edac_align_ptr(&ptr, sizeof(*csi), nr_csrows);
- chi = edac_align_ptr(&ptr, sizeof(*chi), nr_csrows * nr_chans);
- dimm = edac_align_ptr(&ptr, sizeof(*dimm), nr_csrows * nr_chans);
+ layer = edac_align_ptr(&ptr, sizeof(*layer), n_layers);
+ csi = edac_align_ptr(&ptr, sizeof(*csi), tot_csrows);
+ chi = edac_align_ptr(&ptr, sizeof(*chi), tot_csrows * tot_channels);
+ dimm = edac_align_ptr(&ptr, sizeof(*dimm), tot_dimms);
+ for (i = 0; i < n_layers; i++) {
+ count *= layers[i].size;
+ debugf4("%s: errcount layer %d size %d\n", __func__, i, count);
+ ce_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ ue_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ tot_errcount += 2 * count;
+ }
+
+ debugf4("%s: allocating %d error counters\n", __func__, tot_errcount);
pvt = edac_align_ptr(&ptr, sz_pvt, 1);
size = ((unsigned long)pvt) + sz_pvt;
+ debugf1("%s(): allocating %u bytes for mci data (%d %s, %d csrows/channels)\n",
+ __func__, size,
+ tot_dimms,
+ per_rank ? "ranks" : "dimms",
+ tot_csrows * tot_channels);
mci = kzalloc(size, GFP_KERNEL);
if (mci == NULL)
return NULL;
@@ -204,42 +266,90 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
/* Adjust pointers so they point within the memory we just allocated
* rather than an imaginary chunk of memory located at address 0.
*/
+ layer = (struct edac_mc_layer *)(((char *)mci) + ((unsigned long)layer));
csi = (struct csrow_info *)(((char *)mci) + ((unsigned long)csi));
chi = (struct rank_info *)(((char *)mci) + ((unsigned long)chi));
dimm = (struct dimm_info *)(((char *)mci) + ((unsigned long)dimm));
+ for (i = 0; i < n_layers; i++) {
+ mci->ce_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ce_per_layer[i]));
+ mci->ue_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ue_per_layer[i]));
+ }
pvt = sz_pvt ? (((char *)mci) + ((unsigned long)pvt)) : NULL;
/* setup index and various internal pointers */
- mci->mc_idx = edac_index;
+ mci->mc_idx = mc_num;
mci->csrows = csi;
mci->dimms = dimm;
+ mci->tot_dimms = tot_dimms;
mci->pvt_info = pvt;
- mci->nr_csrows = nr_csrows;
+ mci->n_layers = n_layers;
+ mci->layers = layer;
+ memcpy(mci->layers, layers, sizeof(*layer) * n_layers);
+ mci->nr_csrows = tot_csrows;
+ mci->num_cschannel = tot_channels;
+ mci->mem_is_per_rank = per_rank;
/*
- * For now, assumes that a per-csrow arrangement for dimms.
- * This will be latter changed.
+ * Fill the csrow struct
*/
- dimm = mci->dimms;
-
- for (row = 0; row < nr_csrows; row++) {
- csrow = &csi[row];
- csrow->csrow_idx = row;
- csrow->mci = mci;
- csrow->nr_channels = nr_chans;
- chp = &chi[row * nr_chans];
- csrow->channels = chp;
-
- for (chn = 0; chn < nr_chans; chn++) {
+ for (row = 0; row < tot_csrows; row++) {
+ csr = &csi[row];
+ csr->csrow_idx = row;
+ csr->mci = mci;
+ csr->nr_channels = tot_channels;
+ chp = &chi[row * tot_channels];
+ csr->channels = chp;
+
+ for (chn = 0; chn < tot_channels; chn++) {
chan = &chp[chn];
chan->chan_idx = chn;
- chan->csrow = csrow;
+ chan->csrow = csr;
+ }
+ }
- mci->csrows[row].channels[chn].dimm = dimm;
- dimm->csrow = row;
- dimm->csrow_channel = chn;
- dimm++;
- mci->nr_dimms++;
+ /*
+ * Fill the dimm struct
+ */
+ memset(&pos, 0, sizeof(pos));
+ row = 0;
+ chn = 0;
+ debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
+ per_rank ? "ranks" : "dimms");
+ for (i = 0; i < tot_dimms; i++) {
+ chan = &csi[row].channels[chn];
+ dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
+ pos[0], pos[1], pos[2]);
+ dimm->mci = mci;
+
+ debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
+ i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
+ pos[0], pos[1], pos[2], row, chn);
+
+ /* Copy DIMM location */
+ for (j = 0; j < n_layers; j++)
+ dimm->location[j] = pos[j];
+
+ /* Link it to the csrows old API data */
+ chan->dimm = dimm;
+ dimm->csrow = row;
+ dimm->cschannel = chn;
+
+ /* Increment csrow location */
+ for (j = n_layers - 1; j >= 0; j--)
+ if (layers[j].is_virt_csrow)
+ break;
+ row++;
+ if (row == tot_csrows) {
+ row = 0;
+ chn++;
+ }
+
+ /* Increment dimm location */
+ for (j = n_layers - 1; j >= 0; j--) {
+ pos[j]++;
+ if (pos[j] < layers[j].size)
+ break;
+ pos[j] = 0;
}
}
@@ -263,6 +373,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
*/
return mci;
}
+EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
+
+/**
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of layers at the MC hierarchy
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: Size of private storage needed
+ *
+ *
+ * FIXME: drivers handle multi-rank memories in different ways: some
+ * drivers map multi-ranked DIMMs as one DIMM while others
+ * as several DIMMs.
+ *
+ * Everything is kmalloc'ed as one big chunk - more efficient.
+ * It can only be used if all structures have the same lifetime - otherwise
+ * you have to allocate and initialize your own structures.
+ *
+ * Use edac_mc_free() to free mc structures allocated by this function.
+ *
+ * Returns:
+ * On failure: NULL
+ * On success: struct mem_ctl_info pointer
+ */
+
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int mc_num)
+{
+ unsigned n_layers = 2;
+ struct edac_mc_layer layers[n_layers];
+
+ layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+ layers[0].size = nr_csrows;
+ layers[0].is_virt_csrow = true;
+ layers[1].type = EDAC_MC_LAYER_CHANNEL;
+ layers[1].size = nr_chans;
+ layers[1].is_virt_csrow = false;
+
+ return new_edac_mc_alloc(mc_num, ARRAY_SIZE(layers), layers, sz_pvt);
+}
EXPORT_SYMBOL_GPL(edac_mc_alloc);
/**
@@ -528,7 +678,6 @@ EXPORT_SYMBOL(edac_mc_find);
* edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
* create sysfs entries associated with mci structure
* @mci: pointer to the mci structure to be added to the list
- * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
*
* Return:
* 0 Success
@@ -555,6 +704,8 @@ int edac_mc_add_mc(struct mem_ctl_info *mci)
edac_mc_dump_channel(&mci->csrows[i].
channels[j]);
}
+ for (i = 0; i < mci->tot_dimms; i++)
+ edac_mc_dump_dimm(&mci->dimms[i]);
}
#endif
mutex_lock(&mem_ctls_mutex);
@@ -712,261 +863,289 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
}
EXPORT_SYMBOL_GPL(edac_mc_find_csrow_by_page);
-/* FIXME - setable log (warning/emerg) levels */
-/* FIXME - integrate with evlog: http://evlog.sourceforge.net/ */
-void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, unsigned long syndrome,
- int row, int channel, const char *msg)
+const char *edac_layer_name[] = {
+ [EDAC_MC_LAYER_BRANCH] = "branch",
+ [EDAC_MC_LAYER_CHANNEL] = "channel",
+ [EDAC_MC_LAYER_SLOT] = "slot",
+ [EDAC_MC_LAYER_CHIP_SELECT] = "csrow",
+};
+EXPORT_SYMBOL_GPL(edac_layer_name);
+
+static void edac_inc_ce_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
{
- unsigned long remapped_page;
- char *label = NULL;
- u32 grain;
+ int i, index = 0;
- debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
+ mci->ce_count++;
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- if (channel >= mci->csrows[row].nr_channels || channel < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range "
- "(%d >= %d)\n", channel,
- mci->csrows[row].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ce_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_inc_ue_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
+{
+ int i, index = 0;
+
+ mci->ue_count++;
+
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- label = mci->csrows[row].channels[channel].dimm->label;
- grain = mci->csrows[row].channels[channel].dimm->grain;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ue_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_ce_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ u32 grain)
+{
+ unsigned long remapped_page;
if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
edac_mc_printk(mci, KERN_WARNING,
- "CE page 0x%lx, offset 0x%lx, grain %d, syndrome "
- "0x%lx, row %d, channel %d, label \"%s\": %s\n",
- page_frame_number, offset_in_page,
- grain, syndrome, row, channel,
- label, msg);
-
- mci->ce_count++;
- mci->csrows[row].ce_count++;
- mci->csrows[row].channels[channel].dimm->ce_count++;
- mci->csrows[row].channels[channel].ce_count++;
+ "CE %s on %s (%s%s %s)\n",
+ msg, label, location,
+ detail, other_detail);
+ edac_inc_ce_error(mci, enable_per_layer_report, pos);
if (mci->scrub_mode & SCRUB_SW_SRC) {
/*
- * Some MC's can remap memory so that it is still available
- * at a different address when PCI devices map into memory.
- * MC's that can't do this lose the memory where PCI devices
- * are mapped. This mapping is MC dependent and so we call
- * back into the MC driver for it to map the MC page to
- * a physical (CPU) page which can then be mapped to a virtual
- * page - which can then be scrubbed.
- */
+ * Some memory controllers (called MCs below) can remap
+ * memory so that it is still available at a different
+ * address when PCI devices map into memory.
+ * MC's that can't do this, lose the memory where PCI
+ * devices are mapped. This mapping is MC-dependent
+ * and so we call back into the MC driver for it to
+ * map the MC page to a physical (CPU) page which can
+ * then be mapped to a virtual page - which can then
+ * be scrubbed.
+ */
remapped_page = mci->ctl_page_to_phys ?
mci->ctl_page_to_phys(mci, page_frame_number) :
page_frame_number;
- edac_mc_scrub_block(remapped_page, offset_in_page, grain);
+ edac_mc_scrub_block(remapped_page,
+ offset_in_page, grain);
}
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce);
-void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
+static void edac_ue_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report)
{
- if (edac_mc_get_log_ce())
+ if (edac_mc_get_log_ue())
edac_mc_printk(mci, KERN_WARNING,
- "CE - no information available: %s\n", msg);
+ "UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
- mci->ce_noinfo_count++;
- mci->ce_count++;
+ if (edac_mc_get_panic_on_ue())
+ panic("UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
+
+ edac_inc_ue_error(mci, enable_per_layer_report, pos);
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce_no_info);
-void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row, const char *msg)
+#define OTHER_LABEL " or "
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog)
{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chan;
- int chars;
- char *label = NULL;
+ /* FIXME: too much for stack: move it to some pre-alocated area */
+ char detail[80], location[80];
+ char label[(EDAC_MC_LABEL_LEN + 1 + sizeof(OTHER_LABEL)) * mci->tot_dimms];
+ char *p;
+ int row = -1, chan = -1;
+ int pos[EDAC_MAX_LAYERS] = { layer0, layer1, layer2 };
+ int i;
u32 grain;
+ bool enable_per_layer_report = false;
debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
-
- grain = mci->csrows[row].channels[0].dimm->grain;
- label = mci->csrows[row].channels[0].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- for (chan = 1; (chan < mci->csrows[row].nr_channels) && (len > 0);
- chan++) {
- label = mci->csrows[row].channels[chan].dimm->label;
- chars = snprintf(pos, len + 1, ":%s", label);
- len -= chars;
- pos += chars;
+ /*
+ * Check if the event report is consistent and if the memory
+ * location is known. If it is known, enable_per_layer_report will be
+ * true, the DIMM(s) label info will be filled and the per-layer
+ * error counters will be incremented.
+ */
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] >= (int)mci->layers[i].size) {
+ if (type == HW_EVENT_ERR_CORRECTED)
+ p = "CE";
+ else
+ p = "UE";
+
+ edac_mc_printk(mci, KERN_ERR,
+ "INTERNAL ERROR: %s value is out of range (%d >= %d)\n",
+ edac_layer_name[mci->layers[i].type],
+ pos[i], mci->layers[i].size);
+ /*
+ * Instead of just returning it, let's use what's
+ * known about the error. The increment routines and
+ * the DIMM filter logic will do the right thing by
+ * pointing the likely damaged DIMMs.
+ */
+ pos[i] = -1;
+ }
+ if (pos[i] >= 0)
+ enable_per_layer_report = true;
}
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE page 0x%lx, offset 0x%lx, grain %d, row %d, "
- "labels \"%s\": %s\n", page_frame_number,
- offset_in_page, grain, row, labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: UE page 0x%lx, offset 0x%lx, grain %d, "
- "row %d, labels \"%s\": %s\n", mci->mc_idx,
- page_frame_number, offset_in_page,
- grain, row, labels, msg);
-
- mci->ue_count++;
- mci->csrows[row].ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue);
+ /*
+ * Get the dimm label/grain that applies to the match criteria.
+ * As the error algorithm may not be able to point to just one memory
+ * stick, the logic here will get all possible labels that could
+ * pottentially be affected by the error.
+ * On FB-DIMM memory controllers, for uncorrected errors, it is common
+ * to have only the MC channel and the MC dimm (also called "branch")
+ * but the channel is not known, as the memory is arranged in pairs,
+ * where each memory belongs to a separate channel within the same
+ * branch.
+ */
+ grain = 0;
+ p = label;
+ *p = '\0';
+ for (i = 0; i < mci->tot_dimms; i++) {
+ struct dimm_info *dimm = &mci->dimms[i];
-void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
-{
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: Uncorrected Error", mci->mc_idx);
+ if (layer0 >= 0 && layer0 != dimm->location[0])
+ continue;
+ if (layer1 >= 0 && layer1 != dimm->location[1])
+ continue;
+ if (layer2 >= 0 && layer2 != dimm->location[2])
+ continue;
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_WARNING,
- "UE - no information available: %s\n", msg);
- mci->ue_noinfo_count++;
- mci->ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue_no_info);
+ /* get the max grain, over the error match range */
+ if (dimm->grain > grain)
+ grain = dimm->grain;
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process UE events
- */
-void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
- unsigned int csrow,
- unsigned int channela,
- unsigned int channelb, char *msg)
-{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chars;
- char *label;
-
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
-
- if (channela >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-a out of range "
- "(%d >= %d)\n",
- channela, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ /*
+ * If the error is memory-controller wide, there's no need to
+ * seek for the affected DIMMs because the whole
+ * channel/memory controller/... may be affected.
+ * Also, don't show errors for empty DIMM slots.
+ */
+ if (enable_per_layer_report && dimm->nr_pages) {
+ if (p != label) {
+ strcpy(p, OTHER_LABEL);
+ p += strlen(OTHER_LABEL);
+ }
+ strcpy(p, dimm->label);
+ p += strlen(p);
+ *p = '\0';
+
+ /*
+ * get csrow/channel of the DIMM, in order to allow
+ * incrementing the compat API counters
+ */
+ debugf4("%s: %s csrows map: (%d,%d)\n",
+ __func__,
+ mci->mem_is_per_rank ? "rank" : "dimm",
+ dimm->csrow, dimm->cschannel);
+
+ if (row == -1)
+ row = dimm->csrow;
+ else if (row >= 0 && row != dimm->csrow)
+ row = -2;
+
+ if (chan == -1)
+ chan = dimm->cschannel;
+ else if (chan >= 0 && chan != dimm->cschannel)
+ chan = -2;
+ }
}
- if (channelb >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-b out of range "
- "(%d >= %d)\n",
- channelb, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ if (!enable_per_layer_report) {
+ strcpy(label, "any memory");
+ } else {
+ debugf4("%s: csrow/channel to increment: (%d,%d)\n",
+ __func__, row, chan);
+ if (p == label)
+ strcpy(label, "unknown memory");
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ if (row >= 0) {
+ mci->csrows[row].ce_count++;
+ if (chan >= 0)
+ mci->csrows[row].channels[chan].ce_count++;
+ }
+ } else
+ if (row >= 0)
+ mci->csrows[row].ue_count++;
}
- mci->ue_count++;
- mci->csrows[csrow].ue_count++;
-
- /* Generate the DIMM labels from the specified channels */
- label = mci->csrows[csrow].channels[channela].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- chars = snprintf(pos, len + 1, "-%s",
- mci->csrows[csrow].channels[channelb].dimm->label);
-
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela, channelb,
- labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela,
- channelb, labels, msg);
-}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ue);
-
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process CE events
- */
-void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
- unsigned int csrow, unsigned int channel, char *msg)
-{
- char *label = NULL;
+ /* Fill the RAM location data */
+ p = location;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ continue;
- /* Ensure boundary values */
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
- }
- if (channel >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range (%d >= %d)\n",
- channel, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
+ p += sprintf(p, "%s %d ",
+ edac_layer_name[mci->layers[i].type],
+ pos[i]);
}
- label = mci->csrows[csrow].channels[channel].dimm->label;
+ /* Memory type dependent details about the error */
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d syndrome 0x%lx",
+ page_frame_number, offset_in_page,
+ grain, syndrome);
- if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
- edac_mc_printk(mci, KERN_WARNING,
- "CE row %d, channel %d, label \"%s\": %s\n",
- csrow, channel, label, msg);
+ edac_ce_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report,
+ page_frame_number, offset_in_page, grain);
+ } else {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d",
+ page_frame_number, offset_in_page, grain);
- mci->ce_count++;
- mci->csrows[csrow].ce_count++;
- mci->csrows[csrow].channels[channel].dimm->ce_count++;
- mci->csrows[csrow].channels[channel].ce_count++;
+ edac_ue_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report);
+ }
}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ce);
+EXPORT_SYMBOL_GPL(edac_mc_handle_error);
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 3b8798d..c8f507d 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -412,18 +412,20 @@ struct edac_mc_layer {
/* FIXME: add the proper per-location error counts */
struct dimm_info {
char label[EDAC_MC_LABEL_LEN + 1]; /* DIMM label on motherboard */
- unsigned memory_controller;
- unsigned csrow;
- unsigned csrow_channel;
+
+ /* Memory location data */
+ unsigned location[EDAC_MAX_LAYERS];
+
+ struct mem_ctl_info *mci; /* the parent */
u32 grain; /* granularity of reported error in bytes */
enum dev_type dtype; /* memory device type */
enum mem_type mtype; /* memory dimm type */
enum edac_type edac_mode; /* EDAC mode for this dimm */
- u32 nr_pages; /* number of pages in csrow */
+ u32 nr_pages; /* number of pages on this dimm */
- u32 ce_count; /* Correctable Errors for this dimm */
+ unsigned csrow, cschannel; /* Points to the old API data */
};
/**
@@ -443,9 +445,10 @@ struct dimm_info {
*/
struct rank_info {
int chan_idx;
- u32 ce_count;
struct csrow_info *csrow;
struct dimm_info *dimm;
+
+ u32 ce_count; /* Correctable Errors for this csrow */
};
struct csrow_info {
@@ -541,13 +544,18 @@ struct mem_ctl_info {
unsigned long (*ctl_page_to_phys) (struct mem_ctl_info * mci,
unsigned long page);
int mc_idx;
- int nr_csrows;
struct csrow_info *csrows;
+ unsigned nr_csrows, num_cschannel;
+
+ /* Memory Controller hierarchy */
+ unsigned n_layers;
+ struct edac_mc_layer *layers;
+ bool mem_is_per_rank;
/*
* DIMM info. Will eventually remove the entire csrows_info some day
*/
- unsigned nr_dimms;
+ unsigned tot_dimms;
struct dimm_info *dimms;
/*
@@ -562,12 +570,16 @@ struct mem_ctl_info {
const char *dev_name;
char proc_name[MC_PROC_NAME_MAX_LEN + 1];
void *pvt_info;
- u32 ue_noinfo_count; /* Uncorrectable Errors w/o info */
- u32 ce_noinfo_count; /* Correctable Errors w/o info */
- u32 ue_count; /* Total Uncorrectable Errors for this MC */
- u32 ce_count; /* Total Correctable Errors for this MC */
unsigned long start_time; /* mci load start time (in jiffies) */
+ /*
+ * drivers shouldn't access those fields directly, as the core
+ * already handles that.
+ */
+ u32 ce_noinfo_count, ue_noinfo_count;
+ u32 ue_count, ce_count;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+
struct completion complete;
/* edac sysfs device control */
@@ -580,7 +592,7 @@ struct mem_ctl_info {
* by the low level driver.
*
* Set by the low level driver to provide attributes at the
- * controller level, same level as 'ue_count' and 'ce_count' above.
+ * controller level.
* An array of structures, NULL terminated
*
* If attributes are desired, then set to array of attributes
^ permalink raw reply related
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers, second version
From: Borislav Petkov @ 2012-05-04 10:16 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Borislav Petkov, Egor Martovetsky, Niklas Söderlund,
Tim Small, Arvind R., Chris Metcalf, Olof Johansson,
Doug Thompson, Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <4FA29356.3040601@redhat.com>
On Thu, May 03, 2012 at 11:16:54AM -0300, Mauro Carvalho Chehab wrote:
> edac: Change internal representation to work with layers
>
> From: Mauro Carvalho Chehab <mchehab@redhat.com>
>
> Change the EDAC internal representation to work with non-csrow
> based memory controllers.
>
> There are lots of those memory controllers nowadays, and more
> are coming. So, the EDAC internal representation needs to be
> changed, in order to work with those memory controllers, while
> preserving backward compatibility with the old ones.
>
> The edac core was written with the idea that memory controllers
> are able to directly access csrows.
>
> This is not true for FB-DIMM and RAMBUS memory controllers.
>
> Also, some recent advanced memory controllers don't present a per-csrows
> view. Instead, they view memories as DIMMs, instead of ranks.
>
> So, change the allocation and error report routines to allow
> them to work with all types of architectures.
>
> This will allow the removal of several hacks with FB-DIMM and RAMBUS
> memory controllers.
>
> Also, several tests were done on different platforms using different
> x86 drivers.
>
> TODO: a multi-rank DIMMs are currently represented by multiple DIMM
> entries in struct dimm_info. That means that changing a label for one
> rank won't change the same label for the other ranks at the same DIMM.
> This bug is present since the beginning of the EDAC, so it is not a big
> deal. However, on several drivers, it is possible to fix this issue, but
> it should be a per-driver fix, as the csrow => DIMM arrangement may not
> be equal for all. So, don't try to fix it here yet.
>
> I tried to make this patch as short as possible, preceding it with
> several other patches that simplified the logic here. Yet, as the
> internal API changes, all drivers need changes. The changes are
> generally bigger in the drivers for FB-DIMMs.
>
> Cc: Aristeu Rozanski <arozansk@redhat.com>
> Cc: Doug Thompson <norsk5@yahoo.com>
> Cc: Borislav Petkov <borislav.petkov@amd.com>
> Cc: Mark Gross <mark.gross@intel.com>
> Cc: Jason Uhlenkott <juhlenko@akamai.com>
> Cc: Tim Small <tim@buttersideup.com>
> Cc: Ranganathan Desikan <ravi@jetztechnologies.com>
> Cc: "Arvind R." <arvino55@gmail.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Egor Martovetsky <egor@pasemi.com>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Michal Marek <mmarek@suse.cz>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Hitoshi Mitake <h.mitake@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: "Niklas Söderlund" <niklas.soderlund@ericsson.com>
> Cc: Shaohui Xie <Shaohui.Xie@freescale.com>
> Cc: Josh Boyer <jwboyer@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>
> ---
> v18: Addresses the pointed issues on v17 review
>
> diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
> index e48ab31..1286c5e 100644
> --- a/drivers/edac/edac_core.h
> +++ b/drivers/edac/edac_core.h
> @@ -447,8 +447,12 @@ static inline void pci_write_bits32(struct pci_dev *pdev, int offset,
>
> #endif /* CONFIG_PCI */
>
> -extern struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
> - unsigned nr_chans, int edac_index);
> +struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
> + unsigned nr_chans, int edac_index);
> +struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
> + unsigned n_layers,
> + struct edac_mc_layer *layers,
> + unsigned sz_pvt);
> extern int edac_mc_add_mc(struct mem_ctl_info *mci);
> extern void edac_mc_free(struct mem_ctl_info *mci);
> extern struct mem_ctl_info *edac_mc_find(int idx);
> @@ -467,24 +471,78 @@ extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
> * reporting logic and function interface - reduces conditional
> * statement clutter and extra function arguments.
> */
> -extern void edac_mc_handle_ce(struct mem_ctl_info *mci,
> - unsigned long page_frame_number,
> - unsigned long offset_in_page,
> - unsigned long syndrome, int row, int channel,
> - const char *msg);
> -extern void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
> - const char *msg);
> -extern void edac_mc_handle_ue(struct mem_ctl_info *mci,
> - unsigned long page_frame_number,
> - unsigned long offset_in_page, int row,
> - const char *msg);
> -extern void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
> - const char *msg);
> -extern void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci, unsigned int csrow,
> - unsigned int channel0, unsigned int channel1,
> - char *msg);
> -extern void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci, unsigned int csrow,
> - unsigned int channel, char *msg);
> +
> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
> + struct mem_ctl_info *mci,
> + const unsigned long page_frame_number,
> + const unsigned long offset_in_page,
> + const unsigned long syndrome,
> + const int layer0,
> + const int layer1,
> + const int layer2,
> + const char *msg,
> + const char *other_detail,
> + const void *mcelog);
> +
> +static inline void edac_mc_handle_ce(struct mem_ctl_info *mci,
> + unsigned long page_frame_number,
> + unsigned long offset_in_page,
> + unsigned long syndrome, int row, int channel,
> + const char *msg)
> +{
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + page_frame_number, offset_in_page, syndrome,
> + row, channel, -1, msg, NULL, NULL);
> +}
> +
> +static inline void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
> + const char *msg)
> +{
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
> +}
> +
> +static inline void edac_mc_handle_ue(struct mem_ctl_info *mci,
> + unsigned long page_frame_number,
> + unsigned long offset_in_page, int row,
> + const char *msg)
> +{
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + page_frame_number, offset_in_page, 0,
> + row, -1, -1, msg, NULL, NULL);
> +}
> +
> +static inline void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
> + const char *msg)
> +{
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
> +}
> +
> +static inline void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
> + unsigned int csrow,
> + unsigned int channel0,
> + unsigned int channel1,
> + char *msg)
> +{
> + /*
> + *FIXME: The error can also be at channel1 (e. g. at the second
> + * channel of the same branch). The fix is to push
> + * edac_mc_handle_error() call into each driver
> + */
> + edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
> + 0, 0, 0,
> + csrow, channel0, -1, msg, NULL, NULL);
> +}
> +
> +static inline void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
> + unsigned int csrow,
> + unsigned int channel, char *msg)
> +{
> + edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
> + 0, 0, 0,
> + csrow, channel, -1, msg, NULL, NULL);
> +}
>
> /*
> * edac_device APIs
> @@ -496,6 +554,7 @@ extern void edac_device_handle_ue(struct edac_device_ctl_info *edac_dev,
> extern void edac_device_handle_ce(struct edac_device_ctl_info *edac_dev,
> int inst_nr, int block_nr, const char *msg);
> extern int edac_device_alloc_index(void);
> +extern const char *edac_layer_name[];
>
> /*
> * edac_pci APIs
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index 6ec967a..10cebb8 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -44,9 +44,25 @@ static void edac_mc_dump_channel(struct rank_info *chan)
> debugf4("\tchannel = %p\n", chan);
> debugf4("\tchannel->chan_idx = %d\n", chan->chan_idx);
> debugf4("\tchannel->csrow = %p\n\n", chan->csrow);
> - debugf4("\tdimm->ce_count = %d\n", chan->dimm->ce_count);
> - debugf4("\tdimm->label = '%s'\n", chan->dimm->label);
> - debugf4("\tdimm->nr_pages = 0x%x\n", chan->dimm->nr_pages);
> + debugf4("\tchannel->dimm = %p\n", chan->dimm);
> +}
> +
> +static void edac_mc_dump_dimm(struct dimm_info *dimm)
> +{
> + int i;
> +
> + debugf4("\tdimm = %p\n", dimm);
> + debugf4("\tdimm->label = '%s'\n", dimm->label);
> + debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
> + debugf4("\tdimm location ");
> + for (i = 0; i < dimm->mci->n_layers; i++) {
> + printk(KERN_CONT "%d", dimm->location[i]);
> + if (i < dimm->mci->n_layers - 1)
> + printk(KERN_CONT ".");
> + }
> + printk(KERN_CONT "\n");
> + debugf4("\tdimm->grain = %d\n", dimm->grain);
> + debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
> }
>
> static void edac_mc_dump_csrow(struct csrow_info *csrow)
> @@ -70,6 +86,8 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci)
> debugf4("\tmci->edac_check = %p\n", mci->edac_check);
> debugf3("\tmci->nr_csrows = %d, csrows = %p\n",
> mci->nr_csrows, mci->csrows);
> + debugf3("\tmci->nr_dimms = %d, dimms = %p\n",
> + mci->tot_dimms, mci->dimms);
> debugf3("\tdev = %p\n", mci->dev);
> debugf3("\tmod_name:ctl_name = %s:%s\n", mci->mod_name, mci->ctl_name);
> debugf3("\tpvt_info = %p\n\n", mci->pvt_info);
> @@ -157,10 +175,20 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
> }
>
> /**
> - * edac_mc_alloc: Allocate a struct mem_ctl_info structure
> - * @size_pvt: size of private storage needed
> - * @nr_csrows: Number of CWROWS needed for this MC
> - * @nr_chans: Number of channels for the MC
> + * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
> + * @mc_num: Memory controller number
> + * @n_layers: Number of MC hierarchy layers
> + * layers: Describes each layer as seen by the Memory Controller
> + * @size_pvt: size of private storage needed
> + *
> + *
> + * Non-csrow based drivers (like FB-DIMM and RAMBUS ones) will likely report
> + * such DIMMS properly, but the CSROWS-based ones will likely do the wrong
DIMMs csrow-based
> + * thing, as two chip select values are used for dual-rank memories (and 4, for
> + * quad-rank ones). I suspect that this issue could be solved inside the EDAC
> + * core for SDRAM memories, but it requires further study at JEDEC JESD 21C.
The paragraph above is still in, let me repeat my last note:
"This last paragraph sounds innacurately, especially the "likely"
adverbs make it even more confusing. The gist of what you're saying is
already present in the commit message anyway, so drop it here pls."
> + *
> + * In summary, solving this issue is not easy, as it requires a lot of testing.
As before:
"Also superfluous and has nothing to do with edac_mc_alloc()."
Pls remove it.
> * Everything is kmalloc'ed as one big chunk - more efficient.
> * Only can be used if all structures have the same lifetime - otherwise
> @@ -168,22 +196,49 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
> *
> * Use edac_mc_free() to free mc structures allocated by this function.
> *
> + * NOTE: drivers handle multi-rank memories in different ways: in some
> + * drivers, one multi-rank memory stick is mapped as one entry, while, in
> + * others, a single multi-rank memory stick would be mapped into several
> + * entries. Currently, this function will allocate multiple struct dimm_info
> + * on such scenarios, as grouping the multiple ranks require drivers change.
> + *
> * Returns:
> * NULL allocation failed
> * struct mem_ctl_info pointer
Ok, this patch still doesn't contain all the changes I requested for
although you said you did them. Is this not the latest version? I'll
wait for you to sort it out before I continue reviewing...
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
From: Mauro Carvalho Chehab @ 2012-05-04 10:15 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Egor Martovetsky, Niklas Söderlund, Tim Small, Arvind R.,
Chris Metcalf, Olof Johansson, Doug Thompson,
Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <20120504095228.GA18459@aftab.osrc.amd.com>
Em 04-05-2012 06:52, Borislav Petkov escreveu:
> On Thu, May 03, 2012 at 11:16:54AM -0300, Mauro Carvalho Chehab wrote:
>>>> + bool enable_filter,
>>>> + unsigned pos[EDAC_MAX_LAYERS])
>>>
>>> Passing the whole array as an argument instead of only a pointer to it?
>>
>> This is C, and not C++ or Pascal. Only the pointer is passed here. The size
>> of the array is used for type check only.
>
> Right, and you can see where he still has trouble. And by "he" I mean me :).
:)
>
> [ … ]
>
>>>> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
>>>> + struct mem_ctl_info *mci,
>>>> + const unsigned long page_frame_number,
>>>> + const unsigned long offset_in_page,
>>>> + const unsigned long syndrome,
>>>> + const int layer0,
>>>> + const int layer1,
>>>> + const int layer2,
>>>
>>> Instead of passing each layer as an arg, you can prepare the array pos[]
>>> in each edac_mc_hanlde_*() and pass around a pointer to it - you need it
>>> anyway in the edac_mc_inc*() functions.
>>
>> Yes, but the changes at the drivers will be more complex, without any reason:
>> before each call to this function, they would need to create and fill a temporary
>> array.
>>
>> As there are only 3 layers, in the worse case, this way is simpler and more
>> efficient. We can review it, if we ever need more than 3 layers.
>
> I see, the edac_mc_handle_error is the main interface for all edac drivers, ok.
>
> [ … ]
>
>>>> + bool enable_filter = false;
>>>
>>> What does this enable_filter thing mean:
>>>
>>> if (pos[i] >= 0)
>>> enable_filter = true;
>>>
>>> This absolutely needs explanation and better naming!
>>
>> Renamed it to "enable_per_layer_report".
>
> Or "detailed_dimm_report" or ...
Detail is used on another context; "enable_per_layer_report" won't generate
any doubts for anyone reading the code.
>> The code that implement it seems self-explained:
>>
>> ..
>> if (enable_filter && dimm->nr_pages) {
>> if (p != label) {
>> strcpy(p, OTHER_LABEL);
>> p += strlen(OTHER_LABEL);
>> }
>> strcpy(p, dimm->label);
>> p += strlen(p);
>> *p = '\0';
>>
>> ..
>>
>> if (!enable_filter) {
>> strcpy(label, "any memory");
>> } else {
>> debugf4("%s: csrow/channel to increment: (%d,%d)\n",
>> __func__, row, chan);
>> if (p == label)
>> strcpy(label, "unknown memory");
>> if (type == HW_EVENT_ERR_CORRECTED) {
>> if (row >= 0) {
>> mci->csrows[row].ce_count++;
>> if (chan >= 0)
>> mci->csrows[row].channels[chan].ce_count++;
>> }
>> } else
>> if (row >= 0)
>> mci->csrows[row].ue_count++;
>> }
>>
>> Theis flag indicates if is there any useful information about the affected
>> DIMM(s) provided by the EDAC driver. If this is provided, the DIMM location labels are
>> filtered and reported, and the per-layer error counters are incremented.
>>
>> As it was discussed on previous reviews, with FB-DIMM MCs, and/or when mirror
>> mode/lockstep mode is enabled, the memory controller points errors to 2 DIMMs
>> (or 4 DIMMs, when both mirror mode and lockstep mode are enabled) on most memory
>> controllers, under some conditions. The edac_mc_handle_fbd_ue() function call were
>> created due to that.
>>
>> When comparing with the old code, "enable_filter = false" would be equivalent to call
>> edac_mc_handle_ce_no_info/edac_mc_handle_ue_no_info.
>>
>> I'm adding a comment about it.
>
> Much better, thanks.
>
> Btw, I have to admit, this is a pretty strange way of handling the case
> where layers are { -1, -1, -1 }, i.e. edac_mc_handle_error is called
> with the "no info" hint.
Well, negative values are used on Linux to indicate error conditions, so this is not
that strange. Also, it allows partial "no info", as, on some cases, a channel or
a csrow may not be known. So, this allows code simplification at the drivers.
For example, look at this hunk on the amd64_edac conversion patch:
@@ -1585,16 +1609,10 @@ static void f1x_map_sysaddr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr,
if (dct_ganging_enabled(pvt))
chan = get_channel_from_ecc_syndrome(mci, syndrome);
- if (chan >= 0)
- edac_mc_handle_ce(mci, page, offset, syndrome, csrow, chan,
- EDAC_MOD_STR);
- else
- /*
- * Channel unknown, report all channels on this CSROW as failed.
- */
- for (chan = 0; chan < mci->csrows[csrow].nr_channels; chan++)
- edac_mc_handle_ce(mci, page, offset, syndrome,
- csrow, chan, EDAC_MOD_STR);
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ page, offset, syndrome,
+ csrow, chan, -1,
+ EDAC_MOD_STR, "", NULL);
There's no need anymore to check if chan is bigger than 0: if channel is invalid,
the edac_mc_handle_error() will get the DIMM labels for all channels, without needing
to do any loop inside the drivers.
> I'm wondering whether it wouldn't be more readable if you could do
>
> edac_mc_handle_error(HW_EVENT_ERR_INFO_INVALID | ..)
>
> or similar and define such a flag which simply states that. But you'll
> have to change enum hw_event_mc_err_type to a bitfield to allow more
> than one set bit.
>
> Hmm.
>
>
> [ … ]
>
>>> The SCRUB_SW_SRC piece can be another function.
>>
>> It is now part of the edac_ce_error().
>
> Hm, I can't find this function on your "experimental" branch on
> infradead but it is mentioned in the inlined patch below, what's going
> on? Which patch should I be looking at now?
My fault. I forgot to update the push line for the "experimental" remote
at .git/config. I just updated it with the right branch.
The tree on Infradead should now point to the same patch I forwarded at the
ML.
>
> [ … ]
>
>> The following patch addresses the pointed issues. I've updated them
>> on my experimental branch at infradead:
>> git://git.infradead.org/users/mchehab/edac.git experimental
>
> Ok, I checked this one out but can't find the edac_ce_error() function
> as already stated above, pls check.
>
>> They'll also be soon available at:
>> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-edac.git hw_events_v18
>
> Will review the patch below now and reply in another mail.
Thanks!
Mauro
^ permalink raw reply
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
From: Borislav Petkov @ 2012-05-04 9:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Borislav Petkov, Egor Martovetsky, Niklas Söderlund,
Tim Small, Arvind R., Chris Metcalf, Olof Johansson,
Doug Thompson, Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <4FA29356.3040601@redhat.com>
On Thu, May 03, 2012 at 11:16:54AM -0300, Mauro Carvalho Chehab wrote:
> >> + bool enable_filter,
> >> + unsigned pos[EDAC_MAX_LAYERS])
> >
> > Passing the whole array as an argument instead of only a pointer to it?
>
> This is C, and not C++ or Pascal. Only the pointer is passed here. The size
> of the array is used for type check only.
Right, and you can see where he still has trouble. And by "he" I mean me :).
[ … ]
> >> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
> >> + struct mem_ctl_info *mci,
> >> + const unsigned long page_frame_number,
> >> + const unsigned long offset_in_page,
> >> + const unsigned long syndrome,
> >> + const int layer0,
> >> + const int layer1,
> >> + const int layer2,
> >
> > Instead of passing each layer as an arg, you can prepare the array pos[]
> > in each edac_mc_hanlde_*() and pass around a pointer to it - you need it
> > anyway in the edac_mc_inc*() functions.
>
> Yes, but the changes at the drivers will be more complex, without any reason:
> before each call to this function, they would need to create and fill a temporary
> array.
>
> As there are only 3 layers, in the worse case, this way is simpler and more
> efficient. We can review it, if we ever need more than 3 layers.
I see, the edac_mc_handle_error is the main interface for all edac drivers, ok.
[ … ]
> >> + bool enable_filter = false;
> >
> > What does this enable_filter thing mean:
> >
> > if (pos[i] >= 0)
> > enable_filter = true;
> >
> > This absolutely needs explanation and better naming!
>
> Renamed it to "enable_per_layer_report".
Or "detailed_dimm_report" or ...
> The code that implement it seems self-explained:
>
> ..
> if (enable_filter && dimm->nr_pages) {
> if (p != label) {
> strcpy(p, OTHER_LABEL);
> p += strlen(OTHER_LABEL);
> }
> strcpy(p, dimm->label);
> p += strlen(p);
> *p = '\0';
>
> ..
>
> if (!enable_filter) {
> strcpy(label, "any memory");
> } else {
> debugf4("%s: csrow/channel to increment: (%d,%d)\n",
> __func__, row, chan);
> if (p == label)
> strcpy(label, "unknown memory");
> if (type == HW_EVENT_ERR_CORRECTED) {
> if (row >= 0) {
> mci->csrows[row].ce_count++;
> if (chan >= 0)
> mci->csrows[row].channels[chan].ce_count++;
> }
> } else
> if (row >= 0)
> mci->csrows[row].ue_count++;
> }
>
> Theis flag indicates if is there any useful information about the affected
> DIMM(s) provided by the EDAC driver. If this is provided, the DIMM location labels are
> filtered and reported, and the per-layer error counters are incremented.
>
> As it was discussed on previous reviews, with FB-DIMM MCs, and/or when mirror
> mode/lockstep mode is enabled, the memory controller points errors to 2 DIMMs
> (or 4 DIMMs, when both mirror mode and lockstep mode are enabled) on most memory
> controllers, under some conditions. The edac_mc_handle_fbd_ue() function call were
> created due to that.
>
> When comparing with the old code, "enable_filter = false" would be equivalent to call
> edac_mc_handle_ce_no_info/edac_mc_handle_ue_no_info.
>
> I'm adding a comment about it.
Much better, thanks.
Btw, I have to admit, this is a pretty strange way of handling the case
where layers are { -1, -1, -1 }, i.e. edac_mc_handle_error is called
with the "no info" hint.
I'm wondering whether it wouldn't be more readable if you could do
edac_mc_handle_error(HW_EVENT_ERR_INFO_INVALID | ..)
or similar and define such a flag which simply states that. But you'll
have to change enum hw_event_mc_err_type to a bitfield to allow more
than one set bit.
Hmm.
[ … ]
> > The SCRUB_SW_SRC piece can be another function.
>
> It is now part of the edac_ce_error().
Hm, I can't find this function on your "experimental" branch on
infradead but it is mentioned in the inlined patch below, what's going
on? Which patch should I be looking at now?
[ … ]
> The following patch addresses the pointed issues. I've updated them
> on my experimental branch at infradead:
> git://git.infradead.org/users/mchehab/edac.git experimental
Ok, I checked this one out but can't find the edac_ce_error() function
as already stated above, pls check.
> They'll also be soon available at:
> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-edac.git hw_events_v18
Will review the patch below now and reply in another mail.
Thanks.
>
> Regards,
> Mauro
>
> -
>
> edac: Change internal representation to work with layers
>
> From: Mauro Carvalho Chehab <mchehab@redhat.com>
>
> Change the EDAC internal representation to work with non-csrow
> based memory controllers.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply
* Re: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay
From: Benjamin Herrenschmidt @ 2012-05-04 0:10 UTC (permalink / raw)
To: Wang Sheng-Hui
Cc: Stephen Rothwell, linux-kernel, Milton Miller, Anton Blanchard,
linuxppc-dev
In-Reply-To: <4FA31653.5090103@gmail.com>
On Fri, 2012-05-04 at 07:35 +0800, Wang Sheng-Hui wrote:
> > No, Only comment the test, you must absolutely leave the
> > __hard_irq_disable() call ! That's the whole point of the test, make
> > sure we unconditionally disable to see if that fixes the problem, in
> > which case that will tell us that we somewhere accidentally leave
> > irq_happened set to 0x01 while irqs are hard enabled.
>
> It can work.
> My system has been running for about 15 hours without crash.
Ok, so now we need to understand under what circumstances we end up
in a situation where paca->irq_happened is 0x01 and IRQs are hard
enabled. I have a few ideas of things to look at but I'm also off
for the week-end.
I'll have a look next week.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/1] page_alloc.c: remove argument to pageblock_default_order
From: Andrew Morton @ 2012-05-03 23:37 UTC (permalink / raw)
To: rajman mekaco
Cc: linux-ia64, Mel Gorman, linux-kernel, linux-mm, Minchan Kim,
Tejun Heo, linuxppc-dev, KAMEZAWA Hiroyuki
In-Reply-To: <1336065312-2891-1-git-send-email-rajman.mekaco@gmail.com>
On Thu, 3 May 2012 22:45:12 +0530
rajman mekaco <rajman.mekaco@gmail.com> wrote:
> When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined, then
> pageblock_default_order has an argument to it.
>
> However, free_area_init_core will call it without any argument
> anyway.
>
> Remove the argument to pageblock_default_order when
> CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined.
>
> Signed-off-by: rajman mekaco <rajman.mekaco@gmail.com>
> ---
> mm/page_alloc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a712fb9..4b95412 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4274,7 +4274,7 @@ static inline void __init set_pageblock_order(unsigned int order)
> * at compile-time. See include/linux/pageblock-flags.h for the values of
> * pageblock_order based on the kernel config
> */
> -static inline int pageblock_default_order(unsigned int order)
> +static inline int pageblock_default_order(void)
> {
> return MAX_ORDER-1;
> }
Interesting. It has been that way since at least 3.1.
It didn't break the build because pageblock_default_order() is only
ever invoked by set_pageblock_order(), with:
set_pageblock_order(pageblock_default_order());
and set_pageblock_order() is a macro:
#define set_pageblock_order(x) do {} while (0)
There's yet another reason not to use macros, dammit - they hide bugs.
Mel, can you have a think about this please? Can we just kill off
pageblock_default_order() and fold its guts into
set_pageblock_order(void)? Only ia64 and powerpc can define
CONFIG_HUGETLB_PAGE_SIZE_VARIABLE.
--- a/mm/page_alloc.c~a
+++ a/mm/page_alloc.c
@@ -4300,25 +4300,24 @@ static inline void setup_usemap(struct p
#ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
-/* Return a sensible default order for the pageblock size. */
-static inline int pageblock_default_order(void)
-{
- if (HPAGE_SHIFT > PAGE_SHIFT)
- return HUGETLB_PAGE_ORDER;
-
- return MAX_ORDER-1;
-}
-
/* Initialise the number of pages represented by NR_PAGEBLOCK_BITS */
-static inline void __init set_pageblock_order(unsigned int order)
+static inline void __init set_pageblock_order(void)
{
+ unsigned int order;
+
/* Check that pageblock_nr_pages has not already been setup */
if (pageblock_order)
return;
+ if (HPAGE_SHIFT > PAGE_SHIFT)
+ order = HUGETLB_PAGE_ORDER;
+ else
+ order = MAX_ORDER - 1;
+
/*
* Assume the largest contiguous order of interest is a huge page.
- * This value may be variable depending on boot parameters on IA64
+ * This value may be variable depending on boot parameters on IA64 and
+ * powerpc.
*/
pageblock_order = order;
}
@@ -4326,15 +4325,13 @@ static inline void __init set_pageblock_
/*
* When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not set, set_pageblock_order()
- * and pageblock_default_order() are unused as pageblock_order is set
- * at compile-time. See include/linux/pageblock-flags.h for the values of
- * pageblock_order based on the kernel config
+ * is unused as pageblock_order is set at compile-time. See
+ * include/linux/pageblock-flags.h for the values of pageblock_order based on
+ * the kernel config
*/
-static inline int pageblock_default_order(unsigned int order)
+static inline void set_pageblock_order(void)
{
- return MAX_ORDER-1;
}
-#define set_pageblock_order(x) do {} while (0)
#endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
@@ -4422,7 +4419,7 @@ static void __paginginit free_area_init_
if (!size)
continue;
- set_pageblock_order(pageblock_default_order());
+ set_pageblock_order();
setup_usemap(pgdat, zone, size);
ret = init_currently_empty_zone(zone, zone_start_pfn,
size, MEMMAP_EARLY);
_
^ permalink raw reply
* Re: [PATCH] powerpc: use local var instead of local_paca->irq_happened directly in __check_irq_replay
From: Wang Sheng-Hui @ 2012-05-03 23:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stephen Rothwell, linux-kernel, Milton Miller, Anton Blanchard,
linuxppc-dev
In-Reply-To: <1336032561.18712.1.camel@pasglop>
On 2012年05月03日 16:09, Benjamin Herrenschmidt wrote:
> On Thu, 2012-05-03 at 14:59 +0800, Wang Sheng-Hui wrote:
>> On 2012年05月03日 14:33, Wang Sheng-Hui wrote:
>>> if (unlikely(irq_happened != PACA_IRQ_HARD_DIS))
>>>> __hard_irq_disable();
>>
>> I have commented out the 2 lines.
>
> No, Only comment the test, you must absolutely leave the
> __hard_irq_disable() call ! That's the whole point of the test, make
> sure we unconditionally disable to see if that fixes the problem, in
> which case that will tell us that we somewhere accidentally leave
> irq_happened set to 0x01 while irqs are hard enabled.
It can work.
My system has been running for about 15 hours without crash.
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [PATCH v16 01/10]USB/ppc4xx: Add Synopsys DWC OTG Register definitions
From: Wolfgang Denk @ 2012-05-03 18:30 UTC (permalink / raw)
To: Rupjyoti Sarmah; +Cc: linuxppc-dev, rsarmah, linux-kernel
In-Reply-To: <201205031223.q43CN3LN022598@amcc.com>
Dear Rupjyoti Sarmah,
In message <201205031223.q43CN3LN022598@amcc.com> you wrote:
>
> Add Synopsys Design Ware core register definitions.
Olof Johansson <olof@lixom.net> has commented v15 of this patch as
follows:
> No, just start over from scratch. Just leave the crap driver behind,
> use it for reference but write the new one.
>
> It's obvious given that you are already at iteration v15 and it's
> still looking this bad that this is not realistic to get reviewed and
> accepted as-is. I don't think staging is a good target either -- what
> the driver really needs is _functional_ cut-down to only cover the use
> cases that your product uses, and staging cleanups are mostly around
> style and refactoring, not changing, fixing or removing functionality.
> ...
> I don't think you understood what I meant. Try building an ARM config
> with this driver enabled, for example, and you'll see that it breaks
> the build.
See http://thread.gmane.org/gmane.linux.usb.general/53348/focus=53913
for the full context and other important comments.
It seems most of these requests have been ignored so far.
I would also like to point out that the same Synopsys USB controller
is used in a number of other SoCs (especially ARM chips), and
supported by other drivers, some of these even in mainline.
See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
for a related thread.
Instead of trying to add a completely new driver to mainline (and one
which has been repeatedly been rejected), I vote for focussing on the
existing driver code that is already in mainline, and testing and
improving this so we can use a single implementation of this driver
code for all SoCs that use the same IP block.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
panic: kernel trap (ignored)
^ permalink raw reply
* Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers
From: Mauro Carvalho Chehab @ 2012-05-03 14:16 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shaohui Xie, Jason Uhlenkott, Aristeu Rozanski, Hitoshi Mitake,
Mark Gross, Dmitry Eremin-Solenikov, Ranganathan Desikan,
Egor Martovetsky, Niklas Söderlund, Tim Small, Arvind R.,
Chris Metcalf, Olof Johansson, Doug Thompson,
Linux Edac Mailing List, Michal Marek, Jiri Kosina,
Linux Kernel Mailing List, Joe Perches, Andrew Morton,
linuxppc-dev
In-Reply-To: <20120502133052.GE12914@aftab.osrc.amd.com>
Em 02-05-2012 10:30, Borislav Petkov escreveu:
> Starting a new thread because the old one grew too big and
> is out of my screen. Patch below is git-formatted from
> git://git.infradead.org/users/mchehab/edac.git
>
>> commit 447b7929e633027ffe131f2f8f246bba5690cee7
>> Author: Mauro Carvalho Chehab <mchehab@redhat.com>
>> Date: Wed Apr 18 15:20:50 2012 -0300
>>
>> edac: Change internal representation to work with layers
>>
>> Change the EDAC internal representation to work with non-csrow
>> based memory controllers.
>>
>> There are lots of those memory controllers nowadays, and more
>> are coming. So, the EDAC internal representation needs to be
>> changed, in order to work with those memory controllers, while
>> preserving backward compatibility with the old ones.
>>
>> The edac core was written with the idea that memory controllers
>> are able to directly access csrows.
>>
>> This is not true for FB-DIMM and RAMBUS memory controllers.
>>
>> Also, some recent advanced memory controllers don't present a per-csrows
>> view. Instead, they view memories as DIMMs, instead of ranks.
>>
>> So, change the allocation and error report routines to allow
>> them to work with all types of architectures.
>>
>> This will allow the removal of several hacks with FB-DIMM and RAMBUS
>> memory controllers.
>>
>> Also, several tests were done on different platforms using different
>> x86 drivers.
>>
>> TODO: a multi-rank DIMMs are currently represented by multiple DIMM
>> entries in struct dimm_info. That means that changing a label for one
>> rank won't change the same label for the other ranks at the same DIMM.
>> This bug is present since the beginning of the EDAC, so it is not a big
>> deal. However, on several drivers, it is possible to fix this issue, but
>> it should be a per-driver fix, as the csrow => DIMM arrangement may not
>> be equal for all. So, don't try to fix it here yet.
>>
>> I tried to make this patch as short as possible, preceding it with
>> several other patches that simplified the logic here. Yet, as the
>> internal API changes, all drivers need changes. The changes are
>> generally bigger in the drivers for FB-DIMMs.
>
> <snip already reviewed stuff>
>
>> /* Figure out the offsets of the various items from the start of an mc
>> * structure. We want the alignment of each item to be at least as
>> @@ -191,12 +253,28 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
>> * hardcode everything into a single struct.
>> */
>> mci = edac_align_ptr(&ptr, sizeof(*mci), 1);
>> - csi = edac_align_ptr(&ptr, sizeof(*csi), nr_csrows);
>> - chi = edac_align_ptr(&ptr, sizeof(*chi), nr_csrows * nr_chans);
>> - dimm = edac_align_ptr(&ptr, sizeof(*dimm), nr_csrows * nr_chans);
>> + layer = edac_align_ptr(&ptr, sizeof(*layer), n_layers);
>> + csi = edac_align_ptr(&ptr, sizeof(*csi), tot_csrows);
>> + chi = edac_align_ptr(&ptr, sizeof(*chi), tot_csrows * tot_channels);
>> + dimm = edac_align_ptr(&ptr, sizeof(*dimm), tot_dimms);
>> + count = 1;
>> + for (i = 0; i < n_layers; i++) {
>> + count *= layers[i].size;
>> + debugf4("%s: errcount layer %d size %d\n", __func__, i, count);
>> + ce_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
>> + ue_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
>> + tot_errcount += 2 * count;
>> + }
>> +
>> + debugf4("%s: allocating %d error counters\n", __func__, tot_errcount);
>> pvt = edac_align_ptr(&ptr, sz_pvt, 1);
>> size = ((unsigned long)pvt) + sz_pvt;
>>
>> + debugf1("%s(): allocating %u bytes for mci data (%d %s, %d csrows/channels)\n",
>> + __func__, size,
>> + tot_dimms,
>> + per_rank ? "ranks" : "dimms",
>> + tot_csrows * tot_channels);
>> mci = kzalloc(size, GFP_KERNEL);
>> if (mci == NULL)
>> return NULL;
>> @@ -204,42 +282,101 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
>> /* Adjust pointers so they point within the memory we just allocated
>> * rather than an imaginary chunk of memory located at address 0.
>> */
>> + layer = (struct edac_mc_layer *)(((char *)mci) + ((unsigned long)layer));
>> csi = (struct csrow_info *)(((char *)mci) + ((unsigned long)csi));
>> chi = (struct rank_info *)(((char *)mci) + ((unsigned long)chi));
>> dimm = (struct dimm_info *)(((char *)mci) + ((unsigned long)dimm));
>> + for (i = 0; i < n_layers; i++) {
>> + mci->ce_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ce_per_layer[i]));
>> + mci->ue_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ue_per_layer[i]));
>> + }
>> pvt = sz_pvt ? (((char *)mci) + ((unsigned long)pvt)) : NULL;
>>
>> /* setup index and various internal pointers */
>> mci->mc_idx = edac_index;
>> mci->csrows = csi;
>> mci->dimms = dimm;
>> + mci->tot_dimms = tot_dimms;
>> mci->pvt_info = pvt;
>> - mci->nr_csrows = nr_csrows;
>> + mci->n_layers = n_layers;
>> + mci->layers = layer;
>> + memcpy(mci->layers, layers, sizeof(*layer) * n_layers);
>> + mci->nr_csrows = tot_csrows;
>> + mci->num_cschannel = tot_channels;
>> + mci->mem_is_per_rank = per_rank;
>>
>> /*
>> - * For now, assumes that a per-csrow arrangement for dimms.
>> - * This will be latter changed.
>> + * Fills the csrow struct
>
> Fill
>
>> */
>> - dimm = mci->dimms;
>> -
>> - for (row = 0; row < nr_csrows; row++) {
>> - csrow = &csi[row];
>> - csrow->csrow_idx = row;
>> - csrow->mci = mci;
>> - csrow->nr_channels = nr_chans;
>> - chp = &chi[row * nr_chans];
>> - csrow->channels = chp;
>> -
>> - for (chn = 0; chn < nr_chans; chn++) {
>> + for (row = 0; row < tot_csrows; row++) {
>> + csr = &csi[row];
>> + csr->csrow_idx = row;
>> + csr->mci = mci;
>> + csr->nr_channels = tot_channels;
>> + chp = &chi[row * tot_channels];
>> + csr->channels = chp;
>> +
>> + for (chn = 0; chn < tot_channels; chn++) {
>> chan = &chp[chn];
>> chan->chan_idx = chn;
>> - chan->csrow = csrow;
>> + chan->csrow = csr;
>> + }
>> + }
>>
>> - mci->csrows[row].channels[chn].dimm = dimm;
>> - dimm->csrow = row;
>> - dimm->csrow_channel = chn;
>> - dimm++;
>> - mci->nr_dimms++;
>> + /*
>> + * Fills the dimm struct
>
> Fill
>
>> + */
>> + memset(&pos, 0, sizeof(pos));
>> + row = 0;
>> + chn = 0;
>> + debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
>> + per_rank ? "ranks" : "dimms");
>> + for (i = 0; i < tot_dimms; i++) {
>> + chan = &csi[row].channels[chn];
>> + dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
>> + pos[0], pos[1], pos[2]);
>> + dimm->mci = mci;
>> +
>> + debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
>> + i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
>> + pos[0], pos[1], pos[2], row, chn);
>> +
>> + /* Copy DIMM location */
>> + for (j = 0; j < n_layers; j++)
>> + dimm->location[j] = pos[j];
>> +
>> + /* Link it to the csrows old API data */
>> + chan->dimm = dimm;
>> + dimm->csrow = row;
>> + dimm->cschannel = chn;
>> +
>> + /* Increment csrow location */
>> + if (!rev_order) {
>
> AFAICT, this rev_order is always false (in the final version of the
> patches anyway) and if so, can be completely removed as an argument to
> edac_mc_alloc() and also the code in the else-branch below.
>
>> + for (j = n_layers - 1; j >= 0; j--)
>> + if (!layers[j].is_virt_csrow)
>> + break;
>> + chn++;
>> + if (chn == tot_channels) {
>> + chn = 0;
>> + row++;
>> + }
>> + } else {
>> + for (j = n_layers - 1; j >= 0; j--)
>> + if (layers[j].is_virt_csrow)
>> + break;
>> + row++;
>> + if (row == tot_csrows) {
>> + row = 0;
>> + chn++;
>> + }
>> + }
>> +
>> + /* Increment dimm location */
>> + for (j = n_layers - 1; j >= 0; j--) {
>> + pos[j]++;
>> + if (pos[j] < layers[j].size)
>> + break;
>> + pos[j] = 0;
>> }
>> }
>>
>> @@ -263,6 +400,57 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
>> */
>> return mci;
>> }
>> +EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
>> +
>> +/**
>> + * edac_mc_alloc: Allocate and partially fills a struct mem_ctl_info structure
>
> fill
>
>> + * @edac_index: Memory controller number
>
> How about 'mc_num' for a variable name instead then? I know, I know, it
> was called like that originally but 'edac_index' is misleading.
>
>> + * @n_layers: Nu
>> +mber of layers at the MC hierarchy
>
> needless '\n'
>
>> + * layers: Describes each layer as seen by the Memory Controller
>> + * @rev_order: Fills csrows/cs channels at the reverse order
>
> you can drop that, as put above.
>
>> + * @size_pvt: size of private storage needed
>
> Capitalize: Size
>
>> + *
>> + *
>> + * FIXME: drivers handle multi-rank memories on different ways: on some
>
> in (remove "on")
>
>> + * drivers, one multi-rank memory is mapped as one DIMM, while, on others,
>
> and say: "Some drivers map multi-ranked DIMMs as one DIMM while others
> as several DIMMs".
>
>> + * a single multi-rank DIMM would be mapped into several "dimms".
>> + *
>> + * Non-csrow based drivers (like FB-DIMM and RAMBUS ones) will likely report
>> + * such DIMMS properly, but the csrow-based ones will likely do the wrong
>> + * thing, as two chip select values are used for dual-rank memories (and 4, for
>> + * quad-rank ones). I suspect that this issue could be solved inside the EDAC
>> + * core for SDRAM memories, but it requires further study at JEDEC JESD 21C.
>
> This last paragraph sounds innacurately, especially the "likely" adverbs
> make it even more confusing. The gist of what you're saying is already
> present in the commit message anyway, so drop it here pls.
>
>> + *
>> + * In summary, solving this issue is not easy, as it requires a lot of testing.
>
> Also superfluous and has nothing to do with edac_mc_alloc().
>
>> + *
>> + * Everything is kmalloc'ed as one big chunk - more efficient.
>> + * Only can be used if all structures have the same lifetime - otherwisea
>
> "It can only be used if ..."
>
>> + * you have to allocate and initialize your own structures.
>> + *
>> + * Use edac_mc_free() to free mc structures allocated by this function.
>> + *
>> + * Returns:
>> + * NULL allocation failed
>> + * struct mem_ctl_info pointer
>
> On failure: NULL
> On success: struct mem_ctl_info.
>
>> + */
>> +
>> +struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
>> + unsigned nr_chans, int edac_index)
>> +{
>> + unsigned n_layers = 2;
>> + struct edac_mc_layer layers[n_layers];
>> +
>> + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>> + layers[0].size = nr_csrows;
>> + layers[0].is_virt_csrow = true;
>> + layers[1].type = EDAC_MC_LAYER_CHANNEL;
>> + layers[1].size = nr_chans;
>> + layers[1].is_virt_csrow = false;
>> +
>> + return new_edac_mc_alloc(edac_index, ARRAY_SIZE(layers), layers,
>> + false, sz_pvt);
>> +}
>> EXPORT_SYMBOL_GPL(edac_mc_alloc);
>>
>> /**
>> @@ -528,7 +716,6 @@ EXPORT_SYMBOL(edac_mc_find);
>> * edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
>> * create sysfs entries associated with mci structure
>> * @mci: pointer to the mci structure to be added to the list
>> - * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
>> *
>> * Return:
>> * 0 Success
>> @@ -555,6 +742,8 @@ int edac_mc_add_mc(struct mem_ctl_info *mci)
>> edac_mc_dump_channel(&mci->csrows[i].
>> channels[j]);
>> }
>> + for (i = 0; i < mci->tot_dimms; i++)
>> + edac_mc_dump_dimm(&mci->dimms[i]);
>> }
>> #endif
>> mutex_lock(&mem_ctls_mutex);
>> @@ -712,261 +901,251 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
>> }
>> EXPORT_SYMBOL_GPL(edac_mc_find_csrow_by_page);
>>
>> -/* FIXME - setable log (warning/emerg) levels */
>> -/* FIXME - integrate with evlog: http://evlog.sourceforge.net/ */
>> -void edac_mc_handle_ce(struct mem_ctl_info *mci,
>> - unsigned long page_frame_number,
>> - unsigned long offset_in_page, unsigned long syndrome,
>> - int row, int channel, const char *msg)
>> +const char *edac_layer_name[] = {
>> + [EDAC_MC_LAYER_BRANCH] = "branch",
>> + [EDAC_MC_LAYER_CHANNEL] = "channel",
>> + [EDAC_MC_LAYER_SLOT] = "slot",
>> + [EDAC_MC_LAYER_CHIP_SELECT] = "csrow",
>> +};
>> +EXPORT_SYMBOL_GPL(edac_layer_name);
>> +
>> +static void edac_increment_ce_error(struct mem_ctl_info *mci,
>
> This could be abbreviated to edac_inc_ce_error()
All the above changed, as requested.
>> + bool enable_filter,
>> + unsigned pos[EDAC_MAX_LAYERS])
>
> Passing the whole array as an argument instead of only a pointer to it?
This is C, and not C++ or Pascal. Only the pointer is passed here. The size
of the array is used for type check only.
>> {
>> - unsigned long remapped_page;
>> - char *label = NULL;
>> - u32 grain;
>> + int i, index = 0;
>>
>> - debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
>> + mci->ce_mc++;
>>
>> - /* FIXME - maybe make panic on INTERNAL ERROR an option */
>> - if (row >= mci->nr_csrows || row < 0) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: row out of range "
>> - "(%d >= %d)\n", row, mci->nr_csrows);
>> - edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
>> + if (!enable_filter) {
>> + mci->ce_noinfo_count++;
>> return;
>> }
>>
>> - if (channel >= mci->csrows[row].nr_channels || channel < 0) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: channel out of range "
>> - "(%d >= %d)\n", channel,
>> - mci->csrows[row].nr_channels);
>> - edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
>> - return;
>> - }
>> -
>> - label = mci->csrows[row].channels[channel].dimm->label;
>> - grain = mci->csrows[row].channels[channel].dimm->grain;
>> + for (i = 0; i < mci->n_layers; i++) {
>> + if (pos[i] < 0)
>> + break;
>> + index += pos[i];
>> + mci->ce_per_layer[i][index]++;
>>
>> - if (edac_mc_get_log_ce())
>> - /* FIXME - put in DIMM location */
>> - edac_mc_printk(mci, KERN_WARNING,
>> - "CE page 0x%lx, offset 0x%lx, grain %d, syndrome "
>> - "0x%lx, row %d, channel %d, label \"%s\": %s\n",
>> - page_frame_number, offset_in_page,
>> - grain, syndrome, row, channel,
>> - label, msg);
>> + if (i < mci->n_layers - 1)
>> + index *= mci->layers[i + 1].size;
>> + }
>> +}
>>
>> - mci->ce_count++;
>> - mci->csrows[row].ce_count++;
>> - mci->csrows[row].channels[channel].dimm->ce_count++;
>> - mci->csrows[row].channels[channel].ce_count++;
>> +static void edac_increment_ue_error(struct mem_ctl_info *mci,
>> + bool enable_filter,
>> + unsigned pos[EDAC_MAX_LAYERS])
>> +{
>> + int i, index = 0;
>>
>> - if (mci->scrub_mode & SCRUB_SW_SRC) {
>> - /*
>> - * Some MC's can remap memory so that it is still available
>> - * at a different address when PCI devices map into memory.
>> - * MC's that can't do this lose the memory where PCI devices
>> - * are mapped. This mapping is MC dependent and so we call
>> - * back into the MC driver for it to map the MC page to
>> - * a physical (CPU) page which can then be mapped to a virtual
>> - * page - which can then be scrubbed.
>> - */
>> - remapped_page = mci->ctl_page_to_phys ?
>> - mci->ctl_page_to_phys(mci, page_frame_number) :
>> - page_frame_number;
>> + mci->ue_mc++;
>>
>> - edac_mc_scrub_block(remapped_page, offset_in_page, grain);
>> + if (!enable_filter) {
>> + mci->ce_noinfo_count++;
>> + return;
>> }
>> -}
>> -EXPORT_SYMBOL_GPL(edac_mc_handle_ce);
>>
>> -void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
>> -{
>> - if (edac_mc_get_log_ce())
>> - edac_mc_printk(mci, KERN_WARNING,
>> - "CE - no information available: %s\n", msg);
>> + for (i = 0; i < mci->n_layers; i++) {
>> + if (pos[i] < 0)
>> + break;
>> + index += pos[i];
>> + mci->ue_per_layer[i][index]++;
>>
>> - mci->ce_noinfo_count++;
>> - mci->ce_count++;
>> + if (i < mci->n_layers - 1)
>> + index *= mci->layers[i + 1].size;
>> + }
>> }
>> -EXPORT_SYMBOL_GPL(edac_mc_handle_ce_no_info);
>>
>> -void edac_mc_handle_ue(struct mem_ctl_info *mci,
>> - unsigned long page_frame_number,
>> - unsigned long offset_in_page, int row, const char *msg)
>> +#define OTHER_LABEL " or "
>> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
>> + struct mem_ctl_info *mci,
>> + const unsigned long page_frame_number,
>> + const unsigned long offset_in_page,
>> + const unsigned long syndrome,
>> + const int layer0,
>> + const int layer1,
>> + const int layer2,
>
> Instead of passing each layer as an arg, you can prepare the array pos[]
> in each edac_mc_hanlde_*() and pass around a pointer to it - you need it
> anyway in the edac_mc_inc*() functions.
Yes, but the changes at the drivers will be more complex, without any reason:
before each call to this function, they would need to create and fill a temporary
array.
As there are only 3 layers, in the worse case, this way is simpler and more
efficient. We can review it, if we ever need more than 3 layers.
>
>> + const char *msg,
>> + const char *other_detail,
>> + const void *mcelog)
>
> Also, this function is huuuge and begs to be splitted into small,
> self-contained helpers.
broken into a few more helper functions.
>
>> {
>> - int len = EDAC_MC_LABEL_LEN * 4;
>> - char labels[len + 1];
>> - char *pos = labels;
>> - int chan;
>> - int chars;
>> - char *label = NULL;
>> + unsigned long remapped_page;
>> + /* FIXME: too much for stack: move it to some pre-alocated area */
>> + char detail[80], location[80];
>> + char label[(EDAC_MC_LABEL_LEN + 1 + sizeof(OTHER_LABEL)) * mci->tot_dimms];
>> + char *p;
>> + int row = -1, chan = -1;
>> + int pos[EDAC_MAX_LAYERS] = { layer0, layer1, layer2 };
>> + int i;
>> u32 grain;
>> + bool enable_filter = false;
>
> What does this enable_filter thing mean:
>
> if (pos[i] >= 0)
> enable_filter = true;
>
> This absolutely needs explanation and better naming!
Renamed it to "enable_per_layer_report".
The code that implement it seems self-explained:
..
if (enable_filter && dimm->nr_pages) {
if (p != label) {
strcpy(p, OTHER_LABEL);
p += strlen(OTHER_LABEL);
}
strcpy(p, dimm->label);
p += strlen(p);
*p = '\0';
..
if (!enable_filter) {
strcpy(label, "any memory");
} else {
debugf4("%s: csrow/channel to increment: (%d,%d)\n",
__func__, row, chan);
if (p == label)
strcpy(label, "unknown memory");
if (type == HW_EVENT_ERR_CORRECTED) {
if (row >= 0) {
mci->csrows[row].ce_count++;
if (chan >= 0)
mci->csrows[row].channels[chan].ce_count++;
}
} else
if (row >= 0)
mci->csrows[row].ue_count++;
}
Theis flag indicates if is there any useful information about the affected
DIMM(s) provided by the EDAC driver. If this is provided, the DIMM location labels are
filtered and reported, and the per-layer error counters are incremented.
As it was discussed on previous reviews, with FB-DIMM MCs, and/or when mirror
mode/lockstep mode is enabled, the memory controller points errors to 2 DIMMs
(or 4 DIMMs, when both mirror mode and lockstep mode are enabled) on most memory
controllers, under some conditions. The edac_mc_handle_fbd_ue() function call were
created due to that.
When comparing with the old code, "enable_filter = false" would be equivalent to call
edac_mc_handle_ce_no_info/edac_mc_handle_ue_no_info.
I'm adding a comment about it.
>
>>
>> debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
>>
>> - /* FIXME - maybe make panic on INTERNAL ERROR an option */
>> - if (row >= mci->nr_csrows || row < 0) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: row out of range "
>> - "(%d >= %d)\n", row, mci->nr_csrows);
>> - edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
>> - return;
>> - }
>> -
>> - grain = mci->csrows[row].channels[0].dimm->grain;
>> - label = mci->csrows[row].channels[0].dimm->label;
>> - chars = snprintf(pos, len + 1, "%s", label);
>> - len -= chars;
>> - pos += chars;
>> -
>> - for (chan = 1; (chan < mci->csrows[row].nr_channels) && (len > 0);
>> - chan++) {
>> - label = mci->csrows[row].channels[chan].dimm->label;
>> - chars = snprintf(pos, len + 1, ":%s", label);
>> - len -= chars;
>> - pos += chars;
>> + /* Check if the event report is consistent */
>> + for (i = 0; i < mci->n_layers; i++) {
>> + if (pos[i] >= (int)mci->layers[i].size) {
>> + if (type == HW_EVENT_ERR_CORRECTED) {
>> + p = "CE";
>> + mci->ce_mc++;
>> + } else {
>> + p = "UE";
>> + mci->ue_mc++;
>
> Ok, mci->ce_mc and mci->ue_mc are being incremented here and the same
> happens below again in the edac_inc*_{ce,ue}_error() functions below.
>
> Why? Current layer is within valid elements count of current layer?
Fixed. This is a bug introduced by one of the rebases: In the past, there
was two error counters out there, one for the legacy API, and another one
for the new API.
>
>> + }
>> + edac_mc_printk(mci, KERN_ERR,
>> + "INTERNAL ERROR: %s value is out of range (%d >= %d)\n",
>> + edac_layer_name[mci->layers[i].type],
>> + pos[i], mci->layers[i].size);
>> + /*
>> + * Instead of just returning it, let's use what's
>> + * known about the error. The increment routines and
>> + * the DIMM filter logic will do the right thing by
>> + * pointing the likely damaged DIMMs.
>> + */
>> + pos[i] = -1;
>> + }
>> + if (pos[i] >= 0)
>> + enable_filter = true;
>
> As above, what does that filter logic mean, where it is explained?
Se above.
>
>>
>> - if (edac_mc_get_log_ue())
>> - edac_mc_printk(mci, KERN_EMERG,
>> - "UE page 0x%lx, offset 0x%lx, grain %d, row %d, "
>> - "labels \"%s\": %s\n", page_frame_number,
>> - offset_in_page, grain, row, labels, msg);
>> -
>> - if (edac_mc_get_panic_on_ue())
>> - panic("EDAC MC%d: UE page 0x%lx, offset 0x%lx, grain %d, "
>> - "row %d, labels \"%s\": %s\n", mci->mc_idx,
>> - page_frame_number, offset_in_page,
>> - grain, row, labels, msg);
>> -
>> - mci->ue_count++;
>> - mci->csrows[row].ue_count++;
>> -}
>> -EXPORT_SYMBOL_GPL(edac_mc_handle_ue);
>> + /*
>> + * Get the dimm label/grain that applies to the match criteria.
>> + * As the error algorithm may not be able to point to just one memory,
>
> What exactly do you mean by "memory" here? DIMM, slot, rank? Please be
> more specific.
A memory stick. As I said above, and on a previous review, some chipkill
algorithms/logic may point to 2 or 4 memory sticks, depending if lockstep
mode and/or mirror mode is enabled.
Added "stick" to the comment.
>> + * the logic here will get all possible labels that could pottentially
>> + * be affected by the error.
>> + * On FB-DIMM memory controllers, for uncorrected errors, it is common
>> + * to have only the MC channel and the MC dimm (also called as "rank")
>
> remove "as"
Hmm... actually it should be, instead:
(also called "branch")
Fixed.
>
>> + * but the channel is not known, as the memory is arranged in pairs,
>> + * where each memory belongs to a separate channel within the same
>> + * branch.
>> + * It will also get the max grain, over the error match range
>> + */
>> + grain = 0;
>> + p = label;
>> + *p = '\0';
>> + for (i = 0; i < mci->tot_dimms; i++) {
>> + struct dimm_info *dimm = &mci->dimms[i];
>>
>> -void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
>> -{
>> - if (edac_mc_get_panic_on_ue())
>> - panic("EDAC MC%d: Uncorrected Error", mci->mc_idx);
>> + if (layer0 >= 0 && layer0 != dimm->location[0])
>> + continue;
>> + if (layer1 >= 0 && layer1 != dimm->location[1])
>> + continue;
>> + if (layer2 >= 0 && layer2 != dimm->location[2])
>> + continue;
>>
>> - if (edac_mc_get_log_ue())
>> - edac_mc_printk(mci, KERN_WARNING,
>> - "UE - no information available: %s\n", msg);
>> - mci->ue_noinfo_count++;
>> - mci->ue_count++;
>> -}
>> -EXPORT_SYMBOL_GPL(edac_mc_handle_ue_no_info);
>> + if (dimm->grain > grain)
>> + grain = dimm->grain;
>
> Pls move the "max grain" part of the comment over this lines so that one
> knows what happens.
Ok.
>
>>
>> -/*************************************************************
>> - * On Fully Buffered DIMM modules, this help function is
>> - * called to process UE events
>> - */
>> -void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
>> - unsigned int csrow,
>> - unsigned int channela,
>> - unsigned int channelb, char *msg)
>> -{
>> - int len = EDAC_MC_LABEL_LEN * 4;
>> - char labels[len + 1];
>> - char *pos = labels;
>> - int chars;
>> - char *label;
>> -
>> - if (csrow >= mci->nr_csrows) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: row out of range (%d >= %d)\n",
>> - csrow, mci->nr_csrows);
>> - edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
>> - return;
>> + /*
>> + * If the error is memory-controller wide, there's no sense
>> + * on seeking for the affected DIMMs, as everything may be
>
> "there's no need to seek for the affected DIMMs because the whole
> channel/memory controller/... may be affected"
>
>> + * affected. Also, don't show errors for non-filled dimm's.
>
> for empty DIMM slots.
>
>> + */
>> + if (enable_filter && dimm->nr_pages) {
>> + if (p != label) {
>> + strcpy(p, OTHER_LABEL);
>> + p += strlen(OTHER_LABEL);
>> + }
>> + strcpy(p, dimm->label);
>> + p += strlen(p);
>> + *p = '\0';
>> +
>> + /*
>> + * get csrow/channel of the dimm, in order to allow
>
> DIMM
>
>> + * incrementing the compat API counters
>> + */
>> + debugf4("%s: %s csrows map: (%d,%d)\n",
>> + __func__,
>> + mci->mem_is_per_rank ? "rank" : "dimm",
>> + dimm->csrow, dimm->cschannel);
>
> newline
>
>> + if (row == -1)
>> + row = dimm->csrow;
>> + else if (row >= 0 && row != dimm->csrow)
>> + row = -2;
>
> ditto
>
>> + if (chan == -1)
>> + chan = dimm->cschannel;
>> + else if (chan >= 0 && chan != dimm->cschannel)
>> + chan = -2;
>> + }
>> }
>> -
>> - if (channela >= mci->csrows[csrow].nr_channels) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: channel-a out of range "
>> - "(%d >= %d)\n",
>> - channela, mci->csrows[csrow].nr_channels);
>> - edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
>> - return;
>
> newline here.
>
>> + if (!enable_filter) {
>> + strcpy(label, "any memory");
>> + } else {
>> + debugf4("%s: csrow/channel to increment: (%d,%d)\n",
>> + __func__, row, chan);
>> + if (p == label)
>> + strcpy(label, "unknown memory");
>> + if (type == HW_EVENT_ERR_CORRECTED) {
>> + if (row >= 0) {
>> + mci->csrows[row].ce_count++;
>> + if (chan >= 0)
>> + mci->csrows[row].channels[chan].ce_count++;
>> + }
>> + } else
>> + if (row >= 0)
>> + mci->csrows[row].ue_count++;
>> }
>>
>> - if (channelb >= mci->csrows[csrow].nr_channels) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: channel-b out of range "
>> - "(%d >= %d)\n",
>> - channelb, mci->csrows[csrow].nr_channels);
>> - edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
>> - return;
>> + /* Fill the RAM location data */
>> + p = location;
>> + for (i = 0; i < mci->n_layers; i++) {
>> + if (pos[i] < 0)
>> + continue;
>
> newline.
>
>> + p += sprintf(p, "%s %d ",
>> + edac_layer_name[mci->layers[i].type],
>> + pos[i]);
>> }
>>
>> - mci->ue_count++;
>> - mci->csrows[csrow].ue_count++;
>> -
>> - /* Generate the DIMM labels from the specified channels */
>> - label = mci->csrows[csrow].channels[channela].dimm->label;
>> - chars = snprintf(pos, len + 1, "%s", label);
>> - len -= chars;
>> - pos += chars;
>> -
>> - chars = snprintf(pos, len + 1, "-%s",
>> - mci->csrows[csrow].channels[channelb].dimm->label);
>> -
>> - if (edac_mc_get_log_ue())
>> - edac_mc_printk(mci, KERN_EMERG,
>> - "UE row %d, channel-a= %d channel-b= %d "
>> - "labels \"%s\": %s\n", csrow, channela, channelb,
>> - labels, msg);
>> -
>> - if (edac_mc_get_panic_on_ue())
>> - panic("UE row %d, channel-a= %d channel-b= %d "
>> - "labels \"%s\": %s\n", csrow, channela,
>> - channelb, labels, msg);
>> -}
>> -EXPORT_SYMBOL(edac_mc_handle_fbd_ue);
>> + /* Memory type dependent details about the error */
>> + if (type == HW_EVENT_ERR_CORRECTED)
>> + snprintf(detail, sizeof(detail),
>> + "page 0x%lx offset 0x%lx grain %d syndrome 0x%lx",
>> + page_frame_number, offset_in_page,
>> + grain, syndrome);
>> + else
>> + snprintf(detail, sizeof(detail),
>> + "page 0x%lx offset 0x%lx grain %d",
>> + page_frame_number, offset_in_page, grain);
>> +
>> + if (type == HW_EVENT_ERR_CORRECTED) {
>> + if (edac_mc_get_log_ce())
>> + edac_mc_printk(mci, KERN_WARNING,
>> + "CE %s on %s (%s%s %s)\n",
>> + msg, label, location,
>> + detail, other_detail);
>
> two back-to-back if-statements with the same conditional, pls merge
> them. Better yet, this edac_mc_handle_error() is huuge, pls split its
> functionality into well-abstracted helpers, for example one which deals
> with HW_EVENT_ERR_CORRECTED, another with HW_EVENT_ERR_UNCORRECTED, etc.
>
>> + edac_increment_ce_error(mci, enable_filter, pos);
>> +
>> + if (mci->scrub_mode & SCRUB_SW_SRC) {
>> + /*
>> + * Some MC's can remap memory so that it is still
>
> memory controllers (called MCs below)
>
>> + * available at a different address when PCI devices
>> + * map into memory.
>> + * MC's that can't do this lose the memory where PCI
>
> this, lose...
>
>> + * devices are mapped. This mapping is MC dependent
>
> MC-dependent
Fixed all above.
>
>> + * and so we call back into the MC driver for it to
>> + * map the MC page to a physical (CPU) page which can
>> + * then be mapped to a virtual page - which can then
>> + * be scrubbed.
>> + */
>> + remapped_page = mci->ctl_page_to_phys ?
>> + mci->ctl_page_to_phys(mci, page_frame_number) :
>> + page_frame_number;
>> +
>> + edac_mc_scrub_block(remapped_page,
>> + offset_in_page, grain);
>> + }
>
> The SCRUB_SW_SRC piece can be another function.
It is now part of the edac_ce_error().
>
>> + } else {
>> + if (edac_mc_get_log_ue())
>> + edac_mc_printk(mci, KERN_WARNING,
>> + "UE %s on %s (%s%s %s)\n",
>> + msg, label, location, detail, other_detail);
>>
>> -/*************************************************************
>> - * On Fully Buffered DIMM modules, this help function is
>> - * called to process CE events
>> - */
>> -void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
>> - unsigned int csrow, unsigned int channel, char *msg)
>> -{
>> - char *label = NULL;
>> + if (edac_mc_get_panic_on_ue())
>> + panic("UE %s on %s (%s%s %s)\n",
>> + msg, label, location, detail, other_detail);
>>
>> - /* Ensure boundary values */
>> - if (csrow >= mci->nr_csrows) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: row out of range (%d >= %d)\n",
>> - csrow, mci->nr_csrows);
>> - edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
>> - return;
>> + edac_increment_ue_error(mci, enable_filter, pos);
>> }
>> - if (channel >= mci->csrows[csrow].nr_channels) {
>> - /* something is wrong */
>> - edac_mc_printk(mci, KERN_ERR,
>> - "INTERNAL ERROR: channel out of range (%d >= %d)\n",
>> - channel, mci->csrows[csrow].nr_channels);
>> - edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
>> - return;
>> - }
>> -
>> - label = mci->csrows[csrow].channels[channel].dimm->label;
>> -
>> - if (edac_mc_get_log_ce())
>> - /* FIXME - put in DIMM location */
>> - edac_mc_printk(mci, KERN_WARNING,
>> - "CE row %d, channel %d, label \"%s\": %s\n",
>> - csrow, channel, label, msg);
>> -
>> - mci->ce_count++;
>> - mci->csrows[csrow].ce_count++;
>> - mci->csrows[csrow].channels[channel].dimm->ce_count++;
>> - mci->csrows[csrow].channels[channel].ce_count++;
>> }
>> -EXPORT_SYMBOL(edac_mc_handle_fbd_ce);
>> +EXPORT_SYMBOL_GPL(edac_mc_handle_error);
>> diff --git a/include/linux/edac.h b/include/linux/edac.h
>> index 3b8798d909da..2b66109bc301 100644
>> --- a/include/linux/edac.h
>> +++ b/include/linux/edac.h
>> @@ -412,18 +412,20 @@ struct edac_mc_layer {
>> /* FIXME: add the proper per-location error counts */
>> struct dimm_info {
>> char label[EDAC_MC_LABEL_LEN + 1]; /* DIMM label on motherboard */
>> - unsigned memory_controller;
>> - unsigned csrow;
>> - unsigned csrow_channel;
>> +
>> + /* Memory location data */
>> + unsigned location[EDAC_MAX_LAYERS];
>> +
>> + struct mem_ctl_info *mci; /* the parent */
>>
>> u32 grain; /* granularity of reported error in bytes */
>> enum dev_type dtype; /* memory device type */
>> enum mem_type mtype; /* memory dimm type */
>> enum edac_type edac_mode; /* EDAC mode for this dimm */
>>
>> - u32 nr_pages; /* number of pages in csrow */
>> + u32 nr_pages; /* number of pages on this dimm */
>>
>> - u32 ce_count; /* Correctable Errors for this dimm */
>> + unsigned csrow, cschannel; /* Points to the old API data */
>> };
>>
>> /**
>> @@ -443,9 +445,10 @@ struct dimm_info {
>> */
>> struct rank_info {
>> int chan_idx;
>> - u32 ce_count;
>> struct csrow_info *csrow;
>> struct dimm_info *dimm;
>> +
>> + u32 ce_count; /* Correctable Errors for this csrow */
>> };
>>
>> struct csrow_info {
>> @@ -497,6 +500,11 @@ struct mcidev_sysfs_attribute {
>> ssize_t (*store)(struct mem_ctl_info *, const char *,size_t);
>> };
>>
>> +struct edac_hierarchy {
>> + char *name;
>> + unsigned nr;
>> +};
>
> What is that, unused leftovers?
Likely. I'll drop from this patch, readding it latter if it is still needed.
>
>> +
>> /* MEMORY controller information structure
>> */
>> struct mem_ctl_info {
>> @@ -541,13 +549,18 @@ struct mem_ctl_info {
>> unsigned long (*ctl_page_to_phys) (struct mem_ctl_info * mci,
>> unsigned long page);
>> int mc_idx;
>> - int nr_csrows;
>> struct csrow_info *csrows;
>> + unsigned nr_csrows, num_cschannel;
>> +
>> + /* Memory Controller hierarchy */
>> + unsigned n_layers;
>> + struct edac_mc_layer *layers;
>> + bool mem_is_per_rank;
>>
>> /*
>> * DIMM info. Will eventually remove the entire csrows_info some day
>> */
>> - unsigned nr_dimms;
>> + unsigned tot_dimms;
>> struct dimm_info *dimms;
>>
>> /*
>> @@ -562,12 +575,15 @@ struct mem_ctl_info {
>> const char *dev_name;
>> char proc_name[MC_PROC_NAME_MAX_LEN + 1];
>> void *pvt_info;
>> - u32 ue_noinfo_count; /* Uncorrectable Errors w/o info */
>> - u32 ce_noinfo_count; /* Correctable Errors w/o info */
>> - u32 ue_count; /* Total Uncorrectable Errors for this MC */
>> - u32 ce_count; /* Total Correctable Errors for this MC */
>> + u32 ue_count; /* Total Uncorrectable Errors for this MC */
>> + u32 ce_count; /* Total Correctable Errors for this MC */a
>
> Why are you touching ue_count and ce_count, I don't see any differences
> here?
This is also due to some rebase. Moved the error counters to be together with
the other ones.
>> unsigned long start_time; /* mci load start time (in jiffies) */
>>
>> + /* drivers shouldn't access this struct directly */
>
> Which struct, I only see unsigneds?
I mean those fields. Some drivers were touching at them directly in the past.
>
>> + unsigned ce_noinfo_count, ue_noinfo_count;
>> + unsigned ce_mc, ue_mc;
>
> What are those counters?
In the past, I was keeping ce_count/ue_count for the old API, and ce_mc/ue_mc
for the new API. This got merged on rebase v13 or v14.
Got rid of those.
>
>> + u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
>> +
>> struct completion complete;
>>
>> /* edac sysfs device control */
>> @@ -580,7 +596,7 @@ struct mem_ctl_info {
>> * by the low level driver.
>> *
>> * Set by the low level driver to provide attributes at the
>> - * controller level, same level as 'ue_count' and 'ce_count' above.
>> + * controller level.
>> * An array of structures, NULL terminated
>> *
>> * If attributes are desired, then set to array of attributes
>
The following patch addresses the pointed issues. I've updated them
on my experimental branch at infradead:
git://git.infradead.org/users/mchehab/edac.git experimental
They'll also be soon available at:
git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-edac.git hw_events_v18
Regards,
Mauro
-
edac: Change internal representation to work with layers
From: Mauro Carvalho Chehab <mchehab@redhat.com>
Change the EDAC internal representation to work with non-csrow
based memory controllers.
There are lots of those memory controllers nowadays, and more
are coming. So, the EDAC internal representation needs to be
changed, in order to work with those memory controllers, while
preserving backward compatibility with the old ones.
The edac core was written with the idea that memory controllers
are able to directly access csrows.
This is not true for FB-DIMM and RAMBUS memory controllers.
Also, some recent advanced memory controllers don't present a per-csrows
view. Instead, they view memories as DIMMs, instead of ranks.
So, change the allocation and error report routines to allow
them to work with all types of architectures.
This will allow the removal of several hacks with FB-DIMM and RAMBUS
memory controllers.
Also, several tests were done on different platforms using different
x86 drivers.
TODO: a multi-rank DIMMs are currently represented by multiple DIMM
entries in struct dimm_info. That means that changing a label for one
rank won't change the same label for the other ranks at the same DIMM.
This bug is present since the beginning of the EDAC, so it is not a big
deal. However, on several drivers, it is possible to fix this issue, but
it should be a per-driver fix, as the csrow => DIMM arrangement may not
be equal for all. So, don't try to fix it here yet.
I tried to make this patch as short as possible, preceding it with
several other patches that simplified the logic here. Yet, as the
internal API changes, all drivers need changes. The changes are
generally bigger in the drivers for FB-DIMMs.
Cc: Aristeu Rozanski <arozansk@redhat.com>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Borislav Petkov <borislav.petkov@amd.com>
Cc: Mark Gross <mark.gross@intel.com>
Cc: Jason Uhlenkott <juhlenko@akamai.com>
Cc: Tim Small <tim@buttersideup.com>
Cc: Ranganathan Desikan <ravi@jetztechnologies.com>
Cc: "Arvind R." <arvino55@gmail.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Egor Martovetsky <egor@pasemi.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joe Perches <joe@perches.com>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Hitoshi Mitake <h.mitake@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Niklas Söderlund" <niklas.soderlund@ericsson.com>
Cc: Shaohui Xie <Shaohui.Xie@freescale.com>
Cc: Josh Boyer <jwboyer@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
v18: Addresses the pointed issues on v17 review
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index e48ab31..1286c5e 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -447,8 +447,12 @@ static inline void pci_write_bits32(struct pci_dev *pdev, int offset,
#endif /* CONFIG_PCI */
-extern struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index);
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int edac_index);
+struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt);
extern int edac_mc_add_mc(struct mem_ctl_info *mci);
extern void edac_mc_free(struct mem_ctl_info *mci);
extern struct mem_ctl_info *edac_mc_find(int idx);
@@ -467,24 +471,78 @@ extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
* reporting logic and function interface - reduces conditional
* statement clutter and extra function arguments.
*/
-extern void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page,
- unsigned long syndrome, int row, int channel,
- const char *msg);
-extern void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row,
- const char *msg);
-extern void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
- const char *msg);
-extern void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel0, unsigned int channel1,
- char *msg);
-extern void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci, unsigned int csrow,
- unsigned int channel, char *msg);
+
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog);
+
+static inline void edac_mc_handle_ce(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page,
+ unsigned long syndrome, int row, int channel,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ page_frame_number, offset_in_page, syndrome,
+ row, channel, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue(struct mem_ctl_info *mci,
+ unsigned long page_frame_number,
+ unsigned long offset_in_page, int row,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ page_frame_number, offset_in_page, 0,
+ row, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci,
+ const char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0, -1, -1, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel0,
+ unsigned int channel1,
+ char *msg)
+{
+ /*
+ *FIXME: The error can also be at channel1 (e. g. at the second
+ * channel of the same branch). The fix is to push
+ * edac_mc_handle_error() call into each driver
+ */
+ edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel0, -1, msg, NULL, NULL);
+}
+
+static inline void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
+ unsigned int csrow,
+ unsigned int channel, char *msg)
+{
+ edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
+ 0, 0, 0,
+ csrow, channel, -1, msg, NULL, NULL);
+}
/*
* edac_device APIs
@@ -496,6 +554,7 @@ extern void edac_device_handle_ue(struct edac_device_ctl_info *edac_dev,
extern void edac_device_handle_ce(struct edac_device_ctl_info *edac_dev,
int inst_nr, int block_nr, const char *msg);
extern int edac_device_alloc_index(void);
+extern const char *edac_layer_name[];
/*
* edac_pci APIs
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6ec967a..10cebb8 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -44,9 +44,25 @@ static void edac_mc_dump_channel(struct rank_info *chan)
debugf4("\tchannel = %p\n", chan);
debugf4("\tchannel->chan_idx = %d\n", chan->chan_idx);
debugf4("\tchannel->csrow = %p\n\n", chan->csrow);
- debugf4("\tdimm->ce_count = %d\n", chan->dimm->ce_count);
- debugf4("\tdimm->label = '%s'\n", chan->dimm->label);
- debugf4("\tdimm->nr_pages = 0x%x\n", chan->dimm->nr_pages);
+ debugf4("\tchannel->dimm = %p\n", chan->dimm);
+}
+
+static void edac_mc_dump_dimm(struct dimm_info *dimm)
+{
+ int i;
+
+ debugf4("\tdimm = %p\n", dimm);
+ debugf4("\tdimm->label = '%s'\n", dimm->label);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
+ debugf4("\tdimm location ");
+ for (i = 0; i < dimm->mci->n_layers; i++) {
+ printk(KERN_CONT "%d", dimm->location[i]);
+ if (i < dimm->mci->n_layers - 1)
+ printk(KERN_CONT ".");
+ }
+ printk(KERN_CONT "\n");
+ debugf4("\tdimm->grain = %d\n", dimm->grain);
+ debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
}
static void edac_mc_dump_csrow(struct csrow_info *csrow)
@@ -70,6 +86,8 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci)
debugf4("\tmci->edac_check = %p\n", mci->edac_check);
debugf3("\tmci->nr_csrows = %d, csrows = %p\n",
mci->nr_csrows, mci->csrows);
+ debugf3("\tmci->nr_dimms = %d, dimms = %p\n",
+ mci->tot_dimms, mci->dimms);
debugf3("\tdev = %p\n", mci->dev);
debugf3("\tmod_name:ctl_name = %s:%s\n", mci->mod_name, mci->ctl_name);
debugf3("\tpvt_info = %p\n\n", mci->pvt_info);
@@ -157,10 +175,20 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
}
/**
- * edac_mc_alloc: Allocate a struct mem_ctl_info structure
- * @size_pvt: size of private storage needed
- * @nr_csrows: Number of CWROWS needed for this MC
- * @nr_chans: Number of channels for the MC
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of MC hierarchy layers
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: size of private storage needed
+ *
+ *
+ * Non-csrow based drivers (like FB-DIMM and RAMBUS ones) will likely report
+ * such DIMMS properly, but the CSROWS-based ones will likely do the wrong
+ * thing, as two chip select values are used for dual-rank memories (and 4, for
+ * quad-rank ones). I suspect that this issue could be solved inside the EDAC
+ * core for SDRAM memories, but it requires further study at JEDEC JESD 21C.
+ *
+ * In summary, solving this issue is not easy, as it requires a lot of testing.
*
* Everything is kmalloc'ed as one big chunk - more efficient.
* Only can be used if all structures have the same lifetime - otherwise
@@ -168,22 +196,49 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
*
* Use edac_mc_free() to free mc structures allocated by this function.
*
+ * NOTE: drivers handle multi-rank memories in different ways: in some
+ * drivers, one multi-rank memory stick is mapped as one entry, while, in
+ * others, a single multi-rank memory stick would be mapped into several
+ * entries. Currently, this function will allocate multiple struct dimm_info
+ * on such scenarios, as grouping the multiple ranks require drivers change.
+ *
* Returns:
* NULL allocation failed
* struct mem_ctl_info pointer
*/
-struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
- unsigned nr_chans, int edac_index)
+struct mem_ctl_info *new_edac_mc_alloc(unsigned mc_num,
+ unsigned n_layers,
+ struct edac_mc_layer *layers,
+ unsigned sz_pvt)
{
- void *ptr = NULL;
struct mem_ctl_info *mci;
- struct csrow_info *csi, *csrow;
+ struct edac_mc_layer *layer;
+ struct csrow_info *csi, *csr;
struct rank_info *chi, *chp, *chan;
struct dimm_info *dimm;
- void *pvt;
- unsigned size;
- int row, chn;
- int err;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+ unsigned pos[EDAC_MAX_LAYERS];
+ void *pvt, *ptr = NULL;
+ unsigned size, tot_dimms = 1, count = 1;
+ unsigned tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
+ int i, j, err, row, chn;
+ bool per_rank = false;
+
+ BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
+ /*
+ * Calculate the total amount of dimms and csrows/cschannels while
+ * in the old API emulation mode
+ */
+ for (i = 0; i < n_layers; i++) {
+ tot_dimms *= layers[i].size;
+ if (layers[i].is_virt_csrow)
+ tot_csrows *= layers[i].size;
+ else
+ tot_channels *= layers[i].size;
+
+ if (layers[i].type == EDAC_MC_LAYER_CHIP_SELECT)
+ per_rank = true;
+ }
/* Figure out the offsets of the various items from the start of an mc
* structure. We want the alignment of each item to be at least as
@@ -191,12 +246,27 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
* hardcode everything into a single struct.
*/
mci = edac_align_ptr(&ptr, sizeof(*mci), 1);
- csi = edac_align_ptr(&ptr, sizeof(*csi), nr_csrows);
- chi = edac_align_ptr(&ptr, sizeof(*chi), nr_csrows * nr_chans);
- dimm = edac_align_ptr(&ptr, sizeof(*dimm), nr_csrows * nr_chans);
+ layer = edac_align_ptr(&ptr, sizeof(*layer), n_layers);
+ csi = edac_align_ptr(&ptr, sizeof(*csi), tot_csrows);
+ chi = edac_align_ptr(&ptr, sizeof(*chi), tot_csrows * tot_channels);
+ dimm = edac_align_ptr(&ptr, sizeof(*dimm), tot_dimms);
+ for (i = 0; i < n_layers; i++) {
+ count *= layers[i].size;
+ debugf4("%s: errcount layer %d size %d\n", __func__, i, count);
+ ce_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ ue_per_layer[i] = edac_align_ptr(&ptr, sizeof(u32), count);
+ tot_errcount += 2 * count;
+ }
+
+ debugf4("%s: allocating %d error counters\n", __func__, tot_errcount);
pvt = edac_align_ptr(&ptr, sz_pvt, 1);
size = ((unsigned long)pvt) + sz_pvt;
+ debugf1("%s(): allocating %u bytes for mci data (%d %s, %d csrows/channels)\n",
+ __func__, size,
+ tot_dimms,
+ per_rank ? "ranks" : "dimms",
+ tot_csrows * tot_channels);
mci = kzalloc(size, GFP_KERNEL);
if (mci == NULL)
return NULL;
@@ -204,42 +274,90 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
/* Adjust pointers so they point within the memory we just allocated
* rather than an imaginary chunk of memory located at address 0.
*/
+ layer = (struct edac_mc_layer *)(((char *)mci) + ((unsigned long)layer));
csi = (struct csrow_info *)(((char *)mci) + ((unsigned long)csi));
chi = (struct rank_info *)(((char *)mci) + ((unsigned long)chi));
dimm = (struct dimm_info *)(((char *)mci) + ((unsigned long)dimm));
+ for (i = 0; i < n_layers; i++) {
+ mci->ce_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ce_per_layer[i]));
+ mci->ue_per_layer[i] = (u32 *)((char *)mci + ((unsigned long)ue_per_layer[i]));
+ }
pvt = sz_pvt ? (((char *)mci) + ((unsigned long)pvt)) : NULL;
/* setup index and various internal pointers */
- mci->mc_idx = edac_index;
+ mci->mc_idx = mc_num;
mci->csrows = csi;
mci->dimms = dimm;
+ mci->tot_dimms = tot_dimms;
mci->pvt_info = pvt;
- mci->nr_csrows = nr_csrows;
+ mci->n_layers = n_layers;
+ mci->layers = layer;
+ memcpy(mci->layers, layers, sizeof(*layer) * n_layers);
+ mci->nr_csrows = tot_csrows;
+ mci->num_cschannel = tot_channels;
+ mci->mem_is_per_rank = per_rank;
/*
- * For now, assumes that a per-csrow arrangement for dimms.
- * This will be latter changed.
+ * Fill the csrow struct
*/
- dimm = mci->dimms;
-
- for (row = 0; row < nr_csrows; row++) {
- csrow = &csi[row];
- csrow->csrow_idx = row;
- csrow->mci = mci;
- csrow->nr_channels = nr_chans;
- chp = &chi[row * nr_chans];
- csrow->channels = chp;
-
- for (chn = 0; chn < nr_chans; chn++) {
+ for (row = 0; row < tot_csrows; row++) {
+ csr = &csi[row];
+ csr->csrow_idx = row;
+ csr->mci = mci;
+ csr->nr_channels = tot_channels;
+ chp = &chi[row * tot_channels];
+ csr->channels = chp;
+
+ for (chn = 0; chn < tot_channels; chn++) {
chan = &chp[chn];
chan->chan_idx = chn;
- chan->csrow = csrow;
+ chan->csrow = csr;
+ }
+ }
- mci->csrows[row].channels[chn].dimm = dimm;
- dimm->csrow = row;
- dimm->csrow_channel = chn;
- dimm++;
- mci->nr_dimms++;
+ /*
+ * Fill the dimm struct
+ */
+ memset(&pos, 0, sizeof(pos));
+ row = 0;
+ chn = 0;
+ debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
+ per_rank ? "ranks" : "dimms");
+ for (i = 0; i < tot_dimms; i++) {
+ chan = &csi[row].channels[chn];
+ dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
+ pos[0], pos[1], pos[2]);
+ dimm->mci = mci;
+
+ debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
+ i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
+ pos[0], pos[1], pos[2], row, chn);
+
+ /* Copy DIMM location */
+ for (j = 0; j < n_layers; j++)
+ dimm->location[j] = pos[j];
+
+ /* Link it to the csrows old API data */
+ chan->dimm = dimm;
+ dimm->csrow = row;
+ dimm->cschannel = chn;
+
+ /* Increment csrow location */
+ for (j = n_layers - 1; j >= 0; j--)
+ if (layers[j].is_virt_csrow)
+ break;
+ row++;
+ if (row == tot_csrows) {
+ row = 0;
+ chn++;
+ }
+
+ /* Increment dimm location */
+ for (j = n_layers - 1; j >= 0; j--) {
+ pos[j]++;
+ if (pos[j] < layers[j].size)
+ break;
+ pos[j] = 0;
}
}
@@ -263,6 +381,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
*/
return mci;
}
+EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
+
+/**
+ * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
+ * @mc_num: Memory controller number
+ * @n_layers: Number of layers at the MC hierarchy
+ * layers: Describes each layer as seen by the Memory Controller
+ * @size_pvt: Size of private storage needed
+ *
+ *
+ * FIXME: drivers handle multi-rank memories in different ways: some
+ * drivers map multi-ranked DIMMs as one DIMM while others
+ * as several DIMMs.
+ *
+ * Everything is kmalloc'ed as one big chunk - more efficient.
+ * It can only be used if all structures have the same lifetime - otherwise
+ * you have to allocate and initialize your own structures.
+ *
+ * Use edac_mc_free() to free mc structures allocated by this function.
+ *
+ * Returns:
+ * On failure: NULL
+ * On success: struct mem_ctl_info pointer
+ */
+
+struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
+ unsigned nr_chans, int mc_num)
+{
+ unsigned n_layers = 2;
+ struct edac_mc_layer layers[n_layers];
+
+ layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
+ layers[0].size = nr_csrows;
+ layers[0].is_virt_csrow = true;
+ layers[1].type = EDAC_MC_LAYER_CHANNEL;
+ layers[1].size = nr_chans;
+ layers[1].is_virt_csrow = false;
+
+ return new_edac_mc_alloc(mc_num, ARRAY_SIZE(layers), layers, sz_pvt);
+}
EXPORT_SYMBOL_GPL(edac_mc_alloc);
/**
@@ -528,7 +686,6 @@ EXPORT_SYMBOL(edac_mc_find);
* edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
* create sysfs entries associated with mci structure
* @mci: pointer to the mci structure to be added to the list
- * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
*
* Return:
* 0 Success
@@ -555,6 +712,8 @@ int edac_mc_add_mc(struct mem_ctl_info *mci)
edac_mc_dump_channel(&mci->csrows[i].
channels[j]);
}
+ for (i = 0; i < mci->tot_dimms; i++)
+ edac_mc_dump_dimm(&mci->dimms[i]);
}
#endif
mutex_lock(&mem_ctls_mutex);
@@ -712,261 +871,289 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
}
EXPORT_SYMBOL_GPL(edac_mc_find_csrow_by_page);
-/* FIXME - setable log (warning/emerg) levels */
-/* FIXME - integrate with evlog: http://evlog.sourceforge.net/ */
-void edac_mc_handle_ce(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, unsigned long syndrome,
- int row, int channel, const char *msg)
+const char *edac_layer_name[] = {
+ [EDAC_MC_LAYER_BRANCH] = "branch",
+ [EDAC_MC_LAYER_CHANNEL] = "channel",
+ [EDAC_MC_LAYER_SLOT] = "slot",
+ [EDAC_MC_LAYER_CHIP_SELECT] = "csrow",
+};
+EXPORT_SYMBOL_GPL(edac_layer_name);
+
+static void edac_inc_ce_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
{
- unsigned long remapped_page;
- char *label = NULL;
- u32 grain;
+ int i, index = 0;
- debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
+ mci->ce_count++;
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- if (channel >= mci->csrows[row].nr_channels || channel < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range "
- "(%d >= %d)\n", channel,
- mci->csrows[row].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ce_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_inc_ue_error(struct mem_ctl_info *mci,
+ bool enable_per_layer_report,
+ const int pos[EDAC_MAX_LAYERS])
+{
+ int i, index = 0;
+
+ mci->ue_count++;
+
+ if (!enable_per_layer_report) {
+ mci->ce_noinfo_count++;
return;
}
- label = mci->csrows[row].channels[channel].dimm->label;
- grain = mci->csrows[row].channels[channel].dimm->grain;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ break;
+ index += pos[i];
+ mci->ue_per_layer[i][index]++;
+
+ if (i < mci->n_layers - 1)
+ index *= mci->layers[i + 1].size;
+ }
+}
+
+static void edac_ce_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ u32 grain)
+{
+ unsigned long remapped_page;
if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
edac_mc_printk(mci, KERN_WARNING,
- "CE page 0x%lx, offset 0x%lx, grain %d, syndrome "
- "0x%lx, row %d, channel %d, label \"%s\": %s\n",
- page_frame_number, offset_in_page,
- grain, syndrome, row, channel,
- label, msg);
-
- mci->ce_count++;
- mci->csrows[row].ce_count++;
- mci->csrows[row].channels[channel].dimm->ce_count++;
- mci->csrows[row].channels[channel].ce_count++;
+ "CE %s on %s (%s%s %s)\n",
+ msg, label, location,
+ detail, other_detail);
+ edac_inc_ce_error(mci, enable_per_layer_report, pos);
if (mci->scrub_mode & SCRUB_SW_SRC) {
/*
- * Some MC's can remap memory so that it is still available
- * at a different address when PCI devices map into memory.
- * MC's that can't do this lose the memory where PCI devices
- * are mapped. This mapping is MC dependent and so we call
- * back into the MC driver for it to map the MC page to
- * a physical (CPU) page which can then be mapped to a virtual
- * page - which can then be scrubbed.
- */
+ * Some memory controllers (called MCs below) can remap
+ * memory so that it is still available at a different
+ * address when PCI devices map into memory.
+ * MC's that can't do this, lose the memory where PCI
+ * devices are mapped. This mapping is MC-dependent
+ * and so we call back into the MC driver for it to
+ * map the MC page to a physical (CPU) page which can
+ * then be mapped to a virtual page - which can then
+ * be scrubbed.
+ */
remapped_page = mci->ctl_page_to_phys ?
mci->ctl_page_to_phys(mci, page_frame_number) :
page_frame_number;
- edac_mc_scrub_block(remapped_page, offset_in_page, grain);
+ edac_mc_scrub_block(remapped_page,
+ offset_in_page, grain);
}
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce);
-void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
+static void edac_ue_error(struct mem_ctl_info *mci,
+ const int pos[EDAC_MAX_LAYERS],
+ const char *msg,
+ const char *location,
+ const char *label,
+ const char *detail,
+ const char *other_detail,
+ const bool enable_per_layer_report)
{
- if (edac_mc_get_log_ce())
+ if (edac_mc_get_log_ue())
edac_mc_printk(mci, KERN_WARNING,
- "CE - no information available: %s\n", msg);
+ "UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
- mci->ce_noinfo_count++;
- mci->ce_count++;
+ if (edac_mc_get_panic_on_ue())
+ panic("UE %s on %s (%s%s %s)\n",
+ msg, label, location, detail, other_detail);
+
+ edac_inc_ue_error(mci, enable_per_layer_report, pos);
}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ce_no_info);
-void edac_mc_handle_ue(struct mem_ctl_info *mci,
- unsigned long page_frame_number,
- unsigned long offset_in_page, int row, const char *msg)
+#define OTHER_LABEL " or "
+void edac_mc_handle_error(const enum hw_event_mc_err_type type,
+ struct mem_ctl_info *mci,
+ const unsigned long page_frame_number,
+ const unsigned long offset_in_page,
+ const unsigned long syndrome,
+ const int layer0,
+ const int layer1,
+ const int layer2,
+ const char *msg,
+ const char *other_detail,
+ const void *mcelog)
{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chan;
- int chars;
- char *label = NULL;
+ /* FIXME: too much for stack: move it to some pre-alocated area */
+ char detail[80], location[80];
+ char label[(EDAC_MC_LABEL_LEN + 1 + sizeof(OTHER_LABEL)) * mci->tot_dimms];
+ char *p;
+ int row = -1, chan = -1;
+ int pos[EDAC_MAX_LAYERS] = { layer0, layer1, layer2 };
+ int i;
u32 grain;
+ bool enable_per_layer_report = false;
debugf3("MC%d: %s()\n", mci->mc_idx, __func__);
- /* FIXME - maybe make panic on INTERNAL ERROR an option */
- if (row >= mci->nr_csrows || row < 0) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range "
- "(%d >= %d)\n", row, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
-
- grain = mci->csrows[row].channels[0].dimm->grain;
- label = mci->csrows[row].channels[0].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- for (chan = 1; (chan < mci->csrows[row].nr_channels) && (len > 0);
- chan++) {
- label = mci->csrows[row].channels[chan].dimm->label;
- chars = snprintf(pos, len + 1, ":%s", label);
- len -= chars;
- pos += chars;
+ /*
+ * Check if the event report is consistent and if the memory
+ * location is known. If it is known, enable_per_layer_report will be
+ * true, the DIMM(s) label info will be filled and the per-layer
+ * error counters will be incremented.
+ */
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] >= (int)mci->layers[i].size) {
+ if (type == HW_EVENT_ERR_CORRECTED)
+ p = "CE";
+ else
+ p = "UE";
+
+ edac_mc_printk(mci, KERN_ERR,
+ "INTERNAL ERROR: %s value is out of range (%d >= %d)\n",
+ edac_layer_name[mci->layers[i].type],
+ pos[i], mci->layers[i].size);
+ /*
+ * Instead of just returning it, let's use what's
+ * known about the error. The increment routines and
+ * the DIMM filter logic will do the right thing by
+ * pointing the likely damaged DIMMs.
+ */
+ pos[i] = -1;
+ }
+ if (pos[i] >= 0)
+ enable_per_layer_report = true;
}
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE page 0x%lx, offset 0x%lx, grain %d, row %d, "
- "labels \"%s\": %s\n", page_frame_number,
- offset_in_page, grain, row, labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: UE page 0x%lx, offset 0x%lx, grain %d, "
- "row %d, labels \"%s\": %s\n", mci->mc_idx,
- page_frame_number, offset_in_page,
- grain, row, labels, msg);
-
- mci->ue_count++;
- mci->csrows[row].ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue);
-
-void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
-{
- if (edac_mc_get_panic_on_ue())
- panic("EDAC MC%d: Uncorrected Error", mci->mc_idx);
+ /*
+ * Get the dimm label/grain that applies to the match criteria.
+ * As the error algorithm may not be able to point to just one memory
+ * stick, the logic here will get all possible labels that could
+ * pottentially be affected by the error.
+ * On FB-DIMM memory controllers, for uncorrected errors, it is common
+ * to have only the MC channel and the MC dimm (also called "branch")
+ * but the channel is not known, as the memory is arranged in pairs,
+ * where each memory belongs to a separate channel within the same
+ * branch.
+ */
+ grain = 0;
+ p = label;
+ *p = '\0';
+ for (i = 0; i < mci->tot_dimms; i++) {
+ struct dimm_info *dimm = &mci->dimms[i];
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_WARNING,
- "UE - no information available: %s\n", msg);
- mci->ue_noinfo_count++;
- mci->ue_count++;
-}
-EXPORT_SYMBOL_GPL(edac_mc_handle_ue_no_info);
+ if (layer0 >= 0 && layer0 != dimm->location[0])
+ continue;
+ if (layer1 >= 0 && layer1 != dimm->location[1])
+ continue;
+ if (layer2 >= 0 && layer2 != dimm->location[2])
+ continue;
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process UE events
- */
-void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
- unsigned int csrow,
- unsigned int channela,
- unsigned int channelb, char *msg)
-{
- int len = EDAC_MC_LABEL_LEN * 4;
- char labels[len + 1];
- char *pos = labels;
- int chars;
- char *label;
-
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
- }
+ /* get the max grain, over the error match range */
+ if (dimm->grain > grain)
+ grain = dimm->grain;
- if (channela >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-a out of range "
- "(%d >= %d)\n",
- channela, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ /*
+ * If the error is memory-controller wide, there's no need to
+ * seek for the affected DIMMs because the whole
+ * channel/memory controller/... may be affected.
+ * Also, don't show errors for empty DIMM slots.
+ */
+ if (enable_per_layer_report && dimm->nr_pages) {
+ if (p != label) {
+ strcpy(p, OTHER_LABEL);
+ p += strlen(OTHER_LABEL);
+ }
+ strcpy(p, dimm->label);
+ p += strlen(p);
+ *p = '\0';
+
+ /*
+ * get csrow/channel of the DIMM, in order to allow
+ * incrementing the compat API counters
+ */
+ debugf4("%s: %s csrows map: (%d,%d)\n",
+ __func__,
+ mci->mem_is_per_rank ? "rank" : "dimm",
+ dimm->csrow, dimm->cschannel);
+
+ if (row == -1)
+ row = dimm->csrow;
+ else if (row >= 0 && row != dimm->csrow)
+ row = -2;
+
+ if (chan == -1)
+ chan = dimm->cschannel;
+ else if (chan >= 0 && chan != dimm->cschannel)
+ chan = -2;
+ }
}
- if (channelb >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel-b out of range "
- "(%d >= %d)\n",
- channelb, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ue_no_info(mci, "INTERNAL ERROR");
- return;
+ if (!enable_per_layer_report) {
+ strcpy(label, "any memory");
+ } else {
+ debugf4("%s: csrow/channel to increment: (%d,%d)\n",
+ __func__, row, chan);
+ if (p == label)
+ strcpy(label, "unknown memory");
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ if (row >= 0) {
+ mci->csrows[row].ce_count++;
+ if (chan >= 0)
+ mci->csrows[row].channels[chan].ce_count++;
+ }
+ } else
+ if (row >= 0)
+ mci->csrows[row].ue_count++;
}
- mci->ue_count++;
- mci->csrows[csrow].ue_count++;
-
- /* Generate the DIMM labels from the specified channels */
- label = mci->csrows[csrow].channels[channela].dimm->label;
- chars = snprintf(pos, len + 1, "%s", label);
- len -= chars;
- pos += chars;
-
- chars = snprintf(pos, len + 1, "-%s",
- mci->csrows[csrow].channels[channelb].dimm->label);
-
- if (edac_mc_get_log_ue())
- edac_mc_printk(mci, KERN_EMERG,
- "UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela, channelb,
- labels, msg);
-
- if (edac_mc_get_panic_on_ue())
- panic("UE row %d, channel-a= %d channel-b= %d "
- "labels \"%s\": %s\n", csrow, channela,
- channelb, labels, msg);
-}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ue);
-
-/*************************************************************
- * On Fully Buffered DIMM modules, this help function is
- * called to process CE events
- */
-void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
- unsigned int csrow, unsigned int channel, char *msg)
-{
- char *label = NULL;
+ /* Fill the RAM location data */
+ p = location;
+ for (i = 0; i < mci->n_layers; i++) {
+ if (pos[i] < 0)
+ continue;
- /* Ensure boundary values */
- if (csrow >= mci->nr_csrows) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: row out of range (%d >= %d)\n",
- csrow, mci->nr_csrows);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
- }
- if (channel >= mci->csrows[csrow].nr_channels) {
- /* something is wrong */
- edac_mc_printk(mci, KERN_ERR,
- "INTERNAL ERROR: channel out of range (%d >= %d)\n",
- channel, mci->csrows[csrow].nr_channels);
- edac_mc_handle_ce_no_info(mci, "INTERNAL ERROR");
- return;
+ p += sprintf(p, "%s %d ",
+ edac_layer_name[mci->layers[i].type],
+ pos[i]);
}
- label = mci->csrows[csrow].channels[channel].dimm->label;
+ /* Memory type dependent details about the error */
+ if (type == HW_EVENT_ERR_CORRECTED) {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d syndrome 0x%lx",
+ page_frame_number, offset_in_page,
+ grain, syndrome);
- if (edac_mc_get_log_ce())
- /* FIXME - put in DIMM location */
- edac_mc_printk(mci, KERN_WARNING,
- "CE row %d, channel %d, label \"%s\": %s\n",
- csrow, channel, label, msg);
+ edac_ce_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report,
+ page_frame_number, offset_in_page, grain);
+ } else {
+ snprintf(detail, sizeof(detail),
+ "page 0x%lx offset 0x%lx grain %d",
+ page_frame_number, offset_in_page, grain);
- mci->ce_count++;
- mci->csrows[csrow].ce_count++;
- mci->csrows[csrow].channels[channel].dimm->ce_count++;
- mci->csrows[csrow].channels[channel].ce_count++;
+ edac_ue_error(mci, pos, msg, location, label, detail,
+ other_detail, enable_per_layer_report);
+ }
}
-EXPORT_SYMBOL(edac_mc_handle_fbd_ce);
+EXPORT_SYMBOL_GPL(edac_mc_handle_error);
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 3b8798d..c8f507d 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -412,18 +412,20 @@ struct edac_mc_layer {
/* FIXME: add the proper per-location error counts */
struct dimm_info {
char label[EDAC_MC_LABEL_LEN + 1]; /* DIMM label on motherboard */
- unsigned memory_controller;
- unsigned csrow;
- unsigned csrow_channel;
+
+ /* Memory location data */
+ unsigned location[EDAC_MAX_LAYERS];
+
+ struct mem_ctl_info *mci; /* the parent */
u32 grain; /* granularity of reported error in bytes */
enum dev_type dtype; /* memory device type */
enum mem_type mtype; /* memory dimm type */
enum edac_type edac_mode; /* EDAC mode for this dimm */
- u32 nr_pages; /* number of pages in csrow */
+ u32 nr_pages; /* number of pages on this dimm */
- u32 ce_count; /* Correctable Errors for this dimm */
+ unsigned csrow, cschannel; /* Points to the old API data */
};
/**
@@ -443,9 +445,10 @@ struct dimm_info {
*/
struct rank_info {
int chan_idx;
- u32 ce_count;
struct csrow_info *csrow;
struct dimm_info *dimm;
+
+ u32 ce_count; /* Correctable Errors for this csrow */
};
struct csrow_info {
@@ -541,13 +544,18 @@ struct mem_ctl_info {
unsigned long (*ctl_page_to_phys) (struct mem_ctl_info * mci,
unsigned long page);
int mc_idx;
- int nr_csrows;
struct csrow_info *csrows;
+ unsigned nr_csrows, num_cschannel;
+
+ /* Memory Controller hierarchy */
+ unsigned n_layers;
+ struct edac_mc_layer *layers;
+ bool mem_is_per_rank;
/*
* DIMM info. Will eventually remove the entire csrows_info some day
*/
- unsigned nr_dimms;
+ unsigned tot_dimms;
struct dimm_info *dimms;
/*
@@ -562,12 +570,16 @@ struct mem_ctl_info {
const char *dev_name;
char proc_name[MC_PROC_NAME_MAX_LEN + 1];
void *pvt_info;
- u32 ue_noinfo_count; /* Uncorrectable Errors w/o info */
- u32 ce_noinfo_count; /* Correctable Errors w/o info */
- u32 ue_count; /* Total Uncorrectable Errors for this MC */
- u32 ce_count; /* Total Correctable Errors for this MC */
unsigned long start_time; /* mci load start time (in jiffies) */
+ /*
+ * drivers shouldn't access those fields directly, as the core
+ * already handles that.
+ */
+ u32 ce_noinfo_count, ue_noinfo_count;
+ u32 ue_count, ce_count;
+ u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
+
struct completion complete;
/* edac sysfs device control */
@@ -580,7 +592,7 @@ struct mem_ctl_info {
* by the low level driver.
*
* Set by the low level driver to provide attributes at the
- * controller level, same level as 'ue_count' and 'ce_count' above.
+ * controller level.
* An array of structures, NULL terminated
*
* If attributes are desired, then set to array of attributes
^ permalink raw reply related
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