* Re: [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 12:06 UTC (permalink / raw)
To: Gavin Shan; +Cc: linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <20121228051824.GA9975@shangw.(null)>
On Fri, Dec 28, 2012 at 01:18:24PM +0800, Gavin Shan wrote:
> On Thu, Dec 27, 2012 at 02:34:00PM -0200, Thadeu Lima de Souza Cascardo wrote:
> >The DDW code uses a eeh_dev struct from the pci_dev. However, this is
> >not set until eeh_add_device_late is called.
> >
> >Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
> >devices are added to the bus, making drivers' probe hooks to be called.
> >These will call set_dma_mask, which will call the DDW code, which will
> >require the eeh_dev struct from pci_dev. This would result in a crash,
> >due to a NULL dereference.
> >
> >Calling eeh_add_device_late after pci_bus_add_devices would make the
> >system BUG, because device files shouldn't be added to devices there
> >were not added to the system. So, a new function is needed to add such
> >files only after pci_bus_add_devices have been called.
> >
>
> Could you please explain for a bit how did you trigger the problem? I'm
> not sure you got it while doing PCI hotplug or just saw the issue during
> system bootup stage :-)
>
I did a DLPAR remove followed by a DLPAR add, using drmgr -r and drmgr
-a. The reason the issue is not trigerred during bootup is that there is
no driver registered yet. So no driver probe will call dma_set_mask.
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> >---
> > arch/powerpc/include/asm/eeh.h | 3 +++
> > arch/powerpc/kernel/pci-common.c | 7 +++++--
> > arch/powerpc/platforms/pseries/eeh.c | 24 +++++++++++++++++++++++-
> > 3 files changed, 31 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> >index b0ef738..71aac19 100644
> >--- a/arch/powerpc/include/asm/eeh.h
> >+++ b/arch/powerpc/include/asm/eeh.h
> >@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
> > void __init eeh_addr_cache_build(void);
> > void eeh_add_device_tree_early(struct device_node *);
> > void eeh_add_device_tree_late(struct pci_bus *);
> >+void eeh_add_device_tree_files(struct pci_bus *);
>
> Since the function is going to add EEH specific sysfs files, its name would
> be something like "eeh_add_sysfs_files" instead of "eeh_add_device_tree_files" :-)
It's indeed a better name.
>
> > void eeh_remove_bus_device(struct pci_dev *, int);
> >
> > /**
> >@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
> >
> > static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
> >
> >+static inline void eeh_add_device_tree_files(struct pci_bus *bus) { }
> >+
>
> It'd better to rename the function name to "eeh_add_sysfs_files" mentioned
> as above.
>
> > static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
> >
> > static inline void eeh_lock(void) { }
> >diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> >index 7f94f76..7b1f14c 100644
> >--- a/arch/powerpc/kernel/pci-common.c
> >+++ b/arch/powerpc/kernel/pci-common.c
> >@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> > pcibios_allocate_bus_resources(bus);
> > pcibios_claim_one_bus(bus);
> >
> >+ /* Fixup EEH */
> >+ eeh_add_device_tree_late(bus);
> >+
> > /* Add new devices to global lists. Register in proc, sysfs. */
> > pci_bus_add_devices(bus);
> >
> >- /* Fixup EEH */
> >- eeh_add_device_tree_late(bus);
> >+ /* Add EEH sysfs files */
> >+ eeh_add_device_tree_files(bus);
>
> The function name would be "eeh_add_sysfs_files" as above.
>
> > }
> > EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
> >
>
> By the way, arch/powerpc/kernel/of_platform.c::of_pci_phb_probe is also calling
> to eeh_add_device_tree_late() as well. Since you have removed part of the logic
> from original eeh_add_device_tree_late(), which is add EEH specific sysfs files,
> and you put that part of logic to eeh_add_device_tree_files(). So I think you
> also need make the similiar change for of_pci_phb_probe() as well :-)
>
Good point. I will look into other call sites. However, I don't have
access to other platforms to test the patch.
> >diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
> >index 9a04322..a667a34 100644
> >--- a/arch/powerpc/platforms/pseries/eeh.c
> >+++ b/arch/powerpc/platforms/pseries/eeh.c
> >@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
> > dev->dev.archdata.edev = edev;
> >
> > eeh_addr_cache_insert_dev(dev);
> >- eeh_sysfs_add_device(dev);
> > }
> >
> > /**
> >@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
> > EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
> >
> > /**
> >+ * eeh_add_device_tree_files - Add EEH sysfs files for the indicated PCI bus
> >+ * @bus: PCI bus
> >+ *
> >+ * This routine must be used to add EEH sysfs files for PCI
> >+ * devices which are attached to the indicated PCI bus. The PCI bus
> >+ * is added after system boot through hotplug or dlpar.
> >+ */
> >+void eeh_add_device_tree_files(struct pci_bus *bus)
> >+{
> >+ struct pci_dev *dev;
> >+
> >+ list_for_each_entry(dev, &bus->devices, bus_list) {
> >+ eeh_sysfs_add_device(dev);
> >+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> >+ struct pci_bus *subbus = dev->subordinate;
> >+ if (subbus)
> >+ eeh_add_device_tree_files(subbus);
> >+ }
> >+ }
> >+}
> >+EXPORT_SYMBOL_GPL(eeh_add_device_tree_files);
> >+
>
> The function name mentioned as above.
>
> >+/**
> > * eeh_remove_device - Undo EEH setup for the indicated pci device
> > * @dev: pci device to be removed
> > * @purge_pe: remove the PE or not
> >
>
> Thanks,
> Gavin
>
Regards,
Thadeu Cascardo.
^ permalink raw reply
* Re: [PATCH] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 16:55 UTC (permalink / raw)
To: Gavin Shan; +Cc: paulus, linuxppc-dev, anton
In-Reply-To: <20121228052135.GB9975@shangw.(null)>
On Fri, Dec 28, 2012 at 01:21:35PM +0800, Gavin Shan wrote:
> On Thu, Dec 27, 2012 at 02:28:06PM -0200, Thadeu Lima de Souza Cascardo wrote:
> >When a device DMA window includes the address 0, it's reserved in the
> >TCE bitmap to avoid returning that address to drivers.
> >
> >When the device is removed, the bitmap is checked for any mappings not
> >removed by the driver, indicating a possible DMA mapping leak. Since the
> >reserved address is not cleared, a message is printed, warning of such a
> >leak.
> >
> >Check for the reservation, and clear it before checking for any other
> >standing mappings.
> >
> >Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> >---
> > arch/powerpc/kernel/iommu.c | 5 +++++
> > 1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> >index 8226c6c..226e9e5 100644
> >--- a/arch/powerpc/kernel/iommu.c
> >+++ b/arch/powerpc/kernel/iommu.c
> >@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> > return;
> > }
> >
> >+ /* In case we have reserved the first bit, we should not emit
> >+ * the warning below. */
>
> At least, the comment would look like:
>
> /*
> * xxxxxxx
> */
>
Sure. New code should always follow coding style. :-)
How do you suggest merging with the comment below? I think it's closer
to the code it comments about, so I'd rather keep it where it is.
> >+ if (tbl->it_offset == 0)
> >+ clear_bit(0, tbl->it_map);
> >+
> > /* verify that table contains no entries */
> > /* it_size is in entries, and we're examining 64 at a time */
>
> The comment would be merged as well? :-)
>
I also considered replacing this code by this:
/* verify that table contains no entries */
- /* it_size is in entries, and we're examining 64 at a time */
- for (i = 0; i < (tbl->it_size/64); i++) {
- if (tbl->it_map[i] != 0) {
+ if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
printk(KERN_WARNING "%s: Unexpected TCEs for
%s\n",
__func__, node_name);
- break;
- }
- }
I'll resend the "unreserving" patch with the fixed comment styling, but
without merging comments. And I will send this other patch for comments.
Regards.
Thadeu Cascardo.
> > for (i = 0; i < (tbl->it_size/64); i++) {
>
> Thanks,
> Gavin
^ permalink raw reply
* [PATCH 1/2] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
In-Reply-To: <20121228165504.GC21831@oc0268524204.ibm.com>
When a device DMA window includes the address 0, it's reserved in the
TCE bitmap to avoid returning that address to drivers.
When the device is removed, the bitmap is checked for any mappings not
removed by the driver, indicating a possible DMA mapping leak. Since the
reserved address is not cleared, a message is printed, warning of such a
leak.
Check for the reservation, and clear it before checking for any other
standing mappings.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/iommu.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 8226c6c..6d48ff8 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -717,6 +717,13 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
return;
}
+ /*
+ * In case we have reserved the first bit, we should not emit
+ * the warning below.
+ */
+ if (tbl->it_offset == 0)
+ clear_bit(0, tbl->it_map);
+
/* verify that table contains no entries */
/* it_size is in entries, and we're examining 64 at a time */
for (i = 0; i < (tbl->it_size/64); i++) {
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
In-Reply-To: <20121228165504.GC21831@oc0268524204.ibm.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/iommu.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 6d48ff8..91e2b99 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -725,14 +725,9 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
clear_bit(0, tbl->it_map);
/* verify that table contains no entries */
- /* it_size is in entries, and we're examining 64 at a time */
- for (i = 0; i < (tbl->it_size/64); i++) {
- if (tbl->it_map[i] != 0) {
+ if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
__func__, node_name);
- break;
- }
- }
/* calculate bitmap size in bytes */
bitmap_sz = (tbl->it_size + 7) / 8;
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:13 UTC (permalink / raw)
To: linuxppc-dev
Cc: shangw, linux-kernel, stable, paulus,
Thadeu Lima de Souza Cascardo, bhelgaas
In-Reply-To: <20121228120616.GB21831@oc0268524204.ibm.com>
The DDW code uses a eeh_dev struct from the pci_dev. However, this is
not set until eeh_add_device_late is called.
Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
devices are added to the bus, making drivers' probe hooks to be called.
These will call set_dma_mask, which will call the DDW code, which will
require the eeh_dev struct from pci_dev. This would result in a crash,
due to a NULL dereference.
Calling eeh_add_device_late after pci_bus_add_devices would make the
system BUG, because device files shouldn't be added to devices there
were not added to the system. So, a new function is needed to add such
files only after pci_bus_add_devices have been called.
Cc: stable@vger.kernel.org
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 3 +++
arch/powerpc/kernel/of_platform.c | 3 +++
arch/powerpc/kernel/pci-common.c | 7 +++++--
arch/powerpc/platforms/pseries/eeh.c | 24 +++++++++++++++++++++++-
4 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b0ef738..0f816da 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
void __init eeh_addr_cache_build(void);
void eeh_add_device_tree_early(struct device_node *);
void eeh_add_device_tree_late(struct pci_bus *);
+void eeh_add_sysfs_files(struct pci_bus *);
void eeh_remove_bus_device(struct pci_dev *, int);
/**
@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
+static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
+
static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
static inline void eeh_lock(void) { }
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index c5fc6b2..500dd32 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -91,6 +91,9 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
/* Add probed PCI devices to the device model */
pci_bus_add_devices(phb->bus);
+ /* sysfs files should only be added after devices are added */
+ eeh_add_sysfs_files(phb->bus);
+
return 0;
}
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f94f76..4d3de7e 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
pcibios_allocate_bus_resources(bus);
pcibios_claim_one_bus(bus);
+ /* Fixup EEH */
+ eeh_add_device_tree_late(bus);
+
/* Add new devices to global lists. Register in proc, sysfs. */
pci_bus_add_devices(bus);
- /* Fixup EEH */
- eeh_add_device_tree_late(bus);
+ /* sysfs files should only be added after devices are added */
+ eeh_add_sysfs_files(bus);
}
EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 9a04322..6b73d6c 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
dev->dev.archdata.edev = edev;
eeh_addr_cache_insert_dev(dev);
- eeh_sysfs_add_device(dev);
}
/**
@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
/**
+ * eeh_add_sysfs_files - Add EEH sysfs files for the indicated PCI bus
+ * @bus: PCI bus
+ *
+ * This routine must be used to add EEH sysfs files for PCI
+ * devices which are attached to the indicated PCI bus. The PCI bus
+ * is added after system boot through hotplug or dlpar.
+ */
+void eeh_add_sysfs_files(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ eeh_sysfs_add_device(dev);
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+ struct pci_bus *subbus = dev->subordinate;
+ if (subbus)
+ eeh_add_sysfs_files(subbus);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
+
+/**
* eeh_remove_device - Undo EEH setup for the indicated pci device
* @dev: pci device to be removed
* @purge_pe: remove the PE or not
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] EEH/OF: checking for CONFIG_EEH is not needed
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:13 UTC (permalink / raw)
To: linuxppc-dev
Cc: bhelgaas, paulus, linux-kernel, Thadeu Lima de Souza Cascardo,
shangw
In-Reply-To: <20121228120616.GB21831@oc0268524204.ibm.com>
The functions used are already defined as empty inline functions for the
case where EEH is disabled.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/of_platform.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index 2049f2d..c5fc6b2 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -71,10 +71,8 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
eeh_dev_phb_init_dynamic(phb);
/* Register devices with EEH */
-#ifdef CONFIG_EEH
if (dev->dev.of_node->child)
eeh_add_device_tree_early(dev->dev.of_node);
-#endif /* CONFIG_EEH */
/* Scan the bus */
pcibios_scan_phb(phb);
@@ -88,9 +86,7 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
pcibios_claim_one_bus(phb->bus);
/* Finish EEH setup */
-#ifdef CONFIG_EEH
eeh_add_device_tree_late(phb->bus);
-#endif
/* Add probed PCI devices to the device model */
pci_bus_add_devices(phb->bus);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v5 01/14] memory-hotplug: try to offline the memory twice to avoid dependence
From: Wen Congyang @ 2012-12-30 5:49 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
akpm, linuxppc-dev
In-Reply-To: <50DA68A8.4060001@jp.fujitsu.com>
At 12/26/2012 11:02 AM, Kamezawa Hiroyuki Wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> memory can't be offlined when CONFIG_MEMCG is selected.
>> For example: there is a memory device on node 1. The address range
>> is [1G, 1.5G). You will find 4 new directories memory8, memory9, memory10,
>> and memory11 under the directory /sys/devices/system/memory/.
>>
>> If CONFIG_MEMCG is selected, we will allocate memory to store page cgroup
>> when we online pages. When we online memory8, the memory stored page cgroup
>> is not provided by this memory device. But when we online memory9, the memory
>> stored page cgroup may be provided by memory8. So we can't offline memory8
>> now. We should offline the memory in the reversed order.
>>
>
> If memory8 is onlined as NORMAL memory ...right ?
Yes, memory8 is onlined as NORMAL memory. And when we online memory9, we allocate
memory from memory8 to store page cgroup information.
>
> IIUC, vmalloc() uses __GFP_HIGHMEM but doesn't use __GFP_MOVABLE.
>
>> When the memory device is hotremoved, we will auto offline memory provided
>> by this memory device. But we don't know which memory is onlined first, so
>> offlining memory may fail. In such case, iterate twice to offline the memory.
>> 1st iterate: offline every non primary memory block.
>> 2nd iterate: offline primary (i.e. first added) memory block.
>>
>> This idea is suggested by KOSAKI Motohiro.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>
> I'm not sure but the whole DIMM should be onlined as MOVABLE mem ?
If the whole DIMM is onlined as MOVABLE mem, we can offline it, and don't
retry again.
>
> Anyway, I agree this kind of retry is required if memory is onlined as NORMAL mem.
> But retry-once is ok ?
I'am not sure, but I think in most cases the user may online the memory according first
which is hot-added first. So we may always fail in the first time, and retry-once can
success.
Thanks
Wen Congyang
>
> Thanks,
> -Kame
>
>> ---
>> mm/memory_hotplug.c | 16 ++++++++++++++--
>> 1 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index d04ed87..62e04c9 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1388,10 +1388,13 @@ int remove_memory(u64 start, u64 size)
>> unsigned long start_pfn, end_pfn;
>> unsigned long pfn, section_nr;
>> int ret;
>> + int return_on_error = 0;
>> + int retry = 0;
>>
>> start_pfn = PFN_DOWN(start);
>> end_pfn = start_pfn + PFN_DOWN(size);
>>
>> +repeat:
>> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>> section_nr = pfn_to_section_nr(pfn);
>> if (!present_section_nr(section_nr))
>> @@ -1410,14 +1413,23 @@ int remove_memory(u64 start, u64 size)
>>
>> ret = offline_memory_block(mem);
>> if (ret) {
>> - kobject_put(&mem->dev.kobj);
>> - return ret;
>> + if (return_on_error) {
>> + kobject_put(&mem->dev.kobj);
>> + return ret;
>> + } else {
>> + retry = 1;
>> + }
>> }
>> }
>>
>> if (mem)
>> kobject_put(&mem->dev.kobj);
>>
>> + if (retry) {
>> + return_on_error = 1;
>> + goto repeat;
>> + }
>> +
>> return 0;
>> }
>> #else
>>
>
>
>
^ permalink raw reply
* Re: [PATCH v5 01/14] memory-hotplug: try to offline the memory twice to avoid dependence
From: Wen Congyang @ 2012-12-30 5:58 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
akpm, linuxppc-dev
In-Reply-To: <50D96543.6010903@parallels.com>
At 12/25/2012 04:35 PM, Glauber Costa Wrote:
> On 12/24/2012 04:09 PM, Tang Chen wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> memory can't be offlined when CONFIG_MEMCG is selected.
>> For example: there is a memory device on node 1. The address range
>> is [1G, 1.5G). You will find 4 new directories memory8, memory9, memory10,
>> and memory11 under the directory /sys/devices/system/memory/.
>>
>> If CONFIG_MEMCG is selected, we will allocate memory to store page cgroup
>> when we online pages. When we online memory8, the memory stored page cgroup
>> is not provided by this memory device. But when we online memory9, the memory
>> stored page cgroup may be provided by memory8. So we can't offline memory8
>> now. We should offline the memory in the reversed order.
>>
>> When the memory device is hotremoved, we will auto offline memory provided
>> by this memory device. But we don't know which memory is onlined first, so
>> offlining memory may fail. In such case, iterate twice to offline the memory.
>> 1st iterate: offline every non primary memory block.
>> 2nd iterate: offline primary (i.e. first added) memory block.
>>
>> This idea is suggested by KOSAKI Motohiro.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>
> Maybe there is something here that I am missing - I admit that I came
> late to this one, but this really sounds like a very ugly hack, that
> really has no place in here.
>
> Retrying, of course, may make sense, if we have reasonable belief that
> we may now succeed. If this is the case, you need to document - in the
> code - while is that.
>
> The memcg argument, however, doesn't really cut it. Why can't we make
> all page_cgroup allocations local to the node they are describing? If
> memcg is the culprit here, we should fix it, and not retry. If there is
> still any benefit in retrying, then we retry being very specific about why.
We try to make all page_cgroup allocations local to the node they are describing
now. If the memory is the first memory onlined in this node, we will allocate
it from the other node.
For example, node1 has 4 memory blocks: 8-11, and we online it from 8 to 11
1. memory block 8, page_cgroup allocations are in the other nodes
2. memory block 9, page_cgroup allocations are in memory block 8
So we should offline memory block 9 first. But we don't know in which order
the user online the memory block.
I think we can modify memcg like this:
allocate the memory from the memory block they are describing
I am not sure it is OK to do so.
Thanks
Wen Congyang
>
>
>
^ permalink raw reply
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Wen Congyang @ 2012-12-30 6:02 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
akpm, linuxppc-dev
In-Reply-To: <50DCE7C0.8070407@jp.fujitsu.com>
At 12/28/2012 08:28 AM, Kamezawa Hiroyuki Wrote:
> (2012/12/27 21:16), Wen Congyang wrote:
>> At 12/26/2012 11:55 AM, Kamezawa Hiroyuki Wrote:
>>> (2012/12/24 21:09), Tang Chen wrote:
>>>> From: Wen Congyang <wency@cn.fujitsu.com>
>>>>
>>>> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
>>>> should free it when removing a node.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>
>>> I'm sorry but is it safe to remove pgdat ? All zone cache and zonelists are
>>> properly cleared/rebuilded in synchronous way ? and No threads are visinting
>>> zone in vmscan.c ?
>>
>> We have rebuilt zonelists when a zone has no memory after offlining some pages.
>>
>
> How do you guarantee that the address of pgdat/zone is not on stack of any kernel
> threads or other kernel objects without reference counting or other syncing method ?
No way to guarentee this. But, the kernel should not use the address of pgdat/zone when
it is offlined.
Hmm, what about this: reuse the memory when the node is onlined again?
Thanks
Wen Congyang
>
>
> Thanks,
> -Kame
>
>
>
^ permalink raw reply
* [PATCH] Added device tree binding for TDM and TDM phy
From: Sandeep Singh @ 2013-01-02 13:25 UTC (permalink / raw)
To: devicetree-discuss, linuxppc-dev; +Cc: Sandeep Singh, Poonam Aggrwal
This controller is available on many Freescale SOCs like MPC8315, P1020, P1010
and P1022
Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---
.../devicetree/bindings/powerpc/fsl/fsl-tdm.txt | 63 ++++++++++++++++++++
.../devicetree/bindings/powerpc/fsl/tdm-phy.txt | 38 ++++++++++++
2 files changed, 101 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
new file mode 100644
index 0000000..ceb2ef1
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
@@ -0,0 +1,63 @@
+TDM Device Tree Binding
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+TDM (Time Division Multiplexing)
+
+Description:
+
+The TDM is full duplex serial port designed to allow various devices including
+digital signal processors (DSPs) to communicate with a variety of serial devices
+including industry standard framers, codecs, other DSPs and microprocessors.
+
+The below properties describe the device tree bindings for Freescale TDM
+controller. This TDM controller is available on various Freescale Processors
+like MPC8315, P1020, P1022 and P1010.
+
+Required properties:
+
+- compatible
+ Value type: <string>
+ Definition: Should contain "fsl,tdm1.0".
+
+- reg
+ Definition: A standard property. The first reg specifier describes the TDM
+ registers, and the second describes the TDM DMAC registers.
+
+- tdm_tx_clk
+ Value type: <u32 or u64>
+ Definition: This specifies the value of transmit clock. It should not
+ exceed 50Mhz.
+
+- tdm_rx_clk
+ Value type: <u32 or u64>
+ Definition: This specifies the value of receive clock. Its value could be
+ zero, in which case tdm will operate in shared mode. Its value should not
+ exceed 50Mhz.
+
+- interrupts
+ Definition: Two interrupt specifiers. The first is TDM error, and the
+ second is TDM DMAC.
+
+- phy-handle
+ Value type: <phandle>
+ Definition: Phandle of the line controller node or framer node eg. SLIC,
+ E1/T1 etc. (Refer Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt)
+
+- fsl,max-time-slots
+ Value type: <u32>
+ Definition: Maximum number of 8-bit time slots in one TDM frame. This is
+ the maximum number which TDM hardware supports.
+
+Example:
+
+ tdm@16000 {
+ compatible = "fsl,tdm1.0";
+ reg = <0x16000 0x200 0x2c000 0x2000>;
+ tdm_tx_clk = <2048000>;
+ tdm_rx_clk = <0>;
+ interrupts = <16 8 62 8>;
+ phy-handle = <&tdm-phy>;
+ fsl,max-time-slots = <128>;
+ };
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt
new file mode 100644
index 0000000..2563934
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt
@@ -0,0 +1,38 @@
+TDM PHY Device Tree Binding
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+Description:
+TDM PHY is the terminal interface of TDM subsystem. It is typically a line
+control device like E1/T1 framer or SLIC. A TDM device can have multiple TDM
+PHYs.
+
+Required properties:
+
+- compatible
+ Value type: <string>
+ Definition: Should contain generic compatibility like "tdm-phy-slic" or
+ "tdm-phy-e1" or "tdm-phy-t1".
+
+- max-num-ports
+ Value type: <u32>
+ Definition: Defines the maximum number of ports supported by the SLIC
+ device. Only required if the device is SLIC. For E1/T1 devices the number
+ of ports are predefined i.e. (24 in case of T1 and 32 in case of E1).
+
+Apart from the above, there may be other properties required because of the
+bus/interface this device is connected on. It could be SPI/local bus, etc.
+
+Example:
+
+ tdm-phy@0 {
+ compatible = "zarlink,le88266","tdm-phy-slic";
+ reg = <0>;
+ max-num-ports = <4>;
+ spi-max-frequency = <8000000>;
+ };
+
+In the above example properties "reg" and "spi-max-frequency" are SPI specific
+as the SLIC device is connected on SPI interface. These properties might vary
+depending on the specific interface the device is using.
--
1.7.6.GIT
^ permalink raw reply related
* Re: [PATCH v5 04/14] memory-hotplug: remove /sys/firmware/memmap/X sysfs
From: Christoph Lameter @ 2013-01-02 14:24 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, Kamezawa Hiroyuki, laijs, linux-kernel,
minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50DBBBEF.70701@cn.fujitsu.com>
On Thu, 27 Dec 2012, Tang Chen wrote:
> On 12/26/2012 11:30 AM, Kamezawa Hiroyuki wrote:
> >> @@ -41,6 +42,7 @@ struct firmware_map_entry {
> >> const char *type; /* type of the memory range */
> >> struct list_head list; /* entry for the linked list */
> >> struct kobject kobj; /* kobject for each entry */
> >> + unsigned int bootmem:1; /* allocated from bootmem */
> >> };
> >
> > Can't we detect from which the object is allocated from, slab or bootmem ?
> >
> > Hm, for example,
> >
> > PageReserved(virt_to_page(address_of_obj)) ?
> > PageSlab(virt_to_page(address_of_obj)) ?
> >
>
> Hi Kamezawa-san,
>
> I think we can detect it without a new member. I think bootmem:1 member
> is just for convenience. I think I can remove it. :)
Larger size slab allocations may fall back to the page allocator but then
the slabs do not track this allocation. That memory can be freed using the
page allocator.
If you see pageslab then you can always remove using the slab allocator.
Otherwise the page allocator should work (unless it was some
special case bootmem allocation).
^ permalink raw reply
* Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
From: Jiri Olsa @ 2013-01-02 14:58 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: Andi Kleen, Peter Zijlstra, robert.richter, Anton Blanchard,
linux-kernel, Stephane Eranian, linuxppc-dev, Ingo Molnar,
Paul Mackerras, Arnaldo Carvalho de Melo
In-Reply-To: <20121219072802.GA30790@us.ibm.com>
On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
>
> Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
> Make them global so they are available to all architectures.
>
> Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
> variable name as a parameter.
>
hi,
the change looks ok apart from some nits below.
There' another version of the x86 event attributes change
I mentioned earlier:
http://marc.info/?l=linux-kernel&m=135601815224373&w=2
I'm not sure which one will make it in first, but you
guys need to sync ;-) CC-ing Andi and Stephane.
thanks,
jirka
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/cpu/perf_event.c | 17 +++++------------
> include/linux/perf_event.h | 13 +++++++++++++
> 2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 4428fd1..24bc505 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
> .attrs = NULL,
> };
>
> -struct perf_pmu_events_attr {
> - struct device_attribute attr;
> - u64 id;
> -};
> -
> /*
> * Remove all undefined events (x86_pmu.event_map(id) == 0)
> * out of events_attr attributes.
> @@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev, struct device_attribute *at
> return x86_pmu.events_sysfs_show(page, config);
> }
>
> -#define EVENT_VAR(_id) event_attr_##_id
> -#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
> +#define EVENT_VAR(_id) event_attr_##_id
> +#define EVENT_ID(_id) PERF_COUNT_HW_##_id
> +#define EVENT_PTR(_id) PMU_EVENT_PTR(EVENT_VAR(_id))
>
> -#define EVENT_ATTR(_name, _id) \
> -static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
> - .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
> - .id = PERF_COUNT_HW_##_id, \
> -};
> +#define EVENT_ATTR(_name, _id) \
> + PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)
probably no need to define EVENT_ID macro if it's used on just one place
>
> EVENT_ATTR(cpu-cycles, CPU_CYCLES );
> EVENT_ATTR(instructions, INSTRUCTIONS );
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 6bfb2fa..31692cb 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -817,6 +817,19 @@ do { \
> } while (0)
>
>
> +struct perf_pmu_events_attr {
> + struct device_attribute attr;
> + u64 id;
> +};
> +
> +#define PMU_EVENT_PTR(_var) &_var.attr.attr
this one seems superfluous as well, could be replaced by '&'
^ permalink raw reply
* RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-01-03 5:21 UTC (permalink / raw)
To: Joerg Roedel
Cc: Wood Scott-B07421, joerg.roedel@amd.com, Tabi Timur-B04825,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1355493114-21776-1-git-send-email-Varun.Sethi@freescale.com>
Hi Joerg,
It's been a while since I submitted this patch. I have tried to address you=
r comments regarding the subwindow attribute. I would really appreciate if =
I can get some feedback on this patch.
Regards
Varun
> -----Original Message-----
> From: Sethi Varun-B16395
> Sent: Friday, December 21, 2012 7:17 AM
> To: 'Joerg Roedel'
> Cc: Sethi Varun-B16395; joerg.roedel@amd.com; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421
> Subject: RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API
> implementation.
>=20
> ping!!
>=20
> > -----Original Message-----
> > From: Sethi Varun-B16395
> > Sent: Friday, December 14, 2012 7:22 PM
> > To: joerg.roedel@amd.com; iommu@lists.linux-foundation.org; linuxppc-
> > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Tabi Timur-B04825;
> > Wood Scott-B07421
> > Cc: Sethi Varun-B16395
> > Subject: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API
> > implementation.
> >
> > This patchset provides the Freescale PAMU (Peripheral Access
> > Management
> > Unit) driver and the corresponding IOMMU API implementation. PAMU is
> > the IOMMU present on Freescale QorIQ platforms. PAMU can authorize
> > memory access, remap the memory address, and remap the I/O transaction
> type.
> >
> > This set consists of the following patches:
> > 1. Addition of new field in the device (powerpc) archdata structure
> > for storing iommu domain information
> > pointer. This pointer is stored when the device is attached to a
> > particular iommu domain.
> > 2. Add PAMU bypass enable register to the ccsr_guts structure.
> > 3. Addition of domain attributes required by the PAMU driver IOMMU API.
> > 4. PAMU driver and IOMMU API implementation.
> >
> > This patch set is based on the next branch of the iommu git tree
> > maintained by Joerg.
> >
> > Varun Sethi (4):
> > store iommu domain info in device arch data.
> > add pamu bypass enable register to guts.
> > Add iommu attributes for PAMU
> > FSL PAMU driver.
> >
> > arch/powerpc/include/asm/device.h | 4 +
> > arch/powerpc/include/asm/fsl_guts.h | 4 +-
> > drivers/iommu/Kconfig | 8 +
> > drivers/iommu/Makefile | 1 +
> > drivers/iommu/fsl_pamu.c | 1152
> > +++++++++++++++++++++++++++++++++++
> > drivers/iommu/fsl_pamu.h | 398 ++++++++++++
> > drivers/iommu/fsl_pamu_domain.c | 1033
> > +++++++++++++++++++++++++++++++
> > drivers/iommu/fsl_pamu_domain.h | 96 +++
> > include/linux/iommu.h | 49 ++
> > 9 files changed, 2744 insertions(+), 1 deletions(-) create mode
> > 100644 drivers/iommu/fsl_pamu.c create mode 100644
> > drivers/iommu/fsl_pamu.h create mode 100644
> > drivers/iommu/fsl_pamu_domain.c create mode 100644
> > drivers/iommu/fsl_pamu_domain.h
> >
> > --
> > 1.7.4.1
^ permalink raw reply
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-01-03 6:13 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev list, Linux Kernel list
Hi Linus !
Here are a couple of small powerpc fixes. They aren't new bugs (and
they are both CCed to stable) but I didn't see the point of sitting
on the fixes any longer.
Oh and happy new year !
Cheers,
Ben.
The following changes since commit d1c3ed669a2d452cacfb48c2d171a1f364dae2ed:
Linux 3.8-rc2 (2013-01-02 18:13:21 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git
for you to fetch changes up to e6449c9b2d90c1bd9a5985bf05ddebfd1631cd6b:
powerpc: Add missing NULL terminator to avoid boot panic on PPC40x (2013-01-03 16:45:52 +1100)
----------------------------------------------------------------
Gabor Juhos (1):
powerpc: Add missing NULL terminator to avoid boot panic on PPC40x
Shan Hai (1):
powerpc/vdso: Remove redundant locking in update_vsyscall_tz()
arch/powerpc/kernel/time.c | 5 -----
arch/powerpc/platforms/40x/ppc40x_simple.c | 3 ++-
2 files changed, 2 insertions(+), 6 deletions(-)
^ permalink raw reply
* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-01-03 6:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev list, Linux Kernel list
In-Reply-To: <1357193608.3924.37.camel@pasglop>
On Thu, 2013-01-03 at 17:13 +1100, Benjamin Herrenschmidt wrote:
> Hi Linus !
>
> Here are a couple of small powerpc fixes. They aren't new bugs (and
> they are both CCed to stable) but I didn't see the point of sitting
> on the fixes any longer.
Looks like I still need to fix my script to get the branch name right
for when the mirrors haven't caught up...
It's
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
Cheers,
Ben.
^ permalink raw reply
* tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-03 13:20 UTC (permalink / raw)
To: linuxppc-dev
Hello,
I hope someone could help me with my problem. Currently I am porting
a new kernel (3.3.8) for a tqm5200s based board.
The previous kernel was 2.6.23. The new kernel version is needed because
of support for a wireless card.
I got issues with the i2c bus and this kernel. When the kernel boots up,
the i2c initialization ends in a timout. This is the kernel log:
[ 1.460652] i2c /dev entries driver
[ 1.465434] mpc-i2c f0003d40.i2c: timeout 1000000 us
Connected to the bus is an eeprom (Microchip 24c32a) and a realtime
clock (Philips PCF8563).
The i2c bus section in the dtb file looks as follows:
i2c@3d40 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,mpc5200-i2c","fsl-i2c";
reg = <0x3d40 0x40>;
interrupts = <2 16 0>;
fsl5200-clocking;
};
The dtb file from the kernel 2.6.23 looks as follows:
i2c@3d40 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,mpc5200-i2c","fsl-i2c";
reg = <0x3d40 0x40>;
interrupts = <2 16 0>;
fsl5200-clocking;
};
I can`t see any devices in /sys/bus/i2c/devices except the bus itself.
# ls /sys/bus/i2c/devices
# i2c-0
# cat /sys/bus/i2c/devices/i2c-0/name
# MPC adapter
Is there something wrong with my dtb file or is it a bug in the mpc-i2c driver
Best regards
Johannes
^ permalink raw reply
* Re: tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-03 13:25 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <CADwvPCve5GaRZZXuLUq4Es2FLN8AxtbRP4TQP7gQVRXoMw3S0g@mail.gmail.com>
> The dtb file from the kernel 2.6.23 looks as follows:
> i2c@3d40 {
>
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "fsl,mpc5200-i2c","fsl-i2c";
> reg = <0x3d40 0x40>;
> interrupts = <2 16 0>;
> fsl5200-clocking;
> };
sry, but this was a mistake. This is the correct section from the kernel 2.6.23:
i2c@3d40 {
device_type = "i2c";
compatible = "mpc5200-i2c\0fsl-i2c";
cell-index= <1>;
reg = <3d40 40>;
interrupts = <2 10 0>;
interrupt-parent = <&mpc5200_pic>;
fsl5200-clocking;
};
^ permalink raw reply
* Re: tqm5200s i2c bus timeout
From: Anatolij Gustschin @ 2013-01-03 14:11 UTC (permalink / raw)
To: Johannes Braun; +Cc: linuxppc-dev
In-Reply-To: <CADwvPCve5GaRZZXuLUq4Es2FLN8AxtbRP4TQP7gQVRXoMw3S0g@mail.gmail.com>
Hi,
On Thu, 3 Jan 2013 14:20:41 +0100
Johannes Braun <jjo.braun@gmail.com> wrote:
> Hello,
>
> I hope someone could help me with my problem. Currently I am porting
> a new kernel (3.3.8) for a tqm5200s based board.
> The previous kernel was 2.6.23. The new kernel version is needed because
> of support for a wireless card.
>
> I got issues with the i2c bus and this kernel. When the kernel boots up,
> the i2c initialization ends in a timout. This is the kernel log:
>
> [ 1.460652] i2c /dev entries driver
> [ 1.465434] mpc-i2c f0003d40.i2c: timeout 1000000 us
No, the initialization doesn't end in timeout. It is just an info
which timeout value will be used for i2c transfers.
>
> Connected to the bus is an eeprom (Microchip 24c32a) and a realtime
> clock (Philips PCF8563).
> The i2c bus section in the dtb file looks as follows:
>
> i2c@3d40 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "fsl,mpc5200-i2c","fsl-i2c";
> reg = <0x3d40 0x40>;
> interrupts = <2 16 0>;
> fsl5200-clocking;
This "fsl5200-clocking" property is not needed anymore, you can remove it.
> };
>
> The dtb file from the kernel 2.6.23 looks as follows:
> i2c@3d40 {
>
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "fsl,mpc5200-i2c","fsl-i2c";
> reg = <0x3d40 0x40>;
> interrupts = <2 16 0>;
> fsl5200-clocking;
> };
>
> I can`t see any devices in /sys/bus/i2c/devices except the bus itself.
> # ls /sys/bus/i2c/devices
> # i2c-0
> # cat /sys/bus/i2c/devices/i2c-0/name
> # MPC adapter
This is expected since you didn't add sub-nodes for your i2c devices
24c32a and PCF8563 in the i2c adapter node.
> Is there something wrong with my dtb file or is it a bug in the mpc-i2c driver
It is an issue with our dtb file. Please look at the I2C eeprom
sub-node in the arch/powerpc/boot/dts/mpc5121ads.dts file and at
the pcf8563 RTC sub-node in the arch/powerpc/boot/dts/mucmc52.dts
file for an example how to add needed nodes for your devices.
Thanks,
Anatolij
^ permalink raw reply
* Re: [PATCH] powerpc/pasemi: enable PRINTK_TIME in defconfig
From: Olof Johansson @ 2013-01-03 20:54 UTC (permalink / raw)
To: benh; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <1353912688-6649-1-git-send-email-olof@lixom.net>
On Sun, Nov 25, 2012 at 10:51 PM, Olof Johansson <olof@lixom.net> wrote:
> Enable PRINTK_TIME in pasemi_defconfig. Also regenerate it, it seems
> that a lot of options have moved around since last time savedefconfig
> was ran on it.
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
Ping?
-Olof
^ permalink raw reply
* Re: [PATCH] powerpc/pasemi: enable PRINTK_TIME in defconfig
From: Benjamin Herrenschmidt @ 2013-01-03 20:58 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <CAOesGMjE1xHVMwhA_Hv4BL5CGmuReuTTGmRfntmXQ5R90khSLA@mail.gmail.com>
On Thu, 2013-01-03 at 12:54 -0800, Olof Johansson wrote:
> On Sun, Nov 25, 2012 at 10:51 PM, Olof Johansson <olof@lixom.net> wrote:
> > Enable PRINTK_TIME in pasemi_defconfig. Also regenerate it, it seems
> > that a lot of options have moved around since last time savedefconfig
> > was ran on it.
> >
> > Signed-off-by: Olof Johansson <olof@lixom.net>
>
> Ping?
I'm in the middle of harvesting stuff from patchwork for -next, this is one of them.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] EEH/OF: checking for CONFIG_EEH is not needed
From: Gavin Shan @ 2013-01-04 2:24 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: bhelgaas, paulus, linuxppc-dev, linux-kernel, shangw
In-Reply-To: <1356721999-25159-1-git-send-email-cascardo@linux.vnet.ibm.com>
On Fri, Dec 28, 2012 at 05:13:18PM -0200, Thadeu Lima de Souza Cascardo wrote:
>The functions used are already defined as empty inline functions for the
>case where EEH is disabled.
>
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>---
> arch/powerpc/kernel/of_platform.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
>diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
>index 2049f2d..c5fc6b2 100644
>--- a/arch/powerpc/kernel/of_platform.c
>+++ b/arch/powerpc/kernel/of_platform.c
>@@ -71,10 +71,8 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
> eeh_dev_phb_init_dynamic(phb);
>
> /* Register devices with EEH */
>-#ifdef CONFIG_EEH
> if (dev->dev.of_node->child)
> eeh_add_device_tree_early(dev->dev.of_node);
>-#endif /* CONFIG_EEH */
>
> /* Scan the bus */
> pcibios_scan_phb(phb);
>@@ -88,9 +86,7 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
> pcibios_claim_one_bus(phb->bus);
>
> /* Finish EEH setup */
>-#ifdef CONFIG_EEH
> eeh_add_device_tree_late(phb->bus);
>-#endif
>
> /* Add probed PCI devices to the device model */
> pci_bus_add_devices(phb->bus);
Thanks,
Gavin
^ permalink raw reply
* [PATCH 0/2] cpuhotplug/nohz: Fix issue of "negative" idle time
From: Srivatsa Vaddagiri @ 2013-01-04 2:58 UTC (permalink / raw)
To: Russell King, Paul E. McKenney, Stephen Boyd, linux-arm-kernel,
Mike Frysinger, uclinux-dist-devel, Ralf Baechle, linux-mips,
Benjamin Herrenschmidt, linuxppc-dev, Martin Schwidefsky,
linux-s390, Paul Mundt, linux-sh, David S. Miller, sparclinux,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, mhocko,
srivatsa.bhat
Cc: linux-arm-msm, Srivatsa Vaddagiri, linux-kernel
On most architectures (arm, mips, s390, sh and x86) idle thread of a cpu does
not cleanly exit nohz state before dying upon hot-remove. As a result,
offline cpu is seen to be in nohz mode (ts->idle_active = 1) and its offline
time can potentially be included in total idle time reported via /proc/stat.
When the same cpu later comes online, its offline time however is not included
in its idle time statistics, thus causing a rollback in total idle time to be
observed by applications like top.
Example output from Android top command highlighting this issue is below:
User 232%, System 70%, IOW 46%, IRQ 1%
User 1322 + Nice 0 + Sys 399 + Idle -1423 + IOW 264 + IRQ 0 + SIRQ 7 = 569
top is reporting system to be idle for -1423 ticks over some sampling period.
This happens as total idle time reported in cpu line of /proc/stat *dropped*
from the last value observed (cached) by top command.
While this was originally seen on a ARM platform running 3.4 based kernel, I
could easily recreate it on my x86 desktop running latest tip/master kernel
(HEAD 3a7bfcad). Online/offline a cpu in a tight loop and in another loop read
/proc/stat and observe if total idle time drops from previously read value.
Although commit 7386cdbf (nohz: Fix idle ticks in cpu summary line of
/proc/stat) aims to avoid this bug, its not preemption proof. A
thread could get preempted after the cpu_online() check in get_idle_time(), thus
potentially leading to get_cpu_idle_time_us() being invoked on a offline cpu.
One potential fix is to serialize hotplug with /proc/stat read operation (via
use of get/put_online_cpus()), which I disliked in favor of the other
solution proposed in this series.
In this patch series:
- Patch 1/2 modifies idle loop on architectures arm, mips, s390, sh and x86 to
exit nohz state before the associated idle thread dies upon hotremove. This
fixes the idle time accounting bug.
Patch 1/2 also modifies idle loop on all architectures supporting cpu hotplug
to have idle thread of a dying cpu die immediately after schedule() returns
control to it. I see no point in wasting time via calls to *_enter()/*_exit()
before noticing the need to die and dying.
- Patch 2/2 reverts commit 7386cdbf (nohz: Fix idle ticks in cpu summary line of
/proc/stat). The cpu_online() check introduced by it is no longer necessary
with Patch 1/2 applied. Having fewer code sites worry about online status of
cpus is a good thing!
---
arch/arm/kernel/process.c | 9 ++++-----
arch/arm/kernel/smp.c | 2 +-
arch/blackfin/kernel/process.c | 8 ++++----
arch/mips/kernel/process.c | 6 +++---
arch/powerpc/kernel/idle.c | 2 +-
arch/s390/kernel/process.c | 4 ++--
arch/sh/kernel/idle.c | 5 ++---
arch/sparc/kernel/process_64.c | 3 ++-
arch/x86/kernel/process.c | 5 ++---
fs/proc/stat.c | 14 ++++----------
10 files changed, 25 insertions(+), 33 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH 1/2] cpuhotplug/nohz: Remove offline cpus from nohz-idle state
From: Srivatsa Vaddagiri @ 2013-01-04 2:58 UTC (permalink / raw)
To: Russell King, Paul E. McKenney, Stephen Boyd, linux-arm-kernel,
Mike Frysinger, uclinux-dist-devel, Ralf Baechle, linux-mips,
Benjamin Herrenschmidt, linuxppc-dev, Martin Schwidefsky,
linux-s390, Paul Mundt, linux-sh, David S. Miller, sparclinux,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, mhocko,
srivatsa.bhat
Cc: linux-arm-msm, Srivatsa Vaddagiri, linux-kernel
Modify idle loop of arm, mips, s390, sh and x86 architectures to exit from nohz
state before dying upon hot-remove. This change is needed to avoid userspace
tools like top command from seeing a rollback in total idle time over some
sampling periods.
Additionaly, modify idle loop on all architectures supporting cpu hotplug to
have idle thread of a dying cpu die immediately after scheduler returns control
to it. There is no point in wasting time via calls to *_enter()/*_exit() before
noticing the need to die and dying.
Additional ARM specific change:
Revert commit ff081e05 ("ARM: 7457/1: smp: Fix suspicious
RCU originating from cpu_die()"), which added a RCU_NONIDLE() wrapper
around call to complete(). That wrapper is no longer needed as cpu_die() is
now called outside of a rcu_idle_enter()/exit() section. I also think that the
wait_for_completion() based wait in ARM's __cpu_die() can be replaced with a
busy-loop based one, as the wait there in general should be terminated within
few cycles.
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: uclinux-dist-devel@blackfin.uclinux.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-sh@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: mhocko@suse.cz
Cc: srivatsa.bhat@linux.vnet.ibm.com
Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
---
arch/arm/kernel/process.c | 9 ++++-----
arch/arm/kernel/smp.c | 2 +-
arch/blackfin/kernel/process.c | 8 ++++----
arch/mips/kernel/process.c | 6 +++---
arch/powerpc/kernel/idle.c | 2 +-
arch/s390/kernel/process.c | 4 ++--
arch/sh/kernel/idle.c | 5 ++---
arch/sparc/kernel/process_64.c | 3 ++-
arch/x86/kernel/process.c | 5 ++---
9 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index c6dec5f..254099b 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -191,11 +191,6 @@ void cpu_idle(void)
rcu_idle_enter();
ledtrig_cpu(CPU_LED_IDLE_START);
while (!need_resched()) {
-#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(smp_processor_id()))
- cpu_die();
-#endif
-
/*
* We need to disable interrupts here
* to ensure we don't miss a wakeup call.
@@ -224,6 +219,10 @@ void cpu_idle(void)
rcu_idle_exit();
tick_nohz_idle_exit();
schedule_preempt_disabled();
+#ifdef CONFIG_HOTPLUG_CPU
+ if (cpu_is_offline(smp_processor_id()))
+ cpu_die();
+#endif
}
}
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 84f4cbf..a8e3b8a 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -251,7 +251,7 @@ void __ref cpu_die(void)
mb();
/* Tell __cpu_die() that this CPU is now safe to dispose of */
- RCU_NONIDLE(complete(&cpu_died));
+ complete(&cpu_died);
/*
* actual CPU shutdown procedure is at least platform (if not
diff --git a/arch/blackfin/kernel/process.c b/arch/blackfin/kernel/process.c
index 3e16ad9..2bee1af 100644
--- a/arch/blackfin/kernel/process.c
+++ b/arch/blackfin/kernel/process.c
@@ -83,10 +83,6 @@ void cpu_idle(void)
while (1) {
void (*idle)(void) = pm_idle;
-#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(smp_processor_id()))
- cpu_die();
-#endif
if (!idle)
idle = default_idle;
tick_nohz_idle_enter();
@@ -98,6 +94,10 @@ void cpu_idle(void)
preempt_enable_no_resched();
schedule();
preempt_disable();
+#ifdef CONFIG_HOTPLUG_CPU
+ if (cpu_is_offline(smp_processor_id()))
+ cpu_die();
+#endif
}
}
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index a11c6f9..41102a0 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -71,13 +71,13 @@ void __noreturn cpu_idle(void)
start_critical_timings();
}
}
+ rcu_idle_exit();
+ tick_nohz_idle_exit();
+ schedule_preempt_disabled();
#ifdef CONFIG_HOTPLUG_CPU
if (!cpu_online(cpu) && !cpu_isset(cpu, cpu_callin_map))
play_dead();
#endif
- rcu_idle_exit();
- tick_nohz_idle_exit();
- schedule_preempt_disabled();
}
}
diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
index ea78761..39ad029 100644
--- a/arch/powerpc/kernel/idle.c
+++ b/arch/powerpc/kernel/idle.c
@@ -102,11 +102,11 @@ void cpu_idle(void)
ppc64_runlatch_on();
rcu_idle_exit();
tick_nohz_idle_exit();
+ schedule_preempt_disabled();
if (cpu_should_die()) {
sched_preempt_enable_no_resched();
cpu_die();
}
- schedule_preempt_disabled();
}
}
diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c
index 536d645..5290556 100644
--- a/arch/s390/kernel/process.c
+++ b/arch/s390/kernel/process.c
@@ -66,8 +66,6 @@ unsigned long thread_saved_pc(struct task_struct *tsk)
*/
static void default_idle(void)
{
- if (cpu_is_offline(smp_processor_id()))
- cpu_die();
local_irq_disable();
if (need_resched()) {
local_irq_enable();
@@ -95,6 +93,8 @@ void cpu_idle(void)
if (test_thread_flag(TIF_MCCK_PENDING))
s390_handle_mcck();
schedule_preempt_disabled();
+ if (cpu_is_offline(smp_processor_id()))
+ cpu_die();
}
}
diff --git a/arch/sh/kernel/idle.c b/arch/sh/kernel/idle.c
index 0c91016..f8bc2f0 100644
--- a/arch/sh/kernel/idle.c
+++ b/arch/sh/kernel/idle.c
@@ -96,9 +96,6 @@ void cpu_idle(void)
check_pgt_cache();
rmb();
- if (cpu_is_offline(cpu))
- play_dead();
-
local_irq_disable();
/* Don't trace irqs off for idle */
stop_critical_timings();
@@ -115,6 +112,8 @@ void cpu_idle(void)
rcu_idle_exit();
tick_nohz_idle_exit();
schedule_preempt_disabled();
+ if (cpu_is_offline(cpu))
+ play_dead();
}
}
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..01589e7 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -105,13 +105,14 @@ void cpu_idle(void)
rcu_idle_exit();
tick_nohz_idle_exit();
+ schedule_preempt_disabled();
+
#ifdef CONFIG_HOTPLUG_CPU
if (cpu_is_offline(cpu)) {
sched_preempt_enable_no_resched();
cpu_play_dead();
}
#endif
- schedule_preempt_disabled();
}
}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 2ed787f..3d5f142 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -331,9 +331,6 @@ void cpu_idle(void)
while (!need_resched()) {
rmb();
- if (cpu_is_offline(smp_processor_id()))
- play_dead();
-
/*
* Idle routines should keep interrupts disabled
* from here on, until they go to idle.
@@ -366,6 +363,8 @@ void cpu_idle(void)
preempt_enable_no_resched();
schedule();
preempt_disable();
+ if (cpu_is_offline(smp_processor_id()))
+ play_dead();
}
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH 2/2] Revert "nohz: Fix idle ticks in cpu summary line of /proc/stat" (commit 7386cdbf2f57ea8cff3c9fde93f206e58b9fe13f).
From: Srivatsa Vaddagiri @ 2013-01-04 2:58 UTC (permalink / raw)
To: Russell King, Paul E. McKenney, Stephen Boyd, linux-arm-kernel,
Mike Frysinger, uclinux-dist-devel, Ralf Baechle, linux-mips,
Benjamin Herrenschmidt, linuxppc-dev, Martin Schwidefsky,
linux-s390, Paul Mundt, linux-sh, David S. Miller, sparclinux,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, mhocko,
srivatsa.bhat
Cc: linux-arm-msm, Srivatsa Vaddagiri, linux-kernel
With offline cpus no longer beeing seen in nohz mode (ts->idle_active=0), we
don't need the check for cpu_online() introduced in commit 7386cdbf. Offline
cpu's idle time as last recorded in its ts->idle_sleeptime will be reported
(thus excluding its offline time as part of idle time statistics).
Cc: mhocko@suse.cz
Cc: srivatsa.bhat@linux.vnet.ibm.com
Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
---
fs/proc/stat.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index e296572..64c3b31 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -45,13 +45,10 @@ static cputime64_t get_iowait_time(int cpu)
static u64 get_idle_time(int cpu)
{
- u64 idle, idle_time = -1ULL;
-
- if (cpu_online(cpu))
- idle_time = get_cpu_idle_time_us(cpu, NULL);
+ u64 idle, idle_time = get_cpu_idle_time_us(cpu, NULL);
if (idle_time == -1ULL)
- /* !NO_HZ or cpu offline so we can rely on cpustat.idle */
+ /* !NO_HZ so we can rely on cpustat.idle */
idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
else
idle = usecs_to_cputime64(idle_time);
@@ -61,13 +58,10 @@ static u64 get_idle_time(int cpu)
static u64 get_iowait_time(int cpu)
{
- u64 iowait, iowait_time = -1ULL;
-
- if (cpu_online(cpu))
- iowait_time = get_cpu_iowait_time_us(cpu, NULL);
+ u64 iowait, iowait_time = get_cpu_iowait_time_us(cpu, NULL);
if (iowait_time == -1ULL)
- /* !NO_HZ or cpu offline so we can rely on cpustat.iowait */
+ /* !NO_HZ so we can rely on cpustat.iowait */
iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
else
iowait = usecs_to_cputime64(iowait_time);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCH 2/2] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Gavin Shan @ 2013-01-04 3:19 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: shangw, linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <1356721999-25159-2-git-send-email-cascardo@linux.vnet.ibm.com>
On Fri, Dec 28, 2012 at 05:13:19PM -0200, Thadeu Lima de Souza Cascardo wrote:
>The DDW code uses a eeh_dev struct from the pci_dev. However, this is
>not set until eeh_add_device_late is called.
>
>Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
>devices are added to the bus, making drivers' probe hooks to be called.
>These will call set_dma_mask, which will call the DDW code, which will
>require the eeh_dev struct from pci_dev. This would result in a crash,
>due to a NULL dereference.
>
>Calling eeh_add_device_late after pci_bus_add_devices would make the
>system BUG, because device files shouldn't be added to devices there
>were not added to the system. So, a new function is needed to add such
>files only after pci_bus_add_devices have been called.
>
Thanks, Cascardo. The change looks good to me :-)
Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>Cc: stable@vger.kernel.org
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/eeh.h | 3 +++
> arch/powerpc/kernel/of_platform.c | 3 +++
> arch/powerpc/kernel/pci-common.c | 7 +++++--
> arch/powerpc/platforms/pseries/eeh.c | 24 +++++++++++++++++++++++-
> 4 files changed, 34 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>index b0ef738..0f816da 100644
>--- a/arch/powerpc/include/asm/eeh.h
>+++ b/arch/powerpc/include/asm/eeh.h
>@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
> void __init eeh_addr_cache_build(void);
> void eeh_add_device_tree_early(struct device_node *);
> void eeh_add_device_tree_late(struct pci_bus *);
>+void eeh_add_sysfs_files(struct pci_bus *);
> void eeh_remove_bus_device(struct pci_dev *, int);
>
> /**
>@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
>
> static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
>
>+static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
>+
> static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
>
> static inline void eeh_lock(void) { }
>diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
>index c5fc6b2..500dd32 100644
>--- a/arch/powerpc/kernel/of_platform.c
>+++ b/arch/powerpc/kernel/of_platform.c
>@@ -91,6 +91,9 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
> /* Add probed PCI devices to the device model */
> pci_bus_add_devices(phb->bus);
>
>+ /* sysfs files should only be added after devices are added */
>+ eeh_add_sysfs_files(phb->bus);
>+
> return 0;
> }
>
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index 7f94f76..4d3de7e 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> pcibios_allocate_bus_resources(bus);
> pcibios_claim_one_bus(bus);
>
>+ /* Fixup EEH */
>+ eeh_add_device_tree_late(bus);
>+
> /* Add new devices to global lists. Register in proc, sysfs. */
> pci_bus_add_devices(bus);
>
>- /* Fixup EEH */
>- eeh_add_device_tree_late(bus);
>+ /* sysfs files should only be added after devices are added */
>+ eeh_add_sysfs_files(bus);
> }
> EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
>
>diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
>index 9a04322..6b73d6c 100644
>--- a/arch/powerpc/platforms/pseries/eeh.c
>+++ b/arch/powerpc/platforms/pseries/eeh.c
>@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
> dev->dev.archdata.edev = edev;
>
> eeh_addr_cache_insert_dev(dev);
>- eeh_sysfs_add_device(dev);
> }
>
> /**
>@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
> EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
>
> /**
>+ * eeh_add_sysfs_files - Add EEH sysfs files for the indicated PCI bus
>+ * @bus: PCI bus
>+ *
>+ * This routine must be used to add EEH sysfs files for PCI
>+ * devices which are attached to the indicated PCI bus. The PCI bus
>+ * is added after system boot through hotplug or dlpar.
>+ */
>+void eeh_add_sysfs_files(struct pci_bus *bus)
>+{
>+ struct pci_dev *dev;
>+
>+ list_for_each_entry(dev, &bus->devices, bus_list) {
>+ eeh_sysfs_add_device(dev);
>+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
>+ struct pci_bus *subbus = dev->subordinate;
>+ if (subbus)
>+ eeh_add_sysfs_files(subbus);
>+ }
>+ }
>+}
>+EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
>+
>+/**
> * eeh_remove_device - Undo EEH setup for the indicated pci device
> * @dev: pci device to be removed
> * @purge_pe: remove the PE or not
>
Thanks,
Gavin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox