* 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: Gavin Shan @ 2012-12-28 5:21 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo; +Cc: paulus, linuxppc-dev, anton
In-Reply-To: <1356625686-8943-1-git-send-email-cascardo@linux.vnet.ibm.com>
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
*/
>+ 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? :-)
> for (i = 0; i < (tbl->it_size/64); i++) {
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Gavin Shan @ 2012-12-28 5:18 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: shangw, linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <1356626040-9384-1-git-send-email-cascardo@linux.vnet.ibm.com>
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 :-)
>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" :-)
> 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 :-)
>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
^ permalink raw reply
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Kamezawa Hiroyuki @ 2012-12-28 0:28 UTC (permalink / raw)
To: Wen Congyang
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: <50DC3C26.6060308@cn.fujitsu.com>
(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 ?
Thanks,
-Kame
^ permalink raw reply
* [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-27 16:34 UTC (permalink / raw)
To: linuxppc-dev
Cc: shangw, linux-kernel, stable, paulus,
Thadeu Lima de Souza Cascardo, bhelgaas
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/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 *);
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) { }
+
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);
}
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..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);
+
+/**
* 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] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-27 16:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, anton, Thadeu Lima de Souza Cascardo
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. */
+ 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] PowerPC documentation: fixed path to the powerpc directory
From: Thomas Waldecker @ 2012-12-27 14:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Thomas Waldecker, linux-kernel, linux-doc
ppc -> powerpc
Signed-off-by: Thomas Waldecker <thomas.waldecker@gmail.com>
---
Documentation/powerpc/cpu_features.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/powerpc/cpu_features.txt b/Documentation/powerpc/cpu_features.txt
index ffa4183..ae09df8 100644
--- a/Documentation/powerpc/cpu_features.txt
+++ b/Documentation/powerpc/cpu_features.txt
@@ -11,10 +11,10 @@ split instruction and data caches, and if the CPU supports the DOZE and NAP
sleep modes.
Detection of the feature set is simple. A list of processors can be found in
-arch/ppc/kernel/cputable.c. The PVR register is masked and compared with each
-value in the list. If a match is found, the cpu_features of cur_cpu_spec is
-assigned to the feature bitmask for this processor and a __setup_cpu function
-is called.
+arch/powerpc/kernel/cputable.c. The PVR register is masked and compared with
+each value in the list. If a match is found, the cpu_features of cur_cpu_spec
+is assigned to the feature bitmask for this processor and a __setup_cpu
+function is called.
C code may test 'cur_cpu_spec[smp_processor_id()]->cpu_features' for a
particular feature bit. This is done in quite a few places, for example
@@ -51,6 +51,6 @@ should be used in the majority of cases.
The END_FTR_SECTION macros are implemented by storing information about this
code in the '__ftr_fixup' ELF section. When do_cpu_ftr_fixups
-(arch/ppc/kernel/misc.S) is invoked, it will iterate over the records in
+(arch/powerpc/kernel/misc.S) is invoked, it will iterate over the records in
__ftr_fixup, and if the required feature is not present it will loop writing
nop's from each BEGIN_FTR_SECTION to END_FTR_SECTION.
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Wen Congyang @ 2012-12-27 12:16 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: <50DA7533.6060407@jp.fujitsu.com>
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.
Thanks
Wen Congyang
>
> Thanks,
> -Kame
>
>> ---
>> mm/memory_hotplug.c | 20 +++++++++++++++++++-
>> 1 files changed, 19 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index f8a1d2f..447fa24 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1680,9 +1680,12 @@ static int check_cpu_on_node(void *data)
>> /* offline the node if all memory sections of this node are removed */
>> static void try_offline_node(int nid)
>> {
>> + pg_data_t *pgdat = NODE_DATA(nid);
>> unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
>> - unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
>> + unsigned long end_pfn = start_pfn + pgdat->node_spanned_pages;
>> unsigned long pfn;
>> + struct page *pgdat_page = virt_to_page(pgdat);
>> + int i;
>>
>> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>> unsigned long section_nr = pfn_to_section_nr(pfn);
>> @@ -1709,6 +1712,21 @@ static void try_offline_node(int nid)
>> */
>> node_set_offline(nid);
>> unregister_one_node(nid);
>> +
>> + if (!PageSlab(pgdat_page) && !PageCompound(pgdat_page))
>> + /* node data is allocated from boot memory */
>> + return;
>> +
>> + /* free waittable in each zone */
>> + for (i = 0; i < MAX_NR_ZONES; i++) {
>> + struct zone *zone = pgdat->node_zones + i;
>> +
>> + if (zone->wait_table)
>> + vfree(zone->wait_table);
>> + }
>> +
>> + arch_refresh_nodedata(nid, NULL);
>> + arch_free_nodedata(pgdat);
>> }
>>
>> int __ref remove_memory(int nid, u64 start, u64 size)
>>
>
>
>
^ permalink raw reply
* RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2012-12-27 6:02 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,
Do you have any comments on the patchset?
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
* Re: [PATCH v5 02/14] memory-hotplug: check whether all memory blocks are offlined or not when removing memory
From: Tang Chen @ 2012-12-27 3:10 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <50DA6AB3.2030608@jp.fujitsu.com>
On 12/26/2012 11:10 AM, Kamezawa Hiroyuki wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> From: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>>
>> We remove the memory like this:
>> 1. lock memory hotplug
>> 2. offline a memory block
>> 3. unlock memory hotplug
>> 4. repeat 1-3 to offline all memory blocks
>> 5. lock memory hotplug
>> 6. remove memory(TODO)
>> 7. unlock memory hotplug
>>
>> All memory blocks must be offlined before removing memory. But we don't hold
>> the lock in the whole operation. So we should check whether all memory blocks
>> are offlined before step6. Otherwise, kernel maybe panicked.
>>
>> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
>> Signed-off-by: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>
> a nitpick below.
>
>> +
>> + for (pfn = start_pfn; pfn< end_pfn; pfn += PAGES_PER_SECTION) {
>
> I prefer adding mem = NULL at the start of this for().
Hi Kamezawa-san,
Added, thanks. :)
>
>> + section_nr = pfn_to_section_nr(pfn);
>> + if (!present_section_nr(section_nr))
>> + continue;
>> +
>> + section = __nr_to_section(section_nr);
>> + /* same memblock? */
>> + if (mem)
>> + if ((section_nr>= mem->start_section_nr)&&
>> + (section_nr<= mem->end_section_nr))
>> + continue;
>> +
>
> Thanks,
> -Kame
>
>
>
^ permalink raw reply
* Re: [PATCH v5 04/14] memory-hotplug: remove /sys/firmware/memmap/X sysfs
From: Tang Chen @ 2012-12-27 3:09 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <50DA6F5A.2070601@jp.fujitsu.com>
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. :)
Thanks. :)
^ permalink raw reply
* Re: [PATCH v5 03/14] memory-hotplug: remove redundant codes
From: Tang Chen @ 2012-12-27 3:09 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <50DA6D04.8020906@jp.fujitsu.com>
Hi Kamezawa-san,
Thanks for the reviewing. Please see below. :)
On 12/26/2012 11:20 AM, Kamezawa Hiroyuki wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> From: Wen Congyang<wency@cn.fujitsu.com>
>>
>> offlining memory blocks and checking whether memory blocks are offlined
>> are very similar. This patch introduces a new function to remove
>> redundant codes.
>>
>> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
>> ---
>> mm/memory_hotplug.c | 101 ++++++++++++++++++++++++++++-----------------------
>> 1 files changed, 55 insertions(+), 46 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index d43d97b..dbb04d8 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1381,20 +1381,14 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
>> return __offline_pages(start_pfn, start_pfn + nr_pages, 120 * HZ);
>> }
>>
>> -int remove_memory(u64 start, u64 size)
>
> please add explanation of this function here. If (*func) returns val other than 0,
> this function will fail and returns callback's return value...right ?
>
Yes, it will always return the func()'s return value. I'll add the
comment here. :)
>
>> +static int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
>> + void *arg, int (*func)(struct memory_block *, void *))
>> {
>> struct memory_block *mem = NULL;
>> struct mem_section *section;
>> - 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:
>
> Shouldn't we check lock is held here ? (VM_BUG_ON(!mutex_is_locked(&mem_hotplug_mutex);
Well, I think, after applying this patch, walk_memory_range() will be
a separated function. And it can be used somewhere else where we don't
hold this lock. But for now, we can do this check. :)
>
>
>> for (pfn = start_pfn; pfn< end_pfn; pfn += PAGES_PER_SECTION) {
>> section_nr = pfn_to_section_nr(pfn);
>> if (!present_section_nr(section_nr))
>> @@ -1411,22 +1405,61 @@ repeat:
>> if (!mem)
>> continue;
>>
>> - ret = offline_memory_block(mem);
>> + ret = func(mem, arg);
>> if (ret) {
>> - if (return_on_error) {
>> - kobject_put(&mem->dev.kobj);
>> - return ret;
>> - } else {
>> - retry = 1;
>> - }
>> + kobject_put(&mem->dev.kobj);
>> + return ret;
>> }
>> }
>>
>> if (mem)
>> kobject_put(&mem->dev.kobj);
>>
>> - if (retry) {
>> - return_on_error = 1;
>> + return 0;
>> +}
>> +
>> +static int offline_memory_block_cb(struct memory_block *mem, void *arg)
>> +{
>> + int *ret = arg;
>> + int error = offline_memory_block(mem);
>> +
>> + if (error != 0&& *ret == 0)
>> + *ret = error;
>> +
>> + return 0;
>
> Always returns 0 and run through all mem blocks for scan-and-retry, right ?
> You need explanation here !
Yes, I'll add the comment. :)
>
>
>> +}
>> +
>> +static int is_memblock_offlined_cb(struct memory_block *mem, void *arg)
>> +{
>> + int ret = !is_memblock_offlined(mem);
>> +
>> + if (unlikely(ret))
>> + pr_warn("removing memory fails, because memory "
>> + "[%#010llx-%#010llx] is onlined\n",
>> + PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
>> + PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1))-1);
>> +
>> + return ret;
>> +}
>> +
>> +int remove_memory(u64 start, u64 size)
>> +{
>> + unsigned long start_pfn, end_pfn;
>> + int ret = 0;
>> + int retry = 1;
>> +
>> + start_pfn = PFN_DOWN(start);
>> + end_pfn = start_pfn + PFN_DOWN(size);
>> +
>> +repeat:
>
> please explan why you repeat here .
This repeat is add in patch1. It aims to solve the problem we were
talking about in patch1. I'll add the comment here. :)
>
>> + walk_memory_range(start_pfn, end_pfn,&ret,
>> + offline_memory_block_cb);
>> + if (ret) {
>> + if (!retry)
>> + return ret;
>> +
>> + retry = 0;
>> + ret = 0;
>> goto repeat;
>> }
>>
>> @@ -1444,37 +1477,13 @@ repeat:
>> * memory blocks are offlined.
>> */
>>
>> - for (pfn = start_pfn; pfn< end_pfn; pfn += PAGES_PER_SECTION) {
>> - section_nr = pfn_to_section_nr(pfn);
>> - if (!present_section_nr(section_nr))
>> - continue;
>> -
>> - section = __nr_to_section(section_nr);
>> - /* same memblock? */
>> - if (mem)
>> - if ((section_nr>= mem->start_section_nr)&&
>> - (section_nr<= mem->end_section_nr))
>> - continue;
>> -
>> - mem = find_memory_block_hinted(section, mem);
>> - if (!mem)
>> - continue;
>> -
>> - ret = is_memblock_offlined(mem);
>> - if (!ret) {
>> - pr_warn("removing memory fails, because memory "
>> - "[%#010llx-%#010llx] is onlined\n",
>> - PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
>> - PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1)) - 1);
>> -
>> - kobject_put(&mem->dev.kobj);
>> - unlock_memory_hotplug();
>> - return ret;
>> - }
>
> please explain what you do here. confirming all memory blocks are offlined
> before returning 0 ....right ?
Will be added. :)
Thanks. :)
>
>> + ret = walk_memory_range(start_pfn, end_pfn, NULL,
>> + is_memblock_offlined_cb);
>> + if (ret) {
>> + unlock_memory_hotplug();
>> + return ret;
>> }
>>
>> - if (mem)
>> - kobject_put(&mem->dev.kobj);
>> unlock_memory_hotplug();
>>
>> return 0;
>>
>
> Thanks,
> -Kame
>
>
^ permalink raw reply
* Re: [REGRESSION][3.8.-rc1][ INFO: possible circular locking dependency detected ]
From: Li Zhong @ 2012-12-26 8:18 UTC (permalink / raw)
To: Christian Kujau; +Cc: Cong Wang, linuxppc-dev, LKML, Maciej Rutecki
In-Reply-To: <alpine.DEB.2.01.1212231321560.7378@trent.utfs.org>
On Sun, 2012-12-23 at 13:34 -0800, Christian Kujau wrote:
> On Sat, 22 Dec 2012 at 16:28, Maciej Rutecki wrote:
> > Got during suspend to disk:
>
> I got a similar message on a powerpc G4 system, right after bootup (no
> suspend involved):
>
> http://nerdbynature.de/bits/3.8.0-rc1/
>
> [ 97.803049] ======================================================
> [ 97.803051] [ INFO: possible circular locking dependency detected ]
> [ 97.803059] 3.8.0-rc1-dirty #2 Not tainted
> [ 97.803060] -------------------------------------------------------
> [ 97.803066] kworker/0:1/235 is trying to acquire lock:
> [ 97.803097] ((fb_notifier_list).rwsem){.+.+.+}, at: [<c00606a0>] __blocking_notifier_call_chain+0x44/0x88
> [ 97.803099]
> [ 97.803099] but task is already holding lock:
> [ 97.803110] (console_lock){+.+.+.}, at: [<c03b9fd0>] console_callback+0x20/0x194
> [ 97.803112]
> [ 97.803112] which lock already depends on the new lock.
>
> ...and on it goes. Please see the URL above for the whole dmesg and
> .config.
>
> @Li Zhong: I have applied your fix for the "MAX_STACK_TRACE_ENTRIES too
> low" warning[0] to 3.8-rc1 (hence the -dirty flag), but in the
> backtrace "ret_from_kernel_thread" shows up again. FWIW, your
> patch helped to make the "MAX_STACK_TRACE_ENTRIES too low"
> warning go away in 3.7.0-rc7 and it did not re-appear ever
> since.
The patch fixing "MAX_STACK_TRACE_ENTRIES too low" warning clears the
stack back chain at "ret_from_kernel_thread", so I think it's fine to
see it on the top of the stack.
Thank, Zhong
> Thanks,
> Christian.
>
> [0] http://lkml.indiana.edu/hypermail/linux/kernel/1211.3/01917.html
>
> > [ 269.784867] [ INFO: possible circular locking dependency detected ]
> > [ 269.784869] 3.8.0-rc1 #1 Not tainted
> > [ 269.784870] -------------------------------------------------------
> > [ 269.784871] kworker/u:3/56 is trying to acquire lock:
> > [ 269.784878] ((fb_notifier_list).rwsem){.+.+.+}, at: [<ffffffff81062a1d>]
> > __blocking_notifier_call_chain+0x49/0x80
> > [ 269.784879]
> > [ 269.784879] but task is already holding lock:
> > [ 269.784884] (console_lock){+.+.+.}, at: [<ffffffff812ee4ce>]
> > i915_drm_freeze+0x9e/0xbb
> > [ 269.784884]
> > [ 269.784884] which lock already depends on the new lock.
> > [ 269.784884]
> > [ 269.784885]
> > [ 269.784885] the existing dependency chain (in reverse order) is:
> > [ 269.784887]
> > [ 269.784887] -> #1 (console_lock){+.+.+.}:
> > [ 269.784890] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.784893] [<ffffffff810405a1>] console_lock+0x59/0x5b
> > [ 269.784897] [<ffffffff812ba125>] register_con_driver+0x36/0x128
> > [ 269.784899] [<ffffffff812bb27e>] take_over_console+0x1e/0x45
> > [ 269.784903] [<ffffffff81257a04>] fbcon_takeover+0x56/0x98
> > [ 269.784906] [<ffffffff8125b857>] fbcon_event_notify+0x2c1/0x5ea
> > [ 269.784909] [<ffffffff8149a211>] notifier_call_chain+0x67/0x92
> > [ 269.784911] [<ffffffff81062a33>] __blocking_notifier_call_chain+0x5f/0x80
> > [ 269.784912] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.784915] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.784917] [<ffffffff812505d7>] register_framebuffer+0x20a/0x26e
> > [ 269.784920] [<ffffffff812d3ca0>]
> > drm_fb_helper_single_fb_probe+0x1ce/0x297
> > [ 269.784922] [<ffffffff812d3f40>] drm_fb_helper_initial_config+0x1d7/0x1ef
> > [ 269.784924] [<ffffffff8132cee2>] intel_fbdev_init+0x6f/0x82
> > [ 269.784927] [<ffffffff812f22f6>] i915_driver_load+0xa9e/0xc78
> > [ 269.784929] [<ffffffff812e020c>] drm_get_pci_dev+0x165/0x26d
> > [ 269.784931] [<ffffffff812ee8da>] i915_pci_probe+0x60/0x69
> > [ 269.784933] [<ffffffff8123fe8e>] local_pci_probe+0x39/0x61
> > [ 269.784935] [<ffffffff812400f5>] pci_device_probe+0xba/0xe0
> > [ 269.784938] [<ffffffff8133d3b6>] driver_probe_device+0x99/0x1c4
> > [ 269.784940] [<ffffffff8133d52f>] __driver_attach+0x4e/0x6f
> > [ 269.784942] [<ffffffff8133bae1>] bus_for_each_dev+0x52/0x84
> > [ 269.784944] [<ffffffff8133cec6>] driver_attach+0x19/0x1b
> > [ 269.784946] [<ffffffff8133cb65>] bus_add_driver+0xdf/0x203
> > [ 269.784948] [<ffffffff8133dad3>] driver_register+0x8e/0x114
> > [ 269.784952] [<ffffffff8123f581>] __pci_register_driver+0x5d/0x62
> > [ 269.784953] [<ffffffff812e0395>] drm_pci_init+0x81/0xe6
> > [ 269.784957] [<ffffffff81af7612>] i915_init+0x66/0x68
> > [ 269.784959] [<ffffffff810020b4>] do_one_initcall+0x7a/0x136
> > [ 269.784962] [<ffffffff8147ceaa>] kernel_init+0x141/0x296
> > [ 269.784964] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.784966]
> > [ 269.784966] -> #0 ((fb_notifier_list).rwsem){.+.+.+}:
> > [ 269.784967] [<ffffffff81088955>] __lock_acquire+0xa7e/0xddd
> > [ 269.784969] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.784971] [<ffffffff81495092>] down_read+0x34/0x43
> > [ 269.784973] [<ffffffff81062a1d>] __blocking_notifier_call_chain+0x49/0x80
> > [ 269.784975] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.784977] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.784979] [<ffffffff8124ec47>] fb_set_suspend+0x22/0x4d
> > [ 269.784981] [<ffffffff8132cfe3>] intel_fbdev_set_suspend+0x20/0x22
> > [ 269.784983] [<ffffffff812ee4db>] i915_drm_freeze+0xab/0xbb
> > [ 269.784985] [<ffffffff812eea82>] i915_pm_freeze+0x3d/0x41
> > [ 269.784987] [<ffffffff8123f759>] pci_pm_freeze+0x65/0x8d
> > [ 269.784990] [<ffffffff81342f20>] dpm_run_callback.isra.3+0x27/0x56
> > [ 269.784993] [<ffffffff81343085>] __device_suspend+0x136/0x1b1
> > [ 269.784995] [<ffffffff8134311a>] async_suspend+0x1a/0x58
> > [ 269.784997] [<ffffffff81063a6b>] async_run_entry_fn+0xa4/0x17c
> > [ 269.785000] [<ffffffff81058df2>] process_one_work+0x1cf/0x38e
> > [ 269.785002] [<ffffffff81059290>] worker_thread+0x12e/0x1cc
> > [ 269.785004] [<ffffffff8105d416>] kthread+0xac/0xb4
> > [ 269.785006] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.785006]
> > [ 269.785006] other info that might help us debug this:
> > [ 269.785006]
> > [ 269.785007] Possible unsafe locking scenario:
> > [ 269.785007]
> > [ 269.785008] CPU0 CPU1
> > [ 269.785008] ---- ----
> > [ 269.785009] lock(console_lock);
> > [ 269.785010] lock((fb_notifier_list).rwsem);
> > [ 269.785012] lock(console_lock);
> > [ 269.785013] lock((fb_notifier_list).rwsem);
> > [ 269.785013]
> > [ 269.785013] *** DEADLOCK ***
> > [ 269.785013]
> > [ 269.785014] 4 locks held by kworker/u:3/56:
> > [ 269.785018] #0: (events_unbound){.+.+.+}, at: [<ffffffff81058d77>]
> > process_one_work+0x154/0x38e
> > [ 269.785021] #1: ((&entry->work)){+.+.+.}, at: [<ffffffff81058d77>]
> > process_one_work+0x154/0x38e
> > [ 269.785024] #2: (&__lockdep_no_validate__){......}, at: [<ffffffff81342d85>]
> > device_lock+0xf/0x11
> > [ 269.785027] #3: (console_lock){+.+.+.}, at: [<ffffffff812ee4ce>]
> > i915_drm_freeze+0x9e/0xbb
> > [ 269.785028]
> > [ 269.785028] stack backtrace:
> > [ 269.785029] Pid: 56, comm: kworker/u:3 Not tainted 3.8.0-rc1 #1
> > [ 269.785030] Call Trace:
> > [ 269.785035] [<ffffffff8148fcb5>] print_circular_bug+0x1f8/0x209
> > [ 269.785036] [<ffffffff81088955>] __lock_acquire+0xa7e/0xddd
> > [ 269.785038] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.785040] [<ffffffff81062a1d>] ? __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785042] [<ffffffff81495092>] down_read+0x34/0x43
> > [ 269.785044] [<ffffffff81062a1d>] ? __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785046] [<ffffffff81062a1d>] __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785047] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.785050] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.785052] [<ffffffff8124ec47>] fb_set_suspend+0x22/0x4d
> > [ 269.785054] [<ffffffff8132cfe3>] intel_fbdev_set_suspend+0x20/0x22
> > [ 269.785055] [<ffffffff812ee4db>] i915_drm_freeze+0xab/0xbb
> > [ 269.785057] [<ffffffff812eea82>] i915_pm_freeze+0x3d/0x41
> > [ 269.785060] [<ffffffff8123f759>] pci_pm_freeze+0x65/0x8d
> > [ 269.785062] [<ffffffff8123f6f4>] ? pci_pm_poweroff+0x9c/0x9c
> > [ 269.785064] [<ffffffff81342f20>] dpm_run_callback.isra.3+0x27/0x56
> > [ 269.785066] [<ffffffff81343085>] __device_suspend+0x136/0x1b1
> > [ 269.785068] [<ffffffff81089563>] ? trace_hardirqs_on_caller+0x117/0x173
> > [ 269.785070] [<ffffffff8134311a>] async_suspend+0x1a/0x58
> > [ 269.785072] [<ffffffff81063a6b>] async_run_entry_fn+0xa4/0x17c
> > [ 269.785074] [<ffffffff81058df2>] process_one_work+0x1cf/0x38e
> > [ 269.785076] [<ffffffff81058d77>] ? process_one_work+0x154/0x38e
> > [ 269.785078] [<ffffffff810639c7>] ? async_schedule+0x12/0x12
> > [ 269.785080] [<ffffffff8105679f>] ? spin_lock_irq+0x9/0xb
> > [ 269.785082] [<ffffffff81059290>] worker_thread+0x12e/0x1cc
> > [ 269.785084] [<ffffffff81059162>] ? rescuer_thread+0x187/0x187
> > [ 269.785085] [<ffffffff8105d416>] kthread+0xac/0xb4
> > [ 269.785088] [<ffffffff8105d36a>] ? __kthread_parkme+0x60/0x60
> > [ 269.785090] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.785091] [<ffffffff8105d36a>] ? __kthread_parkme+0x60/0x60
> >
> >
> > Config:
> > http://mrutecki.pl/download/kernel/3.8.0-rc1/s2disk/config-3.8.0-rc1
> >
> > dmesg:
> > http://mrutecki.pl/download/kernel/3.8.0-rc1/s2disk/dmesg-3.8.0-rc1.txt
> >
> >
> > Found similar report:
> > http://marc.info/?l=linux-kernel&m=135546308908700&w=2
> >
> > Regards
> >
> > --
> > Maciej Rutecki
> > http://www.mrutecki.pl
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
^ permalink raw reply
* Re: [PATCH v5 07/14] memory-hotplug: move pgdat_resize_lock into sparse_remove_one_section()
From: Tang Chen @ 2012-12-26 6:20 UTC (permalink / raw)
To: Kamezawa Hiroyuki
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <50DA7357.109@jp.fujitsu.com>
On 12/26/2012 11:47 AM, Kamezawa Hiroyuki wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> In __remove_section(), we locked pgdat_resize_lock when calling
>> sparse_remove_one_section(). This lock will disable irq. But we don't need
>> to lock the whole function. If we do some work to free pagetables in
>> free_section_usemap(), we need to call flush_tlb_all(), which need
>> irq enabled. Otherwise the WARN_ON_ONCE() in smp_call_function_many()
>> will be triggered.
>>
>> Signed-off-by: Tang Chen<tangchen@cn.fujitsu.com>
>> Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
>
> If this is a bug fix, call-trace in your log and BUGFIX or -fix- in patch title
> will be appreciated, I think.
>
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>
Hi Kamezawa-san,
Thanks for the reviewing.
I don't think this would be a bug. It is OK to lock the whole
sparse_remove_one_section() if no tlb flushing in free_section_usemap().
But we need to flush tlb in free_section_usemap(), so we need to take
free_section_usemap() out of the lock. :)
I add the call trace to the patch so that people could review it more
easily.
And here is the call trace for this version:
[ 454.796248] ------------[ cut here ]------------
[ 454.851408] WARNING: at kernel/smp.c:461
smp_call_function_many+0xbd/0x260()
[ 454.935620] Hardware name: PRIMEQUEST 1800E
......
[ 455.652201] Call Trace:
[ 455.681391] [<ffffffff8106e73f>] warn_slowpath_common+0x7f/0xc0
[ 455.753151] [<ffffffff810560a0>] ? leave_mm+0x50/0x50
[ 455.814527] [<ffffffff8106e79a>] warn_slowpath_null+0x1a/0x20
[ 455.884208] [<ffffffff810e7a9d>] smp_call_function_many+0xbd/0x260
[ 455.959082] [<ffffffff810e7ecb>] smp_call_function+0x3b/0x50
[ 456.027722] [<ffffffff810560a0>] ? leave_mm+0x50/0x50
[ 456.089098] [<ffffffff810e7f4b>] on_each_cpu+0x3b/0xc0
[ 456.151512] [<ffffffff81055f0c>] flush_tlb_all+0x1c/0x20
[ 456.216004] [<ffffffff8104f8de>] remove_pagetable+0x14e/0x1d0
[ 456.285683] [<ffffffff8104f978>] vmemmap_free+0x18/0x20
[ 456.349139] [<ffffffff811b8797>] sparse_remove_one_section+0xf7/0x100
[ 456.427126] [<ffffffff811c5fc2>] __remove_section+0xa2/0xb0
[ 456.494726] [<ffffffff811c6070>] __remove_pages+0xa0/0xd0
[ 456.560258] [<ffffffff81669c7b>] arch_remove_memory+0x6b/0xc0
[ 456.629937] [<ffffffff8166ad28>] remove_memory+0xb8/0xf0
[ 456.694431] [<ffffffff813e686f>] acpi_memory_device_remove+0x53/0x96
[ 456.771379] [<ffffffff813b33c4>] acpi_device_remove+0x90/0xb2
[ 456.841059] [<ffffffff8144b02c>] __device_release_driver+0x7c/0xf0
[ 456.915928] [<ffffffff8144b1af>] device_release_driver+0x2f/0x50
[ 456.988719] [<ffffffff813b4476>] acpi_bus_remove+0x32/0x6d
[ 457.055285] [<ffffffff813b4542>] acpi_bus_trim+0x91/0x102
[ 457.120814] [<ffffffff813b463b>] acpi_bus_hot_remove_device+0x88/0x16b
[ 457.199840] [<ffffffff813afda7>] acpi_os_execute_deferred+0x27/0x34
[ 457.275756] [<ffffffff81091ece>] process_one_work+0x20e/0x5c0
[ 457.345434] [<ffffffff81091e5f>] ? process_one_work+0x19f/0x5c0
[ 457.417190] [<ffffffff813afd80>] ?
acpi_os_wait_events_complete+0x23/0x23
[ 457.499332] [<ffffffff81093f6e>] worker_thread+0x12e/0x370
[ 457.565896] [<ffffffff81093e40>] ? manage_workers+0x180/0x180
[ 457.635574] [<ffffffff8109a09e>] kthread+0xee/0x100
[ 457.694871] [<ffffffff810dfaf9>] ? __lock_release+0x129/0x190
[ 457.764552] [<ffffffff81099fb0>] ? __init_kthread_worker+0x70/0x70
[ 457.839427] [<ffffffff81690aac>] ret_from_fork+0x7c/0xb0
[ 457.903914] [<ffffffff81099fb0>] ? __init_kthread_worker+0x70/0x70
[ 457.978784] ---[ end trace 25e85300f542aa01 ]---
Thanks. :)
^ permalink raw reply
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Kamezawa Hiroyuki @ 2012-12-26 3:55 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-15-git-send-email-tangchen@cn.fujitsu.com>
(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 ?
Thanks,
-Kame
> ---
> mm/memory_hotplug.c | 20 +++++++++++++++++++-
> 1 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index f8a1d2f..447fa24 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1680,9 +1680,12 @@ static int check_cpu_on_node(void *data)
> /* offline the node if all memory sections of this node are removed */
> static void try_offline_node(int nid)
> {
> + pg_data_t *pgdat = NODE_DATA(nid);
> unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
> - unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
> + unsigned long end_pfn = start_pfn + pgdat->node_spanned_pages;
> unsigned long pfn;
> + struct page *pgdat_page = virt_to_page(pgdat);
> + int i;
>
> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> unsigned long section_nr = pfn_to_section_nr(pfn);
> @@ -1709,6 +1712,21 @@ static void try_offline_node(int nid)
> */
> node_set_offline(nid);
> unregister_one_node(nid);
> +
> + if (!PageSlab(pgdat_page) && !PageCompound(pgdat_page))
> + /* node data is allocated from boot memory */
> + return;
> +
> + /* free waittable in each zone */
> + for (i = 0; i < MAX_NR_ZONES; i++) {
> + struct zone *zone = pgdat->node_zones + i;
> +
> + if (zone->wait_table)
> + vfree(zone->wait_table);
> + }
> +
> + arch_refresh_nodedata(nid, NULL);
> + arch_free_nodedata(pgdat);
> }
>
> int __ref remove_memory(int nid, u64 start, u64 size)
>
^ permalink raw reply
* Re: [PATCH v5 07/14] memory-hotplug: move pgdat_resize_lock into sparse_remove_one_section()
From: Kamezawa Hiroyuki @ 2012-12-26 3:47 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-8-git-send-email-tangchen@cn.fujitsu.com>
(2012/12/24 21:09), Tang Chen wrote:
> In __remove_section(), we locked pgdat_resize_lock when calling
> sparse_remove_one_section(). This lock will disable irq. But we don't need
> to lock the whole function. If we do some work to free pagetables in
> free_section_usemap(), we need to call flush_tlb_all(), which need
> irq enabled. Otherwise the WARN_ON_ONCE() in smp_call_function_many()
> will be triggered.
>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
If this is a bug fix, call-trace in your log and BUGFIX or -fix- in patch title
will be appreciated, I think.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
^ permalink raw reply
* Re: [PATCH v5 01/14] memory-hotplug: try to offline the memory twice to avoid dependence
From: Kamezawa Hiroyuki @ 2012-12-26 3:02 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-2-git-send-email-tangchen@cn.fujitsu.com>
(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 ?
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 ?
Anyway, I agree this kind of retry is required if memory is onlined as NORMAL mem.
But retry-once is ok ?
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 05/14] memory-hotplug: introduce new function arch_remove_memory() for removing page table depends on architecture
From: Kamezawa Hiroyuki @ 2012-12-26 3:37 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-6-git-send-email-tangchen@cn.fujitsu.com>
(2012/12/24 21:09), Tang Chen wrote:
> From: Wen Congyang <wency@cn.fujitsu.com>
>
> For removing memory, we need to remove page table. But it depends
> on architecture. So the patch introduce arch_remove_memory() for
> removing page table. Now it only calls __remove_pages().
>
> Note: __remove_pages() for some archtecuture is not implemented
> (I don't know how to implement it for s390).
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Then, remove code will be symetric to add codes.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
^ permalink raw reply
* Re: [PATCH v5 04/14] memory-hotplug: remove /sys/firmware/memmap/X sysfs
From: Kamezawa Hiroyuki @ 2012-12-26 3:30 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-5-git-send-email-tangchen@cn.fujitsu.com>
(2012/12/24 21:09), Tang Chen wrote:
> From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
>
> When (hot)adding memory into system, /sys/firmware/memmap/X/{end, start, type}
> sysfs files are created. But there is no code to remove these files. The patch
> implements the function to remove them.
>
> Note: The code does not free firmware_map_entry which is allocated by bootmem.
> So the patch makes memory leak. But I think the memory leak size is
> very samll. And it does not affect the system.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> ---
> drivers/firmware/memmap.c | 98 +++++++++++++++++++++++++++++++++++++++++-
> include/linux/firmware-map.h | 6 +++
> mm/memory_hotplug.c | 5 ++-
> 3 files changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/memmap.c b/drivers/firmware/memmap.c
> index 90723e6..49be12a 100644
> --- a/drivers/firmware/memmap.c
> +++ b/drivers/firmware/memmap.c
> @@ -21,6 +21,7 @@
> #include <linux/types.h>
> #include <linux/bootmem.h>
> #include <linux/slab.h>
> +#include <linux/mm.h>
>
> /*
> * Data types ------------------------------------------------------------------
> @@ -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)) ?
Thanks,
-Kame
^ permalink raw reply
* Re: [PATCH v5 06/14] memory-hotplug: implement register_page_bootmem_info_section of sparse-vmemmap
From: Tang Chen @ 2012-12-26 3:21 UTC (permalink / raw)
To: Jianguo Wu
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
yinghai, laijs, linux-kernel, minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50D95F51.9090007@huawei.com>
On 12/25/2012 04:09 PM, Jianguo Wu wrote:
>> +
>> + if (!cpu=5Fhas=5Fpse) {
>> + next =3D (addr + PAGE=5FSIZE)& PAGE=5FMASK;
>> + pmd =3D pmd=5Foffset(pud, addr);
>> + if (pmd=5Fnone(*pmd))
>> + continue;
>> + get=5Fpage=5Fbootmem(section=5Fnr, pmd=5Fpage(*pmd),
>> + MIX=5FSECTION=5FINFO);
>> +
>> + pte =3D pte=5Foffset=5Fkernel(pmd, addr);
>> + if (pte=5Fnone(*pte))
>> + continue;
>> + get=5Fpage=5Fbootmem(section=5Fnr, pte=5Fpage(*pte),
>> + SECTION=5FINFO);
>> + } else {
>> + next =3D pmd=5Faddr=5Fend(addr, end);
>> +
>> + pmd =3D pmd=5Foffset(pud, addr);
>> + if (pmd=5Fnone(*pmd))
>> + continue;
>> + get=5Fpage=5Fbootmem(section=5Fnr, pmd=5Fpage(*pmd),
>> + SECTION=5FINFO);
>
> Hi Tang=EF=BC=8C
> In this case, pmd maps 512 pages, but you only get=5Fpage=5Fbootmem() on=
the first page.
> I think the whole 512 pages should be get=5Fpage=5Fbootmem(), what do you=
think?
>
Hi Wu,
Yes, thanks. I will fix it. :)
Thanks. :)
=
^ permalink raw reply
* Re: [PATCH v5 03/14] memory-hotplug: remove redundant codes
From: Kamezawa Hiroyuki @ 2012-12-26 3:20 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-4-git-send-email-tangchen@cn.fujitsu.com>
(2012/12/24 21:09), Tang Chen wrote:
> From: Wen Congyang <wency@cn.fujitsu.com>
>
> offlining memory blocks and checking whether memory blocks are offlined
> are very similar. This patch introduces a new function to remove
> redundant codes.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> mm/memory_hotplug.c | 101 ++++++++++++++++++++++++++++-----------------------
> 1 files changed, 55 insertions(+), 46 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index d43d97b..dbb04d8 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1381,20 +1381,14 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
> return __offline_pages(start_pfn, start_pfn + nr_pages, 120 * HZ);
> }
>
> -int remove_memory(u64 start, u64 size)
please add explanation of this function here. If (*func) returns val other than 0,
this function will fail and returns callback's return value...right ?
> +static int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
> + void *arg, int (*func)(struct memory_block *, void *))
> {
> struct memory_block *mem = NULL;
> struct mem_section *section;
> - 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:
Shouldn't we check lock is held here ? (VM_BUG_ON(!mutex_is_locked(&mem_hotplug_mutex);
> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> section_nr = pfn_to_section_nr(pfn);
> if (!present_section_nr(section_nr))
> @@ -1411,22 +1405,61 @@ repeat:
> if (!mem)
> continue;
>
> - ret = offline_memory_block(mem);
> + ret = func(mem, arg);
> if (ret) {
> - if (return_on_error) {
> - kobject_put(&mem->dev.kobj);
> - return ret;
> - } else {
> - retry = 1;
> - }
> + kobject_put(&mem->dev.kobj);
> + return ret;
> }
> }
>
> if (mem)
> kobject_put(&mem->dev.kobj);
>
> - if (retry) {
> - return_on_error = 1;
> + return 0;
> +}
> +
> +static int offline_memory_block_cb(struct memory_block *mem, void *arg)
> +{
> + int *ret = arg;
> + int error = offline_memory_block(mem);
> +
> + if (error != 0 && *ret == 0)
> + *ret = error;
> +
> + return 0;
Always returns 0 and run through all mem blocks for scan-and-retry, right ?
You need explanation here !
> +}
> +
> +static int is_memblock_offlined_cb(struct memory_block *mem, void *arg)
> +{
> + int ret = !is_memblock_offlined(mem);
> +
> + if (unlikely(ret))
> + pr_warn("removing memory fails, because memory "
> + "[%#010llx-%#010llx] is onlined\n",
> + PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
> + PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1))-1);
> +
> + return ret;
> +}
> +
> +int remove_memory(u64 start, u64 size)
> +{
> + unsigned long start_pfn, end_pfn;
> + int ret = 0;
> + int retry = 1;
> +
> + start_pfn = PFN_DOWN(start);
> + end_pfn = start_pfn + PFN_DOWN(size);
> +
> +repeat:
please explan why you repeat here .
> + walk_memory_range(start_pfn, end_pfn, &ret,
> + offline_memory_block_cb);
> + if (ret) {
> + if (!retry)
> + return ret;
> +
> + retry = 0;
> + ret = 0;
> goto repeat;
> }
>
> @@ -1444,37 +1477,13 @@ repeat:
> * memory blocks are offlined.
> */
>
> - for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> - section_nr = pfn_to_section_nr(pfn);
> - if (!present_section_nr(section_nr))
> - continue;
> -
> - section = __nr_to_section(section_nr);
> - /* same memblock? */
> - if (mem)
> - if ((section_nr >= mem->start_section_nr) &&
> - (section_nr <= mem->end_section_nr))
> - continue;
> -
> - mem = find_memory_block_hinted(section, mem);
> - if (!mem)
> - continue;
> -
> - ret = is_memblock_offlined(mem);
> - if (!ret) {
> - pr_warn("removing memory fails, because memory "
> - "[%#010llx-%#010llx] is onlined\n",
> - PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
> - PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1)) - 1);
> -
> - kobject_put(&mem->dev.kobj);
> - unlock_memory_hotplug();
> - return ret;
> - }
please explain what you do here. confirming all memory blocks are offlined
before returning 0 ....right ?
> + ret = walk_memory_range(start_pfn, end_pfn, NULL,
> + is_memblock_offlined_cb);
> + if (ret) {
> + unlock_memory_hotplug();
> + return ret;
> }
>
> - if (mem)
> - kobject_put(&mem->dev.kobj);
> unlock_memory_hotplug();
>
> return 0;
>
Thanks,
-Kame
^ permalink raw reply
* Re: [PATCH v5 08/14] memory-hotplug: Common APIs to support page tables hot-remove
From: Tang Chen @ 2012-12-26 3:19 UTC (permalink / raw)
To: Jianguo Wu
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
yinghai, laijs, linux-kernel, minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50DA6AC5.4020904@cn.fujitsu.com>
On 12/26/2012 11:11 AM, Tang Chen wrote:
> On 12/26/2012 10:49 AM, Tang Chen wrote:
>> On 12/25/2012 04:17 PM, Jianguo Wu wrote:
>>>> +
>>>> +static void __meminit free_pagetable(struct page *page, int order)
>>>> +{
>>>> + struct zone *zone;
>>>> + bool bootmem = false;
>>>> + unsigned long magic;
>>>> +
>>>> + /* bootmem page has reserved flag */
>>>> + if (PageReserved(page)) {
>>>> + __ClearPageReserved(page);
>>>> + bootmem = true;
>>>> +
>>>> + magic = (unsigned long)page->lru.next;
>>>> + if (magic == SECTION_INFO || magic == MIX_SECTION_INFO)
>
> And also, I think we don't need to check MIX_SECTION_INFO since it is
> for the pageblock_flags, not the memmap in the section.
Oh, no :)
We also need to check MIX_SECTION_INFO because we set pgd, pud, pmd
pages as MIX_SECTION_INFO in register_page_bootmem_memmap() in patch6.
Thanks. :)
>
> Thanks. :)
>
>>>> + put_page_bootmem(page);
>>>
>>> Hi Tang,
>>>
>>> For removing memmap of sparse-vmemmap, in cpu_has_pse case, if magic
>>> == SECTION_INFO,
>>> the order will be get_order(PMD_SIZE), so we need a loop here to put
>>> all the 512 pages.
>>>
>> Hi Wu,
>>
>> Thanks for reminding me that. I truely missed it.
>>
>> And since in register_page_bootmem_info_section(), a whole memory
>> section will be set as SECTION_INFO, I think we don't need to check
>> the page magic one by one, just the first one is enough. :)
>>
>> I will fix it, thanks. :)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v5 08/14] memory-hotplug: Common APIs to support page tables hot-remove
From: Tang Chen @ 2012-12-26 3:11 UTC (permalink / raw)
To: Jianguo Wu
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
yinghai, laijs, linux-kernel, minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50DA65B7.2050707@cn.fujitsu.com>
On 12/26/2012 10:49 AM, Tang Chen wrote:
> On 12/25/2012 04:17 PM, Jianguo Wu wrote:
>>> +
>>> +static void __meminit free_pagetable(struct page *page, int order)
>>> +{
>>> + struct zone *zone;
>>> + bool bootmem = false;
>>> + unsigned long magic;
>>> +
>>> + /* bootmem page has reserved flag */
>>> + if (PageReserved(page)) {
>>> + __ClearPageReserved(page);
>>> + bootmem = true;
>>> +
>>> + magic = (unsigned long)page->lru.next;
>>> + if (magic == SECTION_INFO || magic == MIX_SECTION_INFO)
And also, I think we don't need to check MIX_SECTION_INFO since it is
for the pageblock_flags, not the memmap in the section.
Thanks. :)
>>> + put_page_bootmem(page);
>>
>> Hi Tang,
>>
>> For removing memmap of sparse-vmemmap, in cpu_has_pse case, if magic
>> == SECTION_INFO,
>> the order will be get_order(PMD_SIZE), so we need a loop here to put
>> all the 512 pages.
>>
> Hi Wu,
>
> Thanks for reminding me that. I truely missed it.
>
> And since in register_page_bootmem_info_section(), a whole memory
> section will be set as SECTION_INFO, I think we don't need to check
> the page magic one by one, just the first one is enough. :)
>
> I will fix it, thanks. :)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v5 02/14] memory-hotplug: check whether all memory blocks are offlined or not when removing memory
From: Kamezawa Hiroyuki @ 2012-12-26 3:10 UTC (permalink / raw)
To: Tang Chen
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
wujianguo, yinghai, laijs, linux-kernel, minchan.kim, akpm,
linuxppc-dev
In-Reply-To: <1356350964-13437-3-git-send-email-tangchen@cn.fujitsu.com>
(2012/12/24 21:09), Tang Chen wrote:
> From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
>
> We remove the memory like this:
> 1. lock memory hotplug
> 2. offline a memory block
> 3. unlock memory hotplug
> 4. repeat 1-3 to offline all memory blocks
> 5. lock memory hotplug
> 6. remove memory(TODO)
> 7. unlock memory hotplug
>
> All memory blocks must be offlined before removing memory. But we don't hold
> the lock in the whole operation. So we should check whether all memory blocks
> are offlined before step6. Otherwise, kernel maybe panicked.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
a nitpick below.
> ---
> drivers/base/memory.c | 6 +++++
> include/linux/memory_hotplug.h | 1 +
> mm/memory_hotplug.c | 47 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 987604d..8300a18 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -693,6 +693,12 @@ int offline_memory_block(struct memory_block *mem)
> return ret;
> }
>
> +/* return true if the memory block is offlined, otherwise, return false */
> +bool is_memblock_offlined(struct memory_block *mem)
> +{
> + return mem->state == MEM_OFFLINE;
> +}
> +
> /*
> * Initialize the sysfs support for memory devices...
> */
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 4a45c4e..8dd0950 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -247,6 +247,7 @@ extern int add_memory(int nid, u64 start, u64 size);
> extern int arch_add_memory(int nid, u64 start, u64 size);
> extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
> extern int offline_memory_block(struct memory_block *mem);
> +extern bool is_memblock_offlined(struct memory_block *mem);
> extern int remove_memory(u64 start, u64 size);
> extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
> int nr_pages);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 62e04c9..d43d97b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1430,6 +1430,53 @@ repeat:
> goto repeat;
> }
>
> + lock_memory_hotplug();
> +
> + /*
> + * we have offlined all memory blocks like this:
> + * 1. lock memory hotplug
> + * 2. offline a memory block
> + * 3. unlock memory hotplug
> + *
> + * repeat step1-3 to offline the memory block. All memory blocks
> + * must be offlined before removing memory. But we don't hold the
> + * lock in the whole operation. So we should check whether all
> + * memory blocks are offlined.
> + */
> +
> + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
I prefer adding mem = NULL at the start of this for().
> + section_nr = pfn_to_section_nr(pfn);
> + if (!present_section_nr(section_nr))
> + continue;
> +
> + section = __nr_to_section(section_nr);
> + /* same memblock? */
> + if (mem)
> + if ((section_nr >= mem->start_section_nr) &&
> + (section_nr <= mem->end_section_nr))
> + continue;
> +
Thanks,
-Kame
^ permalink raw reply
* Re: [PATCH v5 08/14] memory-hotplug: Common APIs to support page tables hot-remove
From: Tang Chen @ 2012-12-26 2:49 UTC (permalink / raw)
To: Jianguo Wu
Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux, cl,
linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
yinghai, laijs, linux-kernel, minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50D96116.1070305@huawei.com>
On 12/25/2012 04:17 PM, Jianguo Wu wrote:
>> +
>> +static void __meminit free_pagetable(struct page *page, int order)
>> +{
>> + struct zone *zone;
>> + bool bootmem = false;
>> + unsigned long magic;
>> +
>> + /* bootmem page has reserved flag */
>> + if (PageReserved(page)) {
>> + __ClearPageReserved(page);
>> + bootmem = true;
>> +
>> + magic = (unsigned long)page->lru.next;
>> + if (magic == SECTION_INFO || magic == MIX_SECTION_INFO)
>> + put_page_bootmem(page);
>
> Hi Tang,
>
> For removing memmap of sparse-vmemmap, in cpu_has_pse case, if magic == SECTION_INFO,
> the order will be get_order(PMD_SIZE), so we need a loop here to put all the 512 pages.
>
Hi Wu,
Thanks for reminding me that. I truely missed it.
And since in register_page_bootmem_info_section(), a whole memory
section will be set as SECTION_INFO, I think we don't need to check
the page magic one by one, just the first one is enough. :)
I will fix it, thanks. :)
^ 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