* Re: Badness with the kernel version 2.6.35-rc1-git1 running on P6 box
From: Eric Dumazet @ 2010-07-16 12:20 UTC (permalink / raw)
To: divya; +Cc: sachinp, netdev, LKML, linuxppc-dev, Jan-Bernd Themann,
David Miller
In-Reply-To: <1279274185.2549.14.camel@edumazet-laptop>
Le vendredi 16 juillet 2010 à 11:56 +0200, Eric Dumazet a écrit :
> [PATCH] ehea: ehea_get_stats() should use GFP_KERNEL
>
> ehea_get_stats() is called in process context and should use GFP_KERNEL
> allocation instead of GFP_ATOMIC.
>
> Clearing stats at beginning of ehea_get_stats() is racy in case of
> concurrent stat readers.
>
> get_stats() can also use netdev net_device_stats, instead of a private
> copy.
>
> Reported-by: divya <dipraksh@linux.vnet.ibm.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> drivers/net/ehea/ehea.h | 1 -
> drivers/net/ehea/ehea_main.c | 6 ++----
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
>
Hmm, net-next-2.6 contains following patch :
commit 3d8009c780ee90fccb5c171caf30aff839f13547
Author: Brian King <brking@linux.vnet.ibm.com>
Date: Wed Jun 30 11:59:12 2010 +0000
ehea: Allocate stats buffer with GFP_KERNEL
Since ehea_get_stats calls ehea_h_query_ehea_port, which
can sleep, we can also sleep when allocating a page in
this function. This fixes some memory allocation failure
warnings seen under low memory conditions.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 8b92acb..3beba70 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -335,7 +335,7 @@ static struct net_device_stats
*ehea_get_stats(struct net_device *dev)
memset(stats, 0, sizeof(*stats));
- cb2 = (void *)get_zeroed_page(GFP_ATOMIC);
+ cb2 = (void *)get_zeroed_page(GFP_KERNEL);
if (!cb2) {
ehea_error("no mem for cb2");
goto out;
^ permalink raw reply related
* [PATCH 0/2] Removing dead code
From: Christian Dietrich @ 2010-07-16 12:28 UTC (permalink / raw)
To: Milton Miller, Josh Boyer, Matt Porter, Benjamin Herrenschmidt,
Paul Mackerras, Solomon Peachy, David Woodhouse, Mike Frysinger,
Jiri Kosina, Artem Bityutskiy, Alexander Kurz, Russell King,
Ralf Baechle, Manuel Lauss, David S. Miller, Randy Dunlap,
John Linn, Florian Fainelli, Nicolas Pitre, Joe Perches,
Ladislav Michl, David Brown, linuxppc-dev, linux-kernel,
linux-mtd, netdev
Cc: vamos-dev
In-Reply-To: <redwood56-reply-1-miltonm@bga.com>
Hi all!
I merged the two patches from Christoph Egger[1] to remove the
REDWOOD_[456] config depends. And wrote a second patch, which removes
the redwood/mtd mapping module. I hope this is now acceptable to bring
it into the kernel, if this options are really dead.
Regards
Christian Dietrich
[0] http://vamos1.informatik.uni-erlangen.de/
[1] Message-Id: <adba61f63f4439ac17f2e428429f01ae5e65ab15.1279110895.git.siccegge@cs.fau.de>
Christian Dietrich (2):
Remove REDWOOD_[456] config options and conditional code
Removed redwood/mtd mapping
arch/powerpc/platforms/40x/Kconfig | 16 ----
drivers/mtd/maps/Kconfig | 8 --
drivers/mtd/maps/Makefile | 1 -
drivers/mtd/maps/redwood.c | 174 ------------------------------------
drivers/net/Kconfig | 2 +-
drivers/net/smc91x.h | 37 --------
6 files changed, 1 insertions(+), 237 deletions(-)
delete mode 100644 drivers/mtd/maps/redwood.c
^ permalink raw reply
* [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: Christian Dietrich @ 2010-07-16 12:29 UTC (permalink / raw)
To: Milton Miller, Josh Boyer, Matt Porter, Benjamin Herrenschmidt,
Paul Mackerras, Solomon Peachy, David Woodhouse, Mike Frysinger,
Jiri Kosina, Artem Bityutskiy, Alexander Kurz, David S. Miller,
Randy Dunlap, John Linn, Florian Fainelli, Nicolas Pitre,
Joe Perches, Ladislav Michl, David Brown, linuxppc-dev,
linux-kernel, linux-mtd, netdev
Cc: vamos-dev
In-Reply-To: <cover.1279282865.git.qy03fugy@stud.informatik.uni-erlangen.de>
The config options for REDWOOD_[456] were commented out in the powerpc
Kconfig. The ifdefs referencing this options therefore are dead and all
references to this can be removed (Also dependencies in other KConfig
files).
Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/platforms/40x/Kconfig | 16 -------------
drivers/mtd/maps/Kconfig | 2 +-
drivers/mtd/maps/redwood.c | 43 ------------------------------------
drivers/net/Kconfig | 2 +-
drivers/net/smc91x.h | 37 -------------------------------
5 files changed, 2 insertions(+), 98 deletions(-)
diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index ec64264..b721764 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -71,22 +71,6 @@ config MAKALU
help
This option enables support for the AMCC PPC405EX board.
-#config REDWOOD_5
-# bool "Redwood-5"
-# depends on 40x
-# default n
-# select STB03xxx
-# help
-# This option enables support for the IBM STB04 evaluation board.
-
-#config REDWOOD_6
-# bool "Redwood-6"
-# depends on 40x
-# default n
-# select STB03xxx
-# help
-# This option enables support for the IBM STBx25xx evaluation board.
-
#config SYCAMORE
# bool "Sycamore"
# depends on 40x
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index f22bc9f..6629d09 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
config MTD_REDWOOD
tristate "CFI Flash devices mapped on IBM Redwood"
- depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
+ depends on MTD_CFI
help
This enables access routines for the flash chips on the IBM
Redwood board. If you have one of these boards and would like to
diff --git a/drivers/mtd/maps/redwood.c b/drivers/mtd/maps/redwood.c
index 933c0b6..d2c9db0 100644
--- a/drivers/mtd/maps/redwood.c
+++ b/drivers/mtd/maps/redwood.c
@@ -22,8 +22,6 @@
#include <asm/io.h>
-#if !defined (CONFIG_REDWOOD_6)
-
#define WINDOW_ADDR 0xffc00000
#define WINDOW_SIZE 0x00400000
@@ -69,47 +67,6 @@ static struct mtd_partition redwood_flash_partitions[] = {
}
};
-#else /* CONFIG_REDWOOD_6 */
-/* FIXME: the window is bigger - armin */
-#define WINDOW_ADDR 0xff800000
-#define WINDOW_SIZE 0x00800000
-
-#define RW_PART0_OF 0
-#define RW_PART0_SZ 0x400000 /* 4 MiB data */
-#define RW_PART1_OF RW_PART0_OF + RW_PART0_SZ
-#define RW_PART1_SZ 0x10000 /* 64K VPD */
-#define RW_PART2_OF RW_PART1_OF + RW_PART1_SZ
-#define RW_PART2_SZ 0x400000 - (0x10000 + 0x20000)
-#define RW_PART3_OF RW_PART2_OF + RW_PART2_SZ
-#define RW_PART3_SZ 0x20000
-
-static struct mtd_partition redwood_flash_partitions[] = {
- {
- .name = "Redwood filesystem",
- .offset = RW_PART0_OF,
- .size = RW_PART0_SZ
- },
- {
- .name = "Redwood OpenBIOS Vital Product Data",
- .offset = RW_PART1_OF,
- .size = RW_PART1_SZ,
- .mask_flags = MTD_WRITEABLE /* force read-only */
- },
- {
- .name = "Redwood kernel",
- .offset = RW_PART2_OF,
- .size = RW_PART2_SZ
- },
- {
- .name = "Redwood OpenBIOS",
- .offset = RW_PART3_OF,
- .size = RW_PART3_SZ,
- .mask_flags = MTD_WRITEABLE /* force read-only */
- }
-};
-
-#endif /* CONFIG_REDWOOD_6 */
-
struct map_info redwood_flash_map = {
.name = "IBM Redwood",
.size = WINDOW_SIZE,
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ce2fcdd..313d306 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -913,7 +913,7 @@ config SMC91X
tristate "SMC 91C9x/91C1xxx support"
select CRC32
select MII
- depends on ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || \
+ depends on ARM || M32R || SUPERH || \
MIPS || BLACKFIN || MN10300 || COLDFIRE
help
This is a driver for SMC's 91x series of Ethernet chipsets,
diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
index 8d2772c..ee74791 100644
--- a/drivers/net/smc91x.h
+++ b/drivers/net/smc91x.h
@@ -83,43 +83,6 @@ static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
}
}
-#elif defined(CONFIG_REDWOOD_5) || defined(CONFIG_REDWOOD_6)
-
-/* We can only do 16-bit reads and writes in the static memory space. */
-#define SMC_CAN_USE_8BIT 0
-#define SMC_CAN_USE_16BIT 1
-#define SMC_CAN_USE_32BIT 0
-#define SMC_NOWAIT 1
-
-#define SMC_IO_SHIFT 0
-
-#define SMC_inw(a, r) in_be16((volatile u16 *)((a) + (r)))
-#define SMC_outw(v, a, r) out_be16((volatile u16 *)((a) + (r)), v)
-#define SMC_insw(a, r, p, l) \
- do { \
- unsigned long __port = (a) + (r); \
- u16 *__p = (u16 *)(p); \
- int __l = (l); \
- insw(__port, __p, __l); \
- while (__l > 0) { \
- *__p = swab16(*__p); \
- __p++; \
- __l--; \
- } \
- } while (0)
-#define SMC_outsw(a, r, p, l) \
- do { \
- unsigned long __port = (a) + (r); \
- u16 *__p = (u16 *)(p); \
- int __l = (l); \
- while (__l > 0) { \
- /* Believe it or not, the swab isn't needed. */ \
- outw( /* swab16 */ (*__p++), __port); \
- __l--; \
- } \
- } while (0)
-#define SMC_IRQ_FLAGS (0)
-
#elif defined(CONFIG_SA1100_PLEB)
/* We can only do 16-bit reads and writes in the static memory space. */
#define SMC_CAN_USE_8BIT 1
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: Josh Boyer @ 2010-07-16 14:20 UTC (permalink / raw)
To: Christian Dietrich
Cc: Randy Dunlap, linuxppc-dev, Alexander Kurz, Paul Mackerras,
John Linn, David Brown, Ladislav Michl, Solomon Peachy, vamos-dev,
Mike Frysinger, Florian Fainelli, Artem Bityutskiy, Nicolas Pitre,
netdev, linux-kernel, Milton Miller, Jiri Kosina, Joe Perches,
linux-mtd, David Woodhouse, David S. Miller
In-Reply-To: <ca1bb25d203618c3548748f5efb6f125a96c89e0.1279282865.git.qy03fugy@stud.informatik.uni-erlangen.de>
On Fri, Jul 16, 2010 at 02:29:02PM +0200, Christian Dietrich wrote:
>The config options for REDWOOD_[456] were commented out in the powerpc
>Kconfig. The ifdefs referencing this options therefore are dead and all
>references to this can be removed (Also dependencies in other KConfig
>files).
>
>Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
>Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
This seems fine with me.
The only question is which tree it coms through. I'm happy to take it
in via mine if the netdev and MTD people are fine with that. Otherwise,
my ack is below.
Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
josh
>---
> arch/powerpc/platforms/40x/Kconfig | 16 -------------
> drivers/mtd/maps/Kconfig | 2 +-
> drivers/mtd/maps/redwood.c | 43 ------------------------------------
> drivers/net/Kconfig | 2 +-
> drivers/net/smc91x.h | 37 -------------------------------
> 5 files changed, 2 insertions(+), 98 deletions(-)
>
>diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
>index ec64264..b721764 100644
>--- a/arch/powerpc/platforms/40x/Kconfig
>+++ b/arch/powerpc/platforms/40x/Kconfig
>@@ -71,22 +71,6 @@ config MAKALU
> help
> This option enables support for the AMCC PPC405EX board.
>
>-#config REDWOOD_5
>-# bool "Redwood-5"
>-# depends on 40x
>-# default n
>-# select STB03xxx
>-# help
>-# This option enables support for the IBM STB04 evaluation board.
>-
>-#config REDWOOD_6
>-# bool "Redwood-6"
>-# depends on 40x
>-# default n
>-# select STB03xxx
>-# help
>-# This option enables support for the IBM STBx25xx evaluation board.
>-
> #config SYCAMORE
> # bool "Sycamore"
> # depends on 40x
>diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
>index f22bc9f..6629d09 100644
>--- a/drivers/mtd/maps/Kconfig
>+++ b/drivers/mtd/maps/Kconfig
>@@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
>
> config MTD_REDWOOD
> tristate "CFI Flash devices mapped on IBM Redwood"
>- depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
>+ depends on MTD_CFI
> help
> This enables access routines for the flash chips on the IBM
> Redwood board. If you have one of these boards and would like to
>diff --git a/drivers/mtd/maps/redwood.c b/drivers/mtd/maps/redwood.c
>index 933c0b6..d2c9db0 100644
>--- a/drivers/mtd/maps/redwood.c
>+++ b/drivers/mtd/maps/redwood.c
>@@ -22,8 +22,6 @@
>
> #include <asm/io.h>
>
>-#if !defined (CONFIG_REDWOOD_6)
>-
> #define WINDOW_ADDR 0xffc00000
> #define WINDOW_SIZE 0x00400000
>
>@@ -69,47 +67,6 @@ static struct mtd_partition redwood_flash_partitions[] = {
> }
> };
>
>-#else /* CONFIG_REDWOOD_6 */
>-/* FIXME: the window is bigger - armin */
>-#define WINDOW_ADDR 0xff800000
>-#define WINDOW_SIZE 0x00800000
>-
>-#define RW_PART0_OF 0
>-#define RW_PART0_SZ 0x400000 /* 4 MiB data */
>-#define RW_PART1_OF RW_PART0_OF + RW_PART0_SZ
>-#define RW_PART1_SZ 0x10000 /* 64K VPD */
>-#define RW_PART2_OF RW_PART1_OF + RW_PART1_SZ
>-#define RW_PART2_SZ 0x400000 - (0x10000 + 0x20000)
>-#define RW_PART3_OF RW_PART2_OF + RW_PART2_SZ
>-#define RW_PART3_SZ 0x20000
>-
>-static struct mtd_partition redwood_flash_partitions[] = {
>- {
>- .name = "Redwood filesystem",
>- .offset = RW_PART0_OF,
>- .size = RW_PART0_SZ
>- },
>- {
>- .name = "Redwood OpenBIOS Vital Product Data",
>- .offset = RW_PART1_OF,
>- .size = RW_PART1_SZ,
>- .mask_flags = MTD_WRITEABLE /* force read-only */
>- },
>- {
>- .name = "Redwood kernel",
>- .offset = RW_PART2_OF,
>- .size = RW_PART2_SZ
>- },
>- {
>- .name = "Redwood OpenBIOS",
>- .offset = RW_PART3_OF,
>- .size = RW_PART3_SZ,
>- .mask_flags = MTD_WRITEABLE /* force read-only */
>- }
>-};
>-
>-#endif /* CONFIG_REDWOOD_6 */
>-
> struct map_info redwood_flash_map = {
> .name = "IBM Redwood",
> .size = WINDOW_SIZE,
>diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>index ce2fcdd..313d306 100644
>--- a/drivers/net/Kconfig
>+++ b/drivers/net/Kconfig
>@@ -913,7 +913,7 @@ config SMC91X
> tristate "SMC 91C9x/91C1xxx support"
> select CRC32
> select MII
>- depends on ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || \
>+ depends on ARM || M32R || SUPERH || \
> MIPS || BLACKFIN || MN10300 || COLDFIRE
> help
> This is a driver for SMC's 91x series of Ethernet chipsets,
>diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
>index 8d2772c..ee74791 100644
>--- a/drivers/net/smc91x.h
>+++ b/drivers/net/smc91x.h
>@@ -83,43 +83,6 @@ static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
> }
> }
>
>-#elif defined(CONFIG_REDWOOD_5) || defined(CONFIG_REDWOOD_6)
>-
>-/* We can only do 16-bit reads and writes in the static memory space. */
>-#define SMC_CAN_USE_8BIT 0
>-#define SMC_CAN_USE_16BIT 1
>-#define SMC_CAN_USE_32BIT 0
>-#define SMC_NOWAIT 1
>-
>-#define SMC_IO_SHIFT 0
>-
>-#define SMC_inw(a, r) in_be16((volatile u16 *)((a) + (r)))
>-#define SMC_outw(v, a, r) out_be16((volatile u16 *)((a) + (r)), v)
>-#define SMC_insw(a, r, p, l) \
>- do { \
>- unsigned long __port = (a) + (r); \
>- u16 *__p = (u16 *)(p); \
>- int __l = (l); \
>- insw(__port, __p, __l); \
>- while (__l > 0) { \
>- *__p = swab16(*__p); \
>- __p++; \
>- __l--; \
>- } \
>- } while (0)
>-#define SMC_outsw(a, r, p, l) \
>- do { \
>- unsigned long __port = (a) + (r); \
>- u16 *__p = (u16 *)(p); \
>- int __l = (l); \
>- while (__l > 0) { \
>- /* Believe it or not, the swab isn't needed. */ \
>- outw( /* swab16 */ (*__p++), __port); \
>- __l--; \
>- } \
>- } while (0)
>-#define SMC_IRQ_FLAGS (0)
>-
> #elif defined(CONFIG_SA1100_PLEB)
> /* We can only do 16-bit reads and writes in the static memory space. */
> #define SMC_CAN_USE_8BIT 1
>--
>1.7.0.4
>
^ permalink raw reply
* Re: [PATCH 1/5] v2 Split the memory_block structure
From: Nathan Fontenot @ 2010-07-16 15:29 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <20100716090637.3654f91d.kamezawa.hiroyu@jp.fujitsu.com>
Thanks for taking a look a this Kame, answers below...
-Nathan
On 07/15/2010 07:06 PM, KAMEZAWA Hiroyuki wrote:
> On Thu, 15 Jul 2010 13:37:51 -0500
> Nathan Fontenot <nfont@austin.ibm.com> wrote:
>
>> Split the memory_block struct into a memory_block
>> struct to cover each sysfs directory and a new memory_block_section
>> struct for each memory section covered by the sysfs directory.
>> This change allows for creation of memory sysfs directories that
>> can span multiple memory sections.
>>
>> This can be beneficial in that it can reduce the number of memory
>> sysfs directories created at boot. This also allows different
>> architectures to define how many memory sections are covered by
>> a sysfs directory.
>>
>> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
>> ---
>> drivers/base/memory.c | 222 ++++++++++++++++++++++++++++++++++---------------
>> include/linux/memory.h | 11 +-
>> 2 files changed, 167 insertions(+), 66 deletions(-)
>>
>> Index: linux-2.6/drivers/base/memory.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/base/memory.c 2010-07-15 08:48:41.000000000 -0500
>> +++ linux-2.6/drivers/base/memory.c 2010-07-15 09:55:54.000000000 -0500
>> @@ -28,6 +28,14 @@
>> #include <asm/uaccess.h>
>>
>> #define MEMORY_CLASS_NAME "memory"
>> +#define MIN_MEMORY_BLOCK_SIZE (1 << SECTION_SIZE_BITS)
>> +
>> +static int sections_per_block;
>> +
>> +static inline int base_memory_block_id(int section_nr)
>> +{
>> + return (section_nr / sections_per_block) * sections_per_block;
>> +}
>>
>> static struct sysdev_class memory_sysdev_class = {
>> .name = MEMORY_CLASS_NAME,
>> @@ -94,10 +102,9 @@
>> }
>>
>> static void
>> -unregister_memory(struct memory_block *memory, struct mem_section *section)
>> +unregister_memory(struct memory_block *memory)
>> {
>> BUG_ON(memory->sysdev.cls != &memory_sysdev_class);
>> - BUG_ON(memory->sysdev.id != __section_nr(section));
>>
>> /* drop the ref. we got in remove_memory_block() */
>> kobject_put(&memory->sysdev.kobj);
>> @@ -123,13 +130,20 @@
>> static ssize_t show_mem_removable(struct sys_device *dev,
>> struct sysdev_attribute *attr, char *buf)
>> {
>> + struct memory_block *mem;
>> + struct memory_block_section *mbs;
>> unsigned long start_pfn;
>> - int ret;
>> - struct memory_block *mem =
>> - container_of(dev, struct memory_block, sysdev);
>> + int ret = 1;
>> +
>> + mem = container_of(dev, struct memory_block, sysdev);
>> + mutex_lock(&mem->state_mutex);
>>
>> - start_pfn = section_nr_to_pfn(mem->phys_index);
>> - ret = is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + start_pfn = section_nr_to_pfn(mbs->phys_index);
>> + ret &= is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
>> + }
>> +
>> + mutex_unlock(&mem->state_mutex);
>
> Hmm, this means memory cab be offlined the while memory block section. Right ?
> Please write this fact in patch description...
> And Documentaion/memory_hotplug.txt as "From user's perspective, memory section
> is not a unit of memory hotplug anymore".
> And descirbe about a new rule.
You are correct. A memory block is removable only if all of the memory
sections contained within the memory block are removable.
I will include a documentation patch with v3 of the patches to explain this
and to explain that memory add/remove operations are done on a per memory
block basis.
>
>
>> return sprintf(buf, "%d\n", ret);
>> }
>>
>> @@ -182,16 +196,16 @@
>> * OK to have direct references to sparsemem variables in here.
>> */
>> static int
>> -memory_block_action(struct memory_block *mem, unsigned long action)
>> +memory_block_action(struct memory_block_section *mbs, unsigned long action)
>> {
>> int i;
>> unsigned long psection;
>> unsigned long start_pfn, start_paddr;
>> struct page *first_page;
>> int ret;
>> - int old_state = mem->state;
>> + int old_state = mbs->state;
>>
>> - psection = mem->phys_index;
>> + psection = mbs->phys_index;
>> first_page = pfn_to_page(psection << PFN_SECTION_SHIFT);
>>
>> /*
>> @@ -217,18 +231,18 @@
>> ret = online_pages(start_pfn, PAGES_PER_SECTION);
>> break;
>> case MEM_OFFLINE:
>> - mem->state = MEM_GOING_OFFLINE;
>> + mbs->state = MEM_GOING_OFFLINE;
>> start_paddr = page_to_pfn(first_page) << PAGE_SHIFT;
>> ret = remove_memory(start_paddr,
>> PAGES_PER_SECTION << PAGE_SHIFT);
>> if (ret) {
>> - mem->state = old_state;
>> + mbs->state = old_state;
>> break;
>> }
>> break;
>> default:
>> WARN(1, KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
>> - __func__, mem, action, action);
>> + __func__, mbs, action, action);
>> ret = -EINVAL;
>> }
>>
>> @@ -238,19 +252,34 @@
>
> And please check quilt's diff option.
> Usual patche in ML shows a function name in any changes, as
> @@ -241,6 +293,8 @@ static int memory_block_change_state(str
>
> Maybe "-p" option is lacked..
sorry about that. I'm just using the default options with quilt. I'll
play around with it to why this is happening.
>
>
>> static int memory_block_change_state(struct memory_block *mem,
>> unsigned long to_state, unsigned long from_state_req)
>> {
>> + struct memory_block_section *mbs;
>> int ret = 0;
>> +
>> mutex_lock(&mem->state_mutex);
>>
>> - if (mem->state != from_state_req) {
>> - ret = -EINVAL;
>> - goto out;
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + if (mbs->state != from_state_req)
>> + continue;
>> +
>> + ret = memory_block_action(mbs, to_state);
>> + if (ret)
>> + break;
>> + }
>> +
>> + if (ret) {
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + if (mbs->state == from_state_req)
>> + continue;
>> +
>> + if (memory_block_action(mbs, to_state))
>> + printk(KERN_ERR "Could not re-enable memory "
>> + "section %lx\n", mbs->phys_index);
>
> Why re-enable only ? online->fail->offline never happens ?
> If so, please add comment at least.
This should handle both conditions. If we fail to move all of the memory
sections to the 'to_state', it puts all of the memory sections back to the
'from_state_req'.
> BTW, is it guaranteed that all sections under a block has same state after
> boot ?
Yes, during boot all memory sections are marked online.
>
>> + }
>> }
>>
>> - ret = memory_block_action(mem, to_state);
>> if (!ret)
>> mem->state = to_state;
>>
>> -out:
>> mutex_unlock(&mem->state_mutex);
>> return ret;
>> }
>> @@ -260,20 +289,15 @@
>> struct sysdev_attribute *attr, const char *buf, size_t count)
>> {
>> struct memory_block *mem;
>> - unsigned int phys_section_nr;
>> int ret = -EINVAL;
>>
>> mem = container_of(dev, struct memory_block, sysdev);
>> - phys_section_nr = mem->phys_index;
>> -
>> - if (!present_section_nr(phys_section_nr))
>> - goto out;
>>
> I'm sorry but I couldn't remember why this check was necessary...
Not sure either, it appears that it is there to ensure that the memory
section we are trying to act on is actually present.
>
>
>
>> if (!strncmp(buf, "online", min((int)count, 6)))
>> ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
>> else if(!strncmp(buf, "offline", min((int)count, 7)))
>> ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
>> -out:
>> +
>> if (ret)
>> return ret;
>> return count;
>> @@ -435,39 +459,6 @@
>> return 0;
>> }
>>
>> -static int add_memory_block(int nid, struct mem_section *section,
>> - unsigned long state, enum mem_add_context context)
>> -{
>> - struct memory_block *mem = kzalloc(sizeof(*mem), GFP_KERNEL);
>> - unsigned long start_pfn;
>> - int ret = 0;
>> -
>> - if (!mem)
>> - return -ENOMEM;
>> -
>> - mem->phys_index = __section_nr(section);
>> - mem->state = state;
>> - mutex_init(&mem->state_mutex);
>> - start_pfn = section_nr_to_pfn(mem->phys_index);
>> - mem->phys_device = arch_get_memory_phys_device(start_pfn);
>> -
>> - ret = register_memory(mem, section);
>> - if (!ret)
>> - ret = mem_create_simple_file(mem, phys_index);
>> - if (!ret)
>> - ret = mem_create_simple_file(mem, state);
>> - if (!ret)
>> - ret = mem_create_simple_file(mem, phys_device);
>> - if (!ret)
>> - ret = mem_create_simple_file(mem, removable);
>> - if (!ret) {
>> - if (context == HOTPLUG)
>> - ret = register_mem_sect_under_node(mem, nid);
>> - }
>> -
>> - return ret;
>> -}
>> -
>
> I don't say strongly but this kind of move-code should be done in another patch.
ok, I will move the code move piece to a differnet patch.
>
>
>> /*
>> * For now, we have a linear search to go find the appropriate
>> * memory_block corresponding to a particular phys_index. If
>> @@ -482,12 +473,13 @@
>> struct sys_device *sysdev;
>> struct memory_block *mem;
>> char name[sizeof(MEMORY_CLASS_NAME) + 9 + 1];
>> + int block_id = base_memory_block_id(__section_nr(section));
>>
>> /*
>> * This only works because we know that section == sysdev->id
>> * slightly redundant with sysdev_register()
>> */
>> - sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, __section_nr(section));
>> + sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, block_id);
>>
>> kobj = kset_find_obj(&memory_sysdev_class.kset, name);
>> if (!kobj)
>> @@ -499,18 +491,98 @@
>> return mem;
>> }
>>
>> +static int add_mem_block_section(struct memory_block *mem,
>> + int section_nr, unsigned long state)
>> +{
>> + struct memory_block_section *mbs;
>> +
>> + mbs = kzalloc(sizeof(*mbs), GFP_KERNEL);
>> + if (!mbs)
>> + return -ENOMEM;
>> +
>> + mbs->phys_index = section_nr;
>> + mbs->state = state;
>> +
>> + list_add(&mbs->next, &mem->sections);
>> + return 0;
>> +}
>
> Doesn't this "sections" need to be sorted ? Hmm.
We could, but I cannot think of anything we gain by sorting it.
>
>
>> +
>> +static int add_memory_block(int nid, struct mem_section *section,
>> + unsigned long state, enum mem_add_context context)
>> +{
>> + struct memory_block *mem;
>> + int ret = 0;
>> +
>> + mem = find_memory_block(section);
>> + if (!mem) {
>> + unsigned long start_pfn;
>> +
>> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
>> + if (!mem)
>> + return -ENOMEM;
>> +
>> + mem->state = state;
>> + mutex_init(&mem->state_mutex);
>> + start_pfn = section_nr_to_pfn(__section_nr(section));
>> + mem->phys_device = arch_get_memory_phys_device(start_pfn);
>> + INIT_LIST_HEAD(&mem->sections);
>> +
>> + mutex_lock(&mem->state_mutex);
>> +
>> + ret = register_memory(mem, section);
>> + if (!ret)
>> + ret = mem_create_simple_file(mem, phys_index);
>> + if (!ret)
>> + ret = mem_create_simple_file(mem, state);
>> + if (!ret)
>> + ret = mem_create_simple_file(mem, phys_device);
>> + if (!ret)
>> + ret = mem_create_simple_file(mem, removable);
>> + if (!ret) {
>> + if (context == HOTPLUG)
>> + ret = register_mem_sect_under_node(mem, nid);
>> + }
>> + } else {
>> + kobject_put(&mem->sysdev.kobj);
>> + mutex_lock(&mem->state_mutex);
>> + }
>> +
>> + if (!ret)
>> + ret = add_mem_block_section(mem, __section_nr(section), state);
>> +
>> + mutex_unlock(&mem->state_mutex);
>> + return ret;
>> +}
>> +
>> int remove_memory_block(unsigned long node_id, struct mem_section *section,
>> int phys_device)
>> {
>> struct memory_block *mem;
>> + struct memory_block_section *mbs, *tmp;
>> + int section_nr = __section_nr(section);
>>
>> mem = find_memory_block(section);
>> - unregister_mem_sect_under_nodes(mem);
>> - mem_remove_simple_file(mem, phys_index);
>> - mem_remove_simple_file(mem, state);
>> - mem_remove_simple_file(mem, phys_device);
>> - mem_remove_simple_file(mem, removable);
>> - unregister_memory(mem, section);
>> + mutex_lock(&mem->state_mutex);
>> +
>> + /* remove the specified section */
>> + list_for_each_entry_safe(mbs, tmp, &mem->sections, next) {
>> + if (mbs->phys_index == section_nr) {
>> + list_del(&mbs->next);
>> + kfree(mbs);
>> + }
>> + }
>> +
>> + mutex_unlock(&mem->state_mutex);
>> +
>> + if (list_empty(&mem->sections)) {
>> + unregister_mem_sect_under_nodes(mem);
>> + mem_remove_simple_file(mem, phys_index);
>> + mem_remove_simple_file(mem, state);
>> + mem_remove_simple_file(mem, phys_device);
>> + mem_remove_simple_file(mem, removable);
>> + unregister_memory(mem);
>> + kfree(mem);
>> + }
>>
>> return 0;
>> }
>> @@ -532,6 +604,24 @@
>> return remove_memory_block(0, section, 0);
>> }
>>
>> +u32 __weak memory_block_size(void)
>> +{
>> + return MIN_MEMORY_BLOCK_SIZE;
>> +}
>> +
>> +static u32 get_memory_block_size(void)
>> +{
>> + u32 blk_sz;
>> +
>> + blk_sz = memory_block_size();
>> +
>> + /* Validate blk_sz is a power of 2 and not less than section size */
>> + if ((blk_sz & (blk_sz - 1)) || (blk_sz < MIN_MEMORY_BLOCK_SIZE))
>> + blk_sz = MIN_MEMORY_BLOCK_SIZE;
>> +
>> + return blk_sz;
>> +}
>> +
>> /*
>> * Initialize the sysfs support for memory devices...
>> */
>> @@ -540,12 +630,16 @@
>> unsigned int i;
>> int ret;
>> int err;
>> + int block_sz;
>>
>> memory_sysdev_class.kset.uevent_ops = &memory_uevent_ops;
>> ret = sysdev_class_register(&memory_sysdev_class);
>> if (ret)
>> goto out;
>>
>> + block_sz = get_memory_block_size();
>> + sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
>> +
>> /*
>> * Create entries for memory sections that were found
>> * during boot and have been initialized
>> Index: linux-2.6/include/linux/memory.h
>> ===================================================================
>> --- linux-2.6.orig/include/linux/memory.h 2010-07-15 08:48:41.000000000 -0500
>> +++ linux-2.6/include/linux/memory.h 2010-07-15 09:54:06.000000000 -0500
>> @@ -19,9 +19,15 @@
>> #include <linux/node.h>
>> #include <linux/compiler.h>
>> #include <linux/mutex.h>
>> +#include <linux/list.h>
>>
>> -struct memory_block {
>> +struct memory_block_section {
>> + unsigned long state;
>> unsigned long phys_index;
>> + struct list_head next;
>> +};
>> +
>> +struct memory_block {
>> unsigned long state;
>> /*
>> * This serializes all state change requests. It isn't
>> @@ -34,6 +40,7 @@
>> void *hw; /* optional pointer to fw/hw data */
>> int (*phys_callback)(struct memory_block *);
>> struct sys_device sysdev;
>> + struct list_head sections;
>> };
>>
>> int arch_get_memory_phys_device(unsigned long start_pfn);
>> @@ -113,7 +120,7 @@
>> extern int remove_memory_block(unsigned long, struct mem_section *, int);
>> extern int memory_notify(unsigned long val, void *v);
>> extern int memory_isolate_notify(unsigned long val, void *v);
>> -extern struct memory_block *find_memory_block(unsigned long);
>> +extern struct memory_block *find_memory_block(struct mem_section *);
>> extern int memory_is_hidden(struct mem_section *);
>> #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT)
>> enum mem_add_context { BOOT, HOTPLUG };
>>
>
> Okay, please go ahead. But my 1st impression is that IBM should increase ppc's
> SECTION_SIZE ;)
>
> Thanks,
> -Kame
>
>
>
>
^ permalink raw reply
* Re: [PATCH 2/5] v2 Create new 'end_phys_index' file
From: Nathan Fontenot @ 2010-07-16 15:36 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <20100716090857.5e5c91a3.kamezawa.hiroyu@jp.fujitsu.com>
On 07/15/2010 07:08 PM, KAMEZAWA Hiroyuki wrote:
> On Thu, 15 Jul 2010 13:38:52 -0500
> Nathan Fontenot <nfont@austin.ibm.com> wrote:
>
>> Add a new 'end_phys_index' file to each memory sysfs directory to
>> report the physical index of the last memory section
>> covered by the sysfs directory.
>>
>> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
>
> Does memory_block have to be contiguous between [phys_index, end_phys_index] ?
> Should we provide "# of sections" or "amount of memory under a block" ?
Good point. There is nothing that guarantees that a memory block contains
the contiguous memory sections [phys_index, end_phys_index]. Should there be
a 'memory_sections' file that list the memory sections present in a memory block?
Something along the lines of;
#> cat memory0/memory_sections
0,1,2,3
This could be done instead of the end_phys_index file.
-Nathan
>
> No objections to end_phys_index...buf plz fix diff style.
>
> Thanks,
> -Kame
>
>
>> ---
>> drivers/base/memory.c | 14 +++++++++++++-
>> include/linux/memory.h | 3 +++
>> 2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> Index: linux-2.6/drivers/base/memory.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/base/memory.c 2010-07-15 09:55:54.000000000 -0500
>> +++ linux-2.6/drivers/base/memory.c 2010-07-15 09:56:05.000000000 -0500
>> @@ -121,7 +121,15 @@
>> {
>> struct memory_block *mem =
>> container_of(dev, struct memory_block, sysdev);
>> - return sprintf(buf, "%08lx\n", mem->phys_index);
>> + return sprintf(buf, "%08lx\n", mem->start_phys_index);
>> +}
>> +
>> +static ssize_t show_mem_end_phys_index(struct sys_device *dev,
>> + struct sysdev_attribute *attr, char *buf)
>> +{
>> + struct memory_block *mem =
>> + container_of(dev, struct memory_block, sysdev);
>> + return sprintf(buf, "%08lx\n", mem->end_phys_index);
>> }
>>
>> /*
>> @@ -321,6 +329,7 @@
>> }
>>
>> static SYSDEV_ATTR(phys_index, 0444, show_mem_phys_index, NULL);
>> +static SYSDEV_ATTR(end_phys_index, 0444, show_mem_end_phys_index, NULL);
>> static SYSDEV_ATTR(state, 0644, show_mem_state, store_mem_state);
>> static SYSDEV_ATTR(phys_device, 0444, show_phys_device, NULL);
>> static SYSDEV_ATTR(removable, 0444, show_mem_removable, NULL);
>> @@ -533,6 +542,8 @@
>> if (!ret)
>> ret = mem_create_simple_file(mem, phys_index);
>> if (!ret)
>> + ret = mem_create_simple_file(mem, end_phys_index);
>> + if (!ret)
>> ret = mem_create_simple_file(mem, state);
>> if (!ret)
>> ret = mem_create_simple_file(mem, phys_device);
>> @@ -577,6 +588,7 @@
>> if (list_empty(&mem->sections)) {
>> unregister_mem_sect_under_nodes(mem);
>> mem_remove_simple_file(mem, phys_index);
>> + mem_remove_simple_file(mem, end_phys_index);
>> mem_remove_simple_file(mem, state);
>> mem_remove_simple_file(mem, phys_device);
>> mem_remove_simple_file(mem, removable);
>> Index: linux-2.6/include/linux/memory.h
>> ===================================================================
>> --- linux-2.6.orig/include/linux/memory.h 2010-07-15 09:54:06.000000000 -0500
>> +++ linux-2.6/include/linux/memory.h 2010-07-15 09:56:05.000000000 -0500
>> @@ -29,6 +29,9 @@
>>
>> struct memory_block {
>> unsigned long state;
>> + unsigned long start_phys_index;
>> + unsigned long end_phys_index;
>> +
>> /*
>> * This serializes all state change requests. It isn't
>> * held during creation because the control files are
>>
>> --
>> 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 4/5] v2 Update sysfs node routines for new sysfs memory directories
From: Nathan Fontenot @ 2010-07-16 15:40 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <20100716091239.69f40e47.kamezawa.hiroyu@jp.fujitsu.com>
On 07/15/2010 07:12 PM, KAMEZAWA Hiroyuki wrote:
> On Thu, 15 Jul 2010 13:40:40 -0500
> Nathan Fontenot <nfont@austin.ibm.com> wrote:
>
>> Update the node sysfs directory routines that create
>> links to the memory sysfs directories under each node.
>> This update makes the node code aware that a memory sysfs
>> directory can cover multiple memory sections.
>>
>> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
>
> Shouldn't "static int link_mem_sections(int nid)" be update ?
> It does
> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> register..
>
No, although the name 'link_mem_sections' does imply that it should. The
range of start_pfn..end_pfn examined in this routine is the range of pfn's
covered by the entire node, not a memory_block.
-Nathan
> Thanks,
> -Kame
>
>
>> ---
>> drivers/base/node.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> Index: linux-2.6/drivers/base/node.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/base/node.c 2010-07-15 09:54:06.000000000 -0500
>> +++ linux-2.6/drivers/base/node.c 2010-07-15 09:56:16.000000000 -0500
>> @@ -346,8 +346,10 @@
>> return -EFAULT;
>> if (!node_online(nid))
>> return 0;
>> - sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
>> - sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
>> +
>> + sect_start_pfn = section_nr_to_pfn(mem_blk->start_phys_index);
>> + sect_end_pfn = section_nr_to_pfn(mem_blk->end_phys_index);
>> + sect_end_pfn += PAGES_PER_SECTION - 1;
>> for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
>> int page_nid;
>>
>> @@ -383,8 +385,10 @@
>> if (!unlinked_nodes)
>> return -ENOMEM;
>> nodes_clear(*unlinked_nodes);
>> - sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
>> - sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
>> +
>> + sect_start_pfn = section_nr_to_pfn(mem_blk->start_phys_index);
>> + sect_end_pfn = section_nr_to_pfn(mem_blk->end_phys_index);
>> + sect_end_pfn += PAGES_PER_SECTION - 1;
>> for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
>> int nid;
>>
>>
>> --
>> 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 0/7] De-couple sysfs memory directories from memory sections
From: Nathan Fontenot @ 2010-07-16 15:41 UTC (permalink / raw)
To: Greg KH; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20100716071320.GC31612@kroah.com>
On 07/16/2010 02:13 AM, Greg KH wrote:
> On Mon, Jul 12, 2010 at 10:27:02AM -0500, Nathan Fontenot wrote:
>> This set of patches de-couples the idea that there is a single
>> directory in sysfs for each memory section.
>
> Any reason you didn't cc: the sysfs maintainer on these patches? If
> not, I'll gladly ignore them...
Ummm.... I have no good excuse. :)
Will add you to cc for v3 of the patch.
>
> (hint, scripts/get_maintainer.pl is your friend...)
Thanks.
-Nathan
>
> greg k-h
^ permalink raw reply
* Re: [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: Milton Miller @ 2010-07-16 15:45 UTC (permalink / raw)
To: Christian Dietrich
Cc: Randy Dunlap, linuxppc-dev, Alexander Kurz, Paul Mackerras,
John Linn, David Brown, Ladislav Michl, Solomon Peachy, vamos-dev,
Mike Frysinger, Florian Fainelli, Artem Bityutskiy, Nicolas Pitre,
Jiri Kosina, linux-kernel, Milton Miller, netdev, Joe Perches,
linux-mtd, David Woodhouse, David S. Miller
In-Reply-To: <20100716142055.GA11736@zod.rchland.ibm.com>
On Fri, 16 Jul 2010 at about 08:20:55 -0600 Josh Boyer wrote:
> On Fri, Jul 16, 2010 at 02:29:02PM +0200, Christian Dietrich wrote:
> > The config options for REDWOOD_[456] were commented out in the powerpc
> > Kconfig. The ifdefs referencing this options therefore are dead and all
> > references to this can be removed (Also dependencies in other KConfig
> > files).
> This seems fine with me.
>
> The only question is which tree it coms through. I'm happy to take it
> in via mine if the netdev and MTD people are fine with that. Otherwise,
> my ack is below.
> On Fri, 16 Jul 2010 around 14:29:08 +0200 Christian Dietrich wrote:
> > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> > index f22bc9f..6629d09 100644
> > --- a/drivers/mtd/maps/Kconfig
> > +++ b/drivers/mtd/maps/Kconfig
> > @@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
> >
> > config MTD_REDWOOD
> > tristate "CFI Flash devices mapped on IBM Redwood"
> > - depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
> > + depends on MTD_CFI
> > help
> > This enables access routines for the flash chips on the IBM
> > Redwood board. If you have one of these boards and would like to
> > diff --git a/drivers/mtd/maps/redwood.c b/drivers/mtd/maps/redwood.c
> > index 933c0b6..d2c9db0 100644
> > --- a/drivers/mtd/maps/redwood.c
> > +++ b/drivers/mtd/maps/redwood.c
> > @@ -22,8 +22,6 @@
The patches are unnecssarly coupled by removing the REDWOOD_* symbols
in the MTD area before removing the files and Kconfig completely in
the second patch. This could easily be eliminated by pushing the
two fragments into the second patch.
milton
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Catalin Marinas @ 2010-07-16 16:03 UTC (permalink / raw)
To: Grant Likely
Cc: Daniel Walker, linux-kbuild, Tony Lindgren, Nicolas Pitre,
linux-kernel, Linus Torvalds, Russell King, Uwe Kleine-König,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20100713230352.6781.18644.stgit@angua>
On Wed, 2010-07-14 at 00:04 +0100, Grant Likely wrote:
> - It still doesn't resolve dependencies. A solver would help with this.
> For the time being I work around the problem by running the generated
> config through 'oldconfig' and looking for differences. If the files
> differ (ignoring comments and generateconfig_* options) after oldconfig,
> then the <board>_defconfig target returns a failure. (but leaves the
> new .config intact so the user can resolve it with menuconfig). This
> way at least the user is told when a Kconfig fragment is invalid.
It's not a solver but I'm pushing a patch to warn on selecting symbols
with unmet dependencies so that you can select further symbols (manual
solving). The patch is in linux-next but you also can grab it from:
http://git.kernel.org/?p=linux/kernel/git/cmarinas/linux-2.6-cm.git;a=commitdiff_plain;h=5d87db2d2a332784bbf2b1ec3e141486f4d41d6f
--
Catalin
^ permalink raw reply
* Re: [PATCH 1/5] v2 Split the memory_block structure
From: Dave Hansen @ 2010-07-16 17:15 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linux-mm, linux-kernel, KAMEZAWA Hiroyuki, linuxppc-dev
In-Reply-To: <4C3F557F.3000304@austin.ibm.com>
On Thu, 2010-07-15 at 13:37 -0500, Nathan Fontenot wrote:
> @@ -123,13 +130,20 @@
> static ssize_t show_mem_removable(struct sys_device *dev,
> struct sysdev_attribute *attr, char *buf)
> {
> + struct memory_block *mem;
> + struct memory_block_section *mbs;
> unsigned long start_pfn;
> - int ret;
> - struct memory_block *mem =
> - container_of(dev, struct memory_block, sysdev);
> + int ret = 1;
> +
> + mem = container_of(dev, struct memory_block, sysdev);
> + mutex_lock(&mem->state_mutex);
>
> - start_pfn = section_nr_to_pfn(mem->phys_index);
> - ret = is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
> + list_for_each_entry(mbs, &mem->sections, next) {
> + start_pfn = section_nr_to_pfn(mbs->phys_index);
> + ret &= is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
> + }
> +
> + mutex_unlock(&mem->state_mutex);
> return sprintf(buf, "%d\n", ret);
> }
Now that the "state_mutex" is getting used for other stuff, should we
just make it "mutex"?
> @@ -182,16 +196,16 @@
> * OK to have direct references to sparsemem variables in here.
> */
> static int
> -memory_block_action(struct memory_block *mem, unsigned long action)
> +memory_block_action(struct memory_block_section *mbs, unsigned long action)
> {
> int i;
> unsigned long psection;
> unsigned long start_pfn, start_paddr;
> struct page *first_page;
> int ret;
> - int old_state = mem->state;
> + int old_state = mbs->state;
>
> - psection = mem->phys_index;
> + psection = mbs->phys_index;
> first_page = pfn_to_page(psection << PFN_SECTION_SHIFT);
>
> /*
> @@ -217,18 +231,18 @@
> ret = online_pages(start_pfn, PAGES_PER_SECTION);
> break;
> case MEM_OFFLINE:
> - mem->state = MEM_GOING_OFFLINE;
> + mbs->state = MEM_GOING_OFFLINE;
> start_paddr = page_to_pfn(first_page) << PAGE_SHIFT;
> ret = remove_memory(start_paddr,
> PAGES_PER_SECTION << PAGE_SHIFT);
> if (ret) {
> - mem->state = old_state;
> + mbs->state = old_state;
> break;
> }
> break;
> default:
> WARN(1, KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
> - __func__, mem, action, action);
> + __func__, mbs, action, action);
> ret = -EINVAL;
> }
>
> @@ -238,19 +252,34 @@
> static int memory_block_change_state(struct memory_block *mem,
> unsigned long to_state, unsigned long from_state_req)
> {
> + struct memory_block_section *mbs;
> int ret = 0;
> +
> mutex_lock(&mem->state_mutex);
>
> - if (mem->state != from_state_req) {
> - ret = -EINVAL;
> - goto out;
> + list_for_each_entry(mbs, &mem->sections, next) {
> + if (mbs->state != from_state_req)
> + continue;
> +
> + ret = memory_block_action(mbs, to_state);
> + if (ret)
> + break;
> + }
> +
> + if (ret) {
> + list_for_each_entry(mbs, &mem->sections, next) {
> + if (mbs->state == from_state_req)
> + continue;
> +
> + if (memory_block_action(mbs, to_state))
> + printk(KERN_ERR "Could not re-enable memory "
> + "section %lx\n", mbs->phys_index);
> + }
> }
Please just use a goto here. It's nicer looking, and much more in line
with what's there already.
...
> ===================================================================
> --- linux-2.6.orig/include/linux/memory.h 2010-07-15 08:48:41.000000000 -0500
> +++ linux-2.6/include/linux/memory.h 2010-07-15 09:54:06.000000000 -0500
> @@ -19,9 +19,15 @@
> #include <linux/node.h>
> #include <linux/compiler.h>
> #include <linux/mutex.h>
> +#include <linux/list.h>
>
> -struct memory_block {
> +struct memory_block_section {
> + unsigned long state;
> unsigned long phys_index;
> + struct list_head next;
> +};
> +
> +struct memory_block {
> unsigned long state;
> /*
> * This serializes all state change requests. It isn't
> @@ -34,6 +40,7 @@
> void *hw; /* optional pointer to fw/hw data */
> int (*phys_callback)(struct memory_block *);
> struct sys_device sysdev;
> + struct list_head sections;
> };
It looks like we have state in both the memory_block and
memory_block_section. That seems a bit confusing to me. This also
looks like it would permit non-contiguous memory_block_sections in a
memory_block. Is that what you intended?
If the memory_block's state was inferred to be the same as each
memory_block_section, couldn't we just keep a start and end phys_index
in the memory_block, and get away from having memory_block_sections at
all?
-- Dave
^ permalink raw reply
* Re: [PATCH 3/5] v2 Change the mutex name in the memory_block struct
From: Dave Hansen @ 2010-07-16 17:16 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linux-mm, linux-kernel, KAMEZAWA Hiroyuki, linuxppc-dev
In-Reply-To: <4C3F55FC.4050205@austin.ibm.com>
On Thu, 2010-07-15 at 13:39 -0500, Nathan Fontenot wrote:
>
> Change the name of the memory_block mutex since it is now used for
> more than just gating changes to the status of the memory sections
> covered by the memory sysfs directory.
Heh, sorry about the previous comments. :)
You should move this up to be the first in the series.
-- Dave
^ permalink raw reply
* Re: [PATCH 5/5] v2 Enable multiple sections per directory for ppc
From: Dave Hansen @ 2010-07-16 17:18 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linux-mm, linux-kernel, KAMEZAWA Hiroyuki, linuxppc-dev
In-Reply-To: <4C3F5668.2060407@austin.ibm.com>
On Thu, 2010-07-15 at 13:41 -0500, Nathan Fontenot wrote:
> linux-2.6.orig/arch/powerpc/platforms/pseries/hotplug-memory.c 2010-07-15 09:54:06.000000000 -0500
> +++ linux-2.6/arch/powerpc/platforms/pseries/hotplug-memory.c 2010-07-15 09:56:19.000000000 -0500
> @@ -17,6 +17,54 @@
> #include <asm/pSeries_reconfig.h>
> #include <asm/sparsemem.h>
>
> +static u32 get_memblock_size(void)
> +{
> + struct device_node *np;
> + unsigned int memblock_size = 0;
> +
Please give this sucker some units. get_memblock_size_bytes()?
get_memblock_size_in_g0ats()?
-- Dave
^ permalink raw reply
* Re: Badness with the kernel version 2.6.35-rc1-git1 running on P6 box
From: Dave Hansen @ 2010-07-16 17:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: sachinp, netdev, LKML, linuxppc-dev, Jan-Bernd Themann,
David Miller, divya
In-Reply-To: <1279274185.2549.14.camel@edumazet-laptop>
On Fri, 2010-07-16 at 11:56 +0200, Eric Dumazet wrote:
>
> > SLUB: Unable to allocate memory on node -1 (gfp=0x20)
> > cache: kmalloc-16384, object size: 16384, buffer size: 16384,
> default order: 2, min order: 0
> > node 0: slabs: 28, objs: 292, free: 0
> > ip: page allocation failure. order:0, mode:0x8020
> > Call Trace:
> > [c000000006a0eb40] [c000000000011c30] .show_stack+0x6c/0x16c (unreliable)
> > [c000000006a0ebf0] [c00000000012129c] .__alloc_pages_nodemask+0x6a0/0x75c
> > [c000000006a0ed70] [c0000000001527cc] .alloc_pages_current+0xc4/0x104
> > [c000000006a0ee10] [c00000000011fca4] .__get_free_pages+0x18/0x90
> > [c000000006a0ee90] [c0000000004f7058] .ehea_get_stats+0x4c/0x1bc
> > [c000000006a0ef30] [c0000000005a0a04] .dev_get_stats+0x38/0x64
> > [c000000006a0efc0] [c0000000005b456c] .rtnl_fill_ifinfo+0x35c/0x85c
> > [c000000006a0f150] [c0000000005b5920] .rtmsg_ifinfo+0x164/0x204
> > [c000000006a0f210] [c0000000005a6d6c] .dev_change_flags+0x4c/0x7c
> > [c000000006a0f2a0] [c0000000005b50b4] .do_setlink+0x31c/0x750
> > [c000000006a0f3b0] [c0000000005b6724] .rtnl_newlink+0x388/0x618
> > [c000000006a0f5f0] [c0000000005b6350] .rtnetlink_rcv_msg+0x268/0x2b4
> > [c000000006a0f6a0] [c0000000005cfdc0] .netlink_rcv_skb+0x74/0x108
> > [c000000006a0f730] [c0000000005b60c4] .rtnetlink_rcv+0x38/0x5c
> > [c000000006a0f7c0] [c0000000005cf8c8] .netlink_unicast+0x318/0x3f4
> > [c000000006a0f890] [c0000000005d05b4] .netlink_sendmsg+0x2d0/0x310
> > [c000000006a0f970] [c00000000058e1e8] .sock_sendmsg+0xd4/0x110
> > [c000000006a0fb50] [c00000000058e514] .SyS_sendmsg+0x1f4/0x288
> > [c000000006a0fd70] [c00000000058c2b8] .SyS_socketcall+0x214/0x280
> > [c000000006a0fe30] [c0000000000085b4] syscall_exit+0x0/0x40
> > Mem-Info:
> > Node 0 DMA per-cpu:
> > CPU 0: hi: 0, btch: 1 usd: 0
> > CPU 1: hi: 0, btch: 1 usd: 0
> > CPU 2: hi: 0, btch: 1 usd: 0
> > CPU 3: hi: 0, btch: 1 usd: 0
> >
> > The mainline 2.6.35-rc5 worked fine.
>
> Maybe you were lucky with 2.6.35-rc5
>
> Anyway ehea should not use GFP_ATOMIC in its ehea_get_stats() method,
> called in process context, but GFP_KERNEL.
>
> Another patch is needed for ehea_refill_rq_def() as well.
You're right that this is abusing GFP_ATOMIC.
But is, this is just a normal "GFP_ATOMIC" allocation failure? "SLUB:
Unable to allocate memory on node -1" seems like a somewhat
inappropriate error message for that.
It isn't immediately obvious where the -1 is coming from. Does it truly
mean "allocate from any node" here, or is that a buglet in and of
itself?
-- Dave
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Grant Likely @ 2010-07-16 17:57 UTC (permalink / raw)
To: Catalin Marinas
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Nicolas Pitre, linux-kernel, Linus Torvalds, Russell King,
Uwe Kleine-König, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1279296221.17878.66.camel@e102109-lin.cambridge.arm.com>
On Fri, Jul 16, 2010 at 10:03 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Wed, 2010-07-14 at 00:04 +0100, Grant Likely wrote:
>> - It still doesn't resolve dependencies. =A0A solver would help with thi=
s.
>> =A0 For the time being I work around the problem by running the generate=
d
>> =A0 config through 'oldconfig' and looking for differences. =A0If the fi=
les
>> =A0 differ (ignoring comments and generateconfig_* options) after oldcon=
fig,
>> =A0 then the <board>_defconfig target returns a failure. =A0(but leaves =
the
>> =A0 new .config intact so the user can resolve it with menuconfig). =A0T=
his
>> =A0 way at least the user is told when a Kconfig fragment is invalid.
>
> It's not a solver but I'm pushing a patch to warn on selecting symbols
> with unmet dependencies so that you can select further symbols (manual
> solving). The patch is in linux-next but you also can grab it from:
>
> http://git.kernel.org/?p=3Dlinux/kernel/git/cmarinas/linux-2.6-cm.git;a=
=3Dcommitdiff_plain;h=3D5d87db2d2a332784bbf2b1ec3e141486f4d41d6f
sfr and I were talking about your patch the other day. Just warning
on incomplete dependencies is enough to make it actually workable for
me (without my ugly post-processing step). I was very happy to hear
that it is in linux-next.
Last missing piece is being able to do "select FOO =3D n", which Stephen
is currently working on.
g.
^ permalink raw reply
* Re: cpm_uart_console_write() stuck in waiting for transmitter fifo ready
From: Scott Wood @ 2010-07-16 18:13 UTC (permalink / raw)
To: Shawn Jin; +Cc: ppcdev
In-Reply-To: <AANLkTinVSCwvz5KWbvp4UUcTsd-KZQm0pzfy-XXN_ie=@mail.gmail.com>
On Fri, 16 Jul 2010 00:12:21 -0700
Shawn Jin <shawnxjin@gmail.com> wrote:
> >> Why would the TxBD be filled with all 0xF? Would it be possible that
> >> the bdbase actually points somewhere else other than the TxBD?
> >
> > The virtual address 0xfddfa000 is mapped to 0xfa202000. I suspect that
> > the TxBD of my MPC870 may not start at 0xfa202020.
> >
> > I notice that for adder875, ep88xc and mpc885ads, the muram data's reg
> > = <0 0x1c00> but for mgsuvd, its reg = <0x800 0x1800>. How does the
> > kernel use muram for 885 family SoCs? How much muram should be
> > reserved for data?
> >
> > My RCCR=0x1, meaning the first 512B is for microcode. So the data and
> > the TxBD should really be starting at 0xfa202200? Then my muram data's
> > reg should be <0x200 ?>? What size shall I specify?
>
> After the muram data's reg is changed to <0x200 0x1a00>, the cpm_uart
> driver works properly and the kernel messages are printed on the
> serial port.
The muram node is supposed to show the portions of DPRAM that are
usable by the OS. If some portion has been taken up by microcode (or
anything else not under the OS's control) before the OS has started,
then it must be excluded from the muram node.
-Scott
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Linus Torvalds @ 2010-07-16 18:17 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Nicolas Pitre, linux-kernel,
Uwe Kleine-König, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20100716180701.GA26854@n2100.arm.linux.org.uk>
On Fri, Jul 16, 2010 at 11:07 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>
> I thought Linus' idea was to use:
>
> KBUILD_KCONFIG=file make allnoconfig
See an earlier reply - that is indeed what I suggested, and yes, it
avoids the need to be able to "unselect" things.
However, it turns out that even then you do want to extend the Kconfig
language with the ability to select particular values. Not for boolean
(or even tristate things), but for things that select an integer or
string value etc.
So the "select OPTION=xyz" syntax ends up being a good thing even for
the "-n" (allnoconfig) case too.
And while I think the allnoconfig model has some advantages (the
Kconfig input file ends up being independent of the default values),
that very fact ends up being a disadvantage too (the Kconfig input
file likely ends up being larger, since _hopefully_ the defaults are
sane).
So I'm not at all married to the "allnoconfig" model. It's one way of
doing things, but I think the argument that we should start with the
defaults and modify those instead is not an invalid one.
Linus
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Grant Likely @ 2010-07-16 18:18 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Nicolas Pitre, linux-kernel, Linus Torvalds,
Uwe Kleine-König, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20100716180701.GA26854@n2100.arm.linux.org.uk>
On Fri, Jul 16, 2010 at 12:07 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Jul 16, 2010 at 11:57:55AM -0600, Grant Likely wrote:
>> Last missing piece is being able to do "select FOO =3D n", which Stephen
>> is currently working on.
>
> I thought Linus' idea was to use:
>
> KBUILD_KCONFIG=3Dfile make allnoconfig
That was more a prototype of the idea; but it's a pretty cumbersome
user interface. :-) By changing the makefile to look for kconfig
fragments in the configs directory, the user interface for choosing a
config remains exactly the same.
As for the allnoconfig bit....
> in which case any option which would be presented to the user which hasn'=
t
> been selected by 'file' ends up being set to n. =A0That means there's no
> need for a special "select FOO=3Dn" construct.
...Linus chimed in on this that he doesn't actually care much. I
think defconfig with an empty initial config file makes a lot more
sense than allnoconfig so that we're using the default values from the
normal Kconfig files.
> See one of Linus' replies on June 3:
> Message-ID: <alpine.LFD.2.00.1006031317410.8175@i5.linux-foundation.org>
See this response:
Message-ID: <AANLkTik-QCXFnjma3J28B9h27uajOcDhthTGz99zKgVi@mail.gmail.com>
http://news.gmane.org/find-root.php?message_id=3D%3cAANLkTik%2dQCXFnjma3J28=
B9h27uajOcDhthTGz99zKgVi%40mail.gmail.com%3e
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Nicolas Pitre @ 2010-07-16 18:19 UTC (permalink / raw)
To: Grant Likely
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Uwe Kleine-König, linux-kernel,
Linus Torvalds, Russell King, linuxppc-dev, linux-arm-kernel
In-Reply-To: <AANLkTinyFBTKYaryFANYheTuP-biJUws2x01NRNZPb4M@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2072 bytes --]
On Fri, 16 Jul 2010, Grant Likely wrote:
> On Fri, Jul 16, 2010 at 10:03 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > On Wed, 2010-07-14 at 00:04 +0100, Grant Likely wrote:
> >> - It still doesn't resolve dependencies. A solver would help with this.
> >> For the time being I work around the problem by running the generated
> >> config through 'oldconfig' and looking for differences. If the files
> >> differ (ignoring comments and generateconfig_* options) after oldconfig,
> >> then the <board>_defconfig target returns a failure. (but leaves the
> >> new .config intact so the user can resolve it with menuconfig). This
> >> way at least the user is told when a Kconfig fragment is invalid.
> >
> > It's not a solver but I'm pushing a patch to warn on selecting symbols
> > with unmet dependencies so that you can select further symbols (manual
> > solving). The patch is in linux-next but you also can grab it from:
> >
> > http://git.kernel.org/?p=linux/kernel/git/cmarinas/linux-2.6-cm.git;a=commitdiff_plain;h=5d87db2d2a332784bbf2b1ec3e141486f4d41d6f
>
> sfr and I were talking about your patch the other day. Just warning
> on incomplete dependencies is enough to make it actually workable for
> me (without my ugly post-processing step). I was very happy to hear
> that it is in linux-next.
>
> Last missing piece is being able to do "select FOO = n", which Stephen
> is currently working on.
Instead of (or in addition to) warning for incomplete
dependencies, I'd much prefer if the prerequisites were recursively
selected automatically. This way if some options are moved inside a
submenu at some point with a config symbol for that subcategory
(e.g. CONFIG_NETDEV_1000), or if the subsystem is reorganized into
submodules that are required for some driver to work, then my
config will still be fine.
For example, if I want CONFIG_MTD_CMDLINE_PARTS=y, the system may be
smart enough to notice and automatically enable CONFIG_MTD and
CONFIG_MTD_PARTITIONS without having to carry those in the defconfig.
Nicolas
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Grant Likely @ 2010-07-16 18:21 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Uwe Kleine-König, linux-kernel,
Linus Torvalds, Russell King, linuxppc-dev, linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1007161406120.10598@xanadu.home>
On Fri, Jul 16, 2010 at 12:19 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Fri, 16 Jul 2010, Grant Likely wrote:
>
>> On Fri, Jul 16, 2010 at 10:03 AM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > On Wed, 2010-07-14 at 00:04 +0100, Grant Likely wrote:
>> >> - It still doesn't resolve dependencies. =A0A solver would help with =
this.
>> >> =A0 For the time being I work around the problem by running the gener=
ated
>> >> =A0 config through 'oldconfig' and looking for differences. =A0If the=
files
>> >> =A0 differ (ignoring comments and generateconfig_* options) after old=
config,
>> >> =A0 then the <board>_defconfig target returns a failure. =A0(but leav=
es the
>> >> =A0 new .config intact so the user can resolve it with menuconfig). =
=A0This
>> >> =A0 way at least the user is told when a Kconfig fragment is invalid.
>> >
>> > It's not a solver but I'm pushing a patch to warn on selecting symbols
>> > with unmet dependencies so that you can select further symbols (manual
>> > solving). The patch is in linux-next but you also can grab it from:
>> >
>> > http://git.kernel.org/?p=3Dlinux/kernel/git/cmarinas/linux-2.6-cm.git;=
a=3Dcommitdiff_plain;h=3D5d87db2d2a332784bbf2b1ec3e141486f4d41d6f
>>
>> sfr and I were talking about your patch the other day. =A0Just warning
>> on incomplete dependencies is enough to make it actually workable for
>> me (without my ugly post-processing step). =A0I was very happy to hear
>> that it is in linux-next.
>>
>> Last missing piece is being able to do "select FOO =3D n", which Stephen
>> is currently working on.
>
> Instead of (or in addition to) warning for incomplete
> dependencies, I'd much prefer if the prerequisites were recursively
> selected automatically. =A0This way if some options are moved inside a
> submenu at some point with a config symbol for that subcategory
> (e.g. CONFIG_NETDEV_1000), or if the subsystem is reorganized into
> submodules that are required for some driver to work, then my
> config will still be fine.
>
> For example, if I want CONFIG_MTD_CMDLINE_PARTS=3Dy, the system may be
> smart enough to notice and automatically enable CONFIG_MTD and
> CONFIG_MTD_PARTITIONS without having to carry those in the defconfig.
I fully agree. However, the warnings make the system work now while
we wait for a full solver to be implemented.
g.
^ permalink raw reply
* Re: [PATCH 1/5] v2 Split the memory_block structure
From: Nathan Fontenot @ 2010-07-16 18:23 UTC (permalink / raw)
To: Dave Hansen; +Cc: linux-mm, linux-kernel, KAMEZAWA Hiroyuki, linuxppc-dev
In-Reply-To: <1279300521.9207.222.camel@nimitz>
On 07/16/2010 12:15 PM, Dave Hansen wrote:
> On Thu, 2010-07-15 at 13:37 -0500, Nathan Fontenot wrote:
>> @@ -123,13 +130,20 @@
>> static ssize_t show_mem_removable(struct sys_device *dev,
>> struct sysdev_attribute *attr, char *buf)
>> {
>> + struct memory_block *mem;
>> + struct memory_block_section *mbs;
>> unsigned long start_pfn;
>> - int ret;
>> - struct memory_block *mem =
>> - container_of(dev, struct memory_block, sysdev);
>> + int ret = 1;
>> +
>> + mem = container_of(dev, struct memory_block, sysdev);
>> + mutex_lock(&mem->state_mutex);
>>
>> - start_pfn = section_nr_to_pfn(mem->phys_index);
>> - ret = is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + start_pfn = section_nr_to_pfn(mbs->phys_index);
>> + ret &= is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
>> + }
>> +
>> + mutex_unlock(&mem->state_mutex);
>> return sprintf(buf, "%d\n", ret);
>> }
>
> Now that the "state_mutex" is getting used for other stuff, should we
> just make it "mutex"?
>
>> @@ -182,16 +196,16 @@
>> * OK to have direct references to sparsemem variables in here.
>> */
>> static int
>> -memory_block_action(struct memory_block *mem, unsigned long action)
>> +memory_block_action(struct memory_block_section *mbs, unsigned long action)
>> {
>> int i;
>> unsigned long psection;
>> unsigned long start_pfn, start_paddr;
>> struct page *first_page;
>> int ret;
>> - int old_state = mem->state;
>> + int old_state = mbs->state;
>>
>> - psection = mem->phys_index;
>> + psection = mbs->phys_index;
>> first_page = pfn_to_page(psection << PFN_SECTION_SHIFT);
>>
>> /*
>> @@ -217,18 +231,18 @@
>> ret = online_pages(start_pfn, PAGES_PER_SECTION);
>> break;
>> case MEM_OFFLINE:
>> - mem->state = MEM_GOING_OFFLINE;
>> + mbs->state = MEM_GOING_OFFLINE;
>> start_paddr = page_to_pfn(first_page) << PAGE_SHIFT;
>> ret = remove_memory(start_paddr,
>> PAGES_PER_SECTION << PAGE_SHIFT);
>> if (ret) {
>> - mem->state = old_state;
>> + mbs->state = old_state;
>> break;
>> }
>> break;
>> default:
>> WARN(1, KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
>> - __func__, mem, action, action);
>> + __func__, mbs, action, action);
>> ret = -EINVAL;
>> }
>>
>> @@ -238,19 +252,34 @@
>> static int memory_block_change_state(struct memory_block *mem,
>> unsigned long to_state, unsigned long from_state_req)
>> {
>> + struct memory_block_section *mbs;
>> int ret = 0;
>> +
>> mutex_lock(&mem->state_mutex);
>>
>> - if (mem->state != from_state_req) {
>> - ret = -EINVAL;
>> - goto out;
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + if (mbs->state != from_state_req)
>> + continue;
>> +
>> + ret = memory_block_action(mbs, to_state);
>> + if (ret)
>> + break;
>> + }
>> +
>> + if (ret) {
>> + list_for_each_entry(mbs, &mem->sections, next) {
>> + if (mbs->state == from_state_req)
>> + continue;
>> +
>> + if (memory_block_action(mbs, to_state))
>> + printk(KERN_ERR "Could not re-enable memory "
>> + "section %lx\n", mbs->phys_index);
>> + }
>> }
>
> Please just use a goto here. It's nicer looking, and much more in line
> with what's there already.
Not sure if I follow on where you want the goto. If you mean after the
if (memory_block_action())... I purposely did not have a goto here.
Since this is in the recovery path I wanted to make sure we tried to return
every memory section to the original state.
>
> ...
>> ===================================================================
>> --- linux-2.6.orig/include/linux/memory.h 2010-07-15 08:48:41.000000000 -0500
>> +++ linux-2.6/include/linux/memory.h 2010-07-15 09:54:06.000000000 -0500
>> @@ -19,9 +19,15 @@
>> #include <linux/node.h>
>> #include <linux/compiler.h>
>> #include <linux/mutex.h>
>> +#include <linux/list.h>
>>
>> -struct memory_block {
>> +struct memory_block_section {
>> + unsigned long state;
>> unsigned long phys_index;
>> + struct list_head next;
>> +};
>> +
>> +struct memory_block {
>> unsigned long state;
>> /*
>> * This serializes all state change requests. It isn't
>> @@ -34,6 +40,7 @@
>> void *hw; /* optional pointer to fw/hw data */
>> int (*phys_callback)(struct memory_block *);
>> struct sys_device sysdev;
>> + struct list_head sections;
>> };
>
> It looks like we have state in both the memory_block and
> memory_block_section. That seems a bit confusing to me. This also
> looks like it would permit non-contiguous memory_block_sections in a
> memory_block. Is that what you intended?
The two state fields are a bit confusing, perhaps slightly different
names, block_state and section_state. The reason for two state fileds
is that memory online/offline is done on a memory block and an entire
memory block is considered online or offline. The state field in the
memory_block_section is used to track the state of each of the memory
sections as you work through onlining or offlining the entire block
so that if an error occurs we can return each memory section to its
original state.
You're correct that there is nothing that prevents non-contiguous memory
sections in a memory block. It was not my intention to have this, but
looking at the patches there is nothing to prevent it.
>
> If the memory_block's state was inferred to be the same as each
> memory_block_section, couldn't we just keep a start and end phys_index
> in the memory_block, and get away from having memory_block_sections at
> all?
Oooohhh... I like. Looking at the code it appears this is possible. I'll
try this out and include it in the next version of the patch.
Do you think we need to add an additional file to each memory block directory
to indicate the number of memory sections in the memory block that are actually
present?
-Nathan
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Russell King - ARM Linux @ 2010-07-16 18:07 UTC (permalink / raw)
To: Grant Likely
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Nicolas Pitre, linux-kernel, Linus Torvalds,
Uwe Kleine-König, linuxppc-dev, linux-arm-kernel
In-Reply-To: <AANLkTinyFBTKYaryFANYheTuP-biJUws2x01NRNZPb4M@mail.gmail.com>
On Fri, Jul 16, 2010 at 11:57:55AM -0600, Grant Likely wrote:
> Last missing piece is being able to do "select FOO = n", which Stephen
> is currently working on.
I thought Linus' idea was to use:
KBUILD_KCONFIG=file make allnoconfig
in which case any option which would be presented to the user which hasn't
been selected by 'file' ends up being set to n. That means there's no
need for a special "select FOO=n" construct.
See one of Linus' replies on June 3:
Message-ID: <alpine.LFD.2.00.1006031317410.8175@i5.linux-foundation.org>
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Russell King - ARM Linux @ 2010-07-16 18:30 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Uwe Kleine-König, linux-kernel,
Linus Torvalds, linuxppc-dev, linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1007161406120.10598@xanadu.home>
On Fri, Jul 16, 2010 at 02:19:31PM -0400, Nicolas Pitre wrote:
> For example, if I want CONFIG_MTD_CMDLINE_PARTS=y, the system may be
> smart enough to notice and automatically enable CONFIG_MTD and
> CONFIG_MTD_PARTITIONS without having to carry those in the defconfig.
How do you sort out something like this:
config FOO
bool "Foo"
depends on (A && B) || C
Do you enable A and B, A, B and C or just C?
Bear in mind that A could be 'X86', 'M68K' or any other arch specific
symbol.
I prefer the warning method because it prompts you to investigate what's
changed and sort out the problem by ensuring that the appropriate symbols
are also selected. The automatic selection of dependencies method carries
the risk that it'll do the wrong thing with the above scenario.
^ permalink raw reply
* Re: [RFC PATCH] Kconfig: Enable Kconfig fragments to be used for defconfig
From: Nicolas Pitre @ 2010-07-16 18:31 UTC (permalink / raw)
To: Grant Likely
Cc: Stephen Rothwell, Daniel Walker, linux-kbuild, Tony Lindgren,
Catalin Marinas, Uwe Kleine-König, linux-kernel,
Linus Torvalds, Russell King, linuxppc-dev, linux-arm-kernel
In-Reply-To: <AANLkTilY0rvbp7brGsnyuGhPjFvpVYInNWlAlf5tT4zG@mail.gmail.com>
On Fri, 16 Jul 2010, Grant Likely wrote:
> On Fri, Jul 16, 2010 at 12:19 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > For example, if I want CONFIG_MTD_CMDLINE_PARTS=y, the system may be
> > smart enough to notice and automatically enable CONFIG_MTD and
> > CONFIG_MTD_PARTITIONS without having to carry those in the defconfig.
>
> I fully agree. However, the warnings make the system work now while
> we wait for a full solver to be implemented.
Why can't the tool just _select_ the option it is warning about when a
dependency is not met? That shouldn't require a full solver.
Nicolas
^ permalink raw reply
* Re: [PATCH 1/5] v2 Split the memory_block structure
From: Dave Hansen @ 2010-07-16 18:33 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linux-mm, linux-kernel, KAMEZAWA Hiroyuki, linuxppc-dev
In-Reply-To: <4C40A3BC.3060504@austin.ibm.com>
On Fri, 2010-07-16 at 13:23 -0500, Nathan Fontenot wrote:
> > If the memory_block's state was inferred to be the same as each
> > memory_block_section, couldn't we just keep a start and end phys_index
> > in the memory_block, and get away from having memory_block_sections at
> > all?
>
> Oooohhh... I like. Looking at the code it appears this is possible. I'll
> try this out and include it in the next version of the patch.
>
> Do you think we need to add an additional file to each memory block directory
> to indicate the number of memory sections in the memory block that are actually
> present?
I think it's easiest to just say that each 'memory_block' can only hold
contiguous 'memory_block_sections', and we give either the start/end or
start/length pairs. It gets a lot more complicated if we have to deal
with lots of holes.
I can just see the hardware designers reading this thread, with their
Dr. Evil laughs trying to come up with a reason to give us a couple of
terabytes of RAM with only every-other 16MB area populated. :)
-- Dave
^ 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