2013-03-25 04:10:36

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 0/6] ARM: use NO_BOOTMEM on default configuration

Currently, ARM use traditional 'bootmem' allocator. It use a bitmap for
managing memory space, so initialize a bitmap at first step. It is
a needless overhead if we use 'nobootmem'. 'nobootmem' use a memblock
allocator internally, so there is no additional initializing overhead.
In addition, if we use 'nobootmem', we can save small amount of memory,
because 'nobootmem' manage memory space as byte unit. However,
'bootmem' manage it as page unit, so some space is wasted,
although it is very small. On my system, 20 KB memories can be saved. :)
Using 'nobootmem' have another advantage. Before initializing 'bootmem'
allocator, we use memblock allocator. If we use memblock allocator
after initializing 'bootmem' by mistake, there is possible problem.
Patch '1/6' is good example of this case. 'nobootmem' use memblock
allocator internally, so these risk will be disappeared.

There is one stopper to enable NO_BOOTMEM, it is max_low_pfn.
nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
So we need proper value to max_low_pfn.
But, there is some difficulty related to max_low_pfn. max_low_pfn is used
for two meanings in various architectures. One is for number of pages
in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
as number of pages in lowmem. You can get more information in below link.
http://lwn.net/Articles/543408/
http://lwn.net/Articles/543424/

As I investigated, architectures which use max_low_pfn as maximum pfn are
more than others, so IMHO, to change meaning of max_low_pfn to maximum pfn
is preferable solution to me. This patchset change max_low_pfn as maximum
lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
to this criteria.

AFAIK, there is no real user for max_low_pfn except block/blk-setting.c
and blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
so this patch may not harm anything. But, I'm not expert about this,
so please let me know what I am missing.

I did some working test on my android device and it worked. :)
Feel free to give me some opinion about this patset.
This patchset is based on v3.9-rc4.

Thanks.

Joonsoo Kim (6):
ARM, TCM: initialize TCM in paging_init(), instead of setup_arch()
ARM, crashkernel: use ___alloc_bootmem_node_nopanic() for reserving
memory
ARM, crashkernel: correct total_mem size in reserve_crashkernel()
ARM, mm: don't do arm_bootmem_init() if CONFIG_NO_BOOTMEM
ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem
ARM, mm: enable NO_BOOTMEM for default ARM build

arch/arm/Kconfig | 1 +
arch/arm/kernel/setup.c | 22 ++++++++--------------
arch/arm/kernel/tcm.c | 1 -
arch/arm/kernel/tcm.h | 17 -----------------
arch/arm/mm/init.c | 19 ++++++++++++-------
arch/arm/mm/mmu.c | 2 ++
arch/arm/mm/tcm.h | 17 +++++++++++++++++
7 files changed, 40 insertions(+), 39 deletions(-)
delete mode 100644 arch/arm/kernel/tcm.h
create mode 100644 arch/arm/mm/tcm.h

--
1.7.9.5


2013-03-25 04:10:45

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 4/6] ARM, mm: don't do arm_bootmem_init() if CONFIG_NO_BOOTMEM

arm_bootmem_init() initialize a bitmap for bootmem and
it is not needed for CONFIG_NO_BOOTMEM.
So skip it when CONFIG_NO_BOOTMEM.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index ad722f1..049414a 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -149,6 +149,12 @@ static void __init find_limits(unsigned long *min, unsigned long *max_low,
*max_high = bank_pfn_end(&mi->bank[mi->nr_banks - 1]);
}

+#ifdef CONFIG_NO_BOOTMEM
+static inline void __init arm_bootmem_init(unsigned long start_pfn,
+ unsigned long end_pfn)
+{
+}
+#else
static void __init arm_bootmem_init(unsigned long start_pfn,
unsigned long end_pfn)
{
@@ -169,7 +175,6 @@ static void __init arm_bootmem_init(unsigned long start_pfn,
* Initialise the bootmem allocator, handing the
* memory banks over to bootmem.
*/
- node_set_online(0);
pgdat = NODE_DATA(0);
init_bootmem_node(pgdat, __phys_to_pfn(bitmap), start_pfn, end_pfn);

@@ -200,6 +205,7 @@ static void __init arm_bootmem_init(unsigned long start_pfn,
(end - start) << PAGE_SHIFT, BOOTMEM_DEFAULT);
}
}
+#endif

#ifdef CONFIG_ZONE_DMA

@@ -392,6 +398,7 @@ void __init bootmem_init(void)

find_limits(&min, &max_low, &max_high);

+ node_set_online(0);
arm_bootmem_init(min, max_low);

/*
--
1.7.9.5

2013-03-25 04:11:12

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 1/6] ARM, TCM: initialize TCM in paging_init(), instead of setup_arch()

tcm_init() call iotable_init() and it use early_alloc variants which
do memblock allocation. Directly using memblock allocation after
initializing bootmem should not permitted, because bootmem can't know
where are additinally reserved.
So move tcm_init() to a safe place before initalizing bootmem.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 3f6cbb2..b3990a3 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -56,7 +56,6 @@
#include <asm/virt.h>

#include "atags.h"
-#include "tcm.h"


#if defined(CONFIG_FPE_NWFPE) || defined(CONFIG_FPE_FASTFPE)
@@ -778,8 +777,6 @@ void __init setup_arch(char **cmdline_p)

reserve_crashkernel();

- tcm_init();
-
#ifdef CONFIG_MULTI_IRQ_HANDLER
handle_arch_irq = mdesc->handle_irq;
#endif
diff --git a/arch/arm/kernel/tcm.c b/arch/arm/kernel/tcm.c
index 30ae6bb..f50f19e 100644
--- a/arch/arm/kernel/tcm.c
+++ b/arch/arm/kernel/tcm.c
@@ -17,7 +17,6 @@
#include <asm/mach/map.h>
#include <asm/memory.h>
#include <asm/system_info.h>
-#include "tcm.h"

static struct gen_pool *tcm_pool;
static bool dtcm_present;
diff --git a/arch/arm/kernel/tcm.h b/arch/arm/kernel/tcm.h
deleted file mode 100644
index 8015ad4..0000000
--- a/arch/arm/kernel/tcm.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2008-2009 ST-Ericsson AB
- * License terms: GNU General Public License (GPL) version 2
- * TCM memory handling for ARM systems
- *
- * Author: Linus Walleij <[email protected]>
- * Author: Rickard Andersson <[email protected]>
- */
-
-#ifdef CONFIG_HAVE_TCM
-void __init tcm_init(void);
-#else
-/* No TCM support, just blank inlines to be optimized out */
-inline void tcm_init(void)
-{
-}
-#endif
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e95a996..a7f127d 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -34,6 +34,7 @@
#include <asm/mach/pci.h>

#include "mm.h"
+#include "tcm.h"

/*
* empty_zero_page is a special page that is used for
@@ -1256,6 +1257,7 @@ void __init paging_init(struct machine_desc *mdesc)
dma_contiguous_remap();
devicemaps_init(mdesc);
kmap_init();
+ tcm_init();

top_pmd = pmd_off_k(0xffff0000);

diff --git a/arch/arm/mm/tcm.h b/arch/arm/mm/tcm.h
new file mode 100644
index 0000000..8015ad4
--- /dev/null
+++ b/arch/arm/mm/tcm.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2008-2009 ST-Ericsson AB
+ * License terms: GNU General Public License (GPL) version 2
+ * TCM memory handling for ARM systems
+ *
+ * Author: Linus Walleij <[email protected]>
+ * Author: Rickard Andersson <[email protected]>
+ */
+
+#ifdef CONFIG_HAVE_TCM
+void __init tcm_init(void);
+#else
+/* No TCM support, just blank inlines to be optimized out */
+inline void tcm_init(void)
+{
+}
+#endif
--
1.7.9.5

2013-03-25 04:11:10

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 2/6] ARM, crashkernel: use ___alloc_bootmem_node_nopanic() for reserving memory

For crashkernel, specific address should be reserved.
It can be achived by reserve_bootmem(), but this function is
only for bootmem.

Now, we try to enable nobootmem, therfore change it more general function,
___alloc_bootmem_node_nopanic(). It can be use for both,
bootmem and nobootmem.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index b3990a3..99ffe87 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -674,15 +674,20 @@ static void __init reserve_crashkernel(void)
{
unsigned long long crash_size, crash_base;
unsigned long long total_mem;
+ unsigned long limit = 0;
int ret;

total_mem = get_total_mem();
ret = parse_crashkernel(boot_command_line, total_mem,
&crash_size, &crash_base);
- if (ret)
+ if (ret != 0 || crash_size == 0)
return;

- ret = reserve_bootmem(crash_base, crash_size, BOOTMEM_EXCLUSIVE);
+ if (crash_base != 0)
+ limit = crash_base + crash_size;
+
+ ret = ___alloc_bootmem_node_nopanic(NODE_DATA(0), crash_size,
+ PAGE_ALIGN, crash_base, limit);
if (ret < 0) {
printk(KERN_WARNING "crashkernel reservation failed - "
"memory is in use (0x%lx)\n", (unsigned long)crash_base);
--
1.7.9.5

2013-03-25 04:11:08

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 6/6] ARM, mm: enable NO_BOOTMEM for default ARM build

If we use NO_BOOTMEM, we don't need to initialize a bitmap and
we don't need to do bitmap operation, so we can boot slightly faster.

Additionaly advantage of enabling NO_BOOTMEM is saving more memory.
bootmem allocator manage memories as page unit, so if we request
4 bytes area to bootmem, it actually give us PAGE_SIZE area.
nobootmem manage memory as byte unit, so there is no waste.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 13b7394..8b73417 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -58,6 +58,7 @@ config ARM
select CLONE_BACKWARDS
select OLD_SIGSUSPEND3
select OLD_SIGACTION
+ select NO_BOOTMEM
help
The ARM series is a line of low-power-consumption RISC chip designs
licensed by ARM Ltd and targeted at embedded applications and
--
1.7.9.5

2013-03-25 04:11:05

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
So we need proper value to max_low_pfn.

But, there is some difficulty related to max_low_pfn. max_low_pfn is used
for two meanings in various architectures. One is for number of pages
in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
as number of pages in lowmem. You can get more information in below link.
http://lwn.net/Articles/543408/
http://lwn.net/Articles/543424/

As I investigated, architectures which use max_low_pfn as maximum pfn are
more than others, so to change meaning of max_low_pfn to maximum pfn
is preferable solution to me. This patch change max_low_pfn as maximum
lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
to this criteria.

There is no real user for max_low_pfn except block/blk-setting.c and
blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
so this patch may not harm anything.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 049414a..873f4ca 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -423,12 +423,10 @@ void __init bootmem_init(void)
* This doesn't seem to be used by the Linux memory manager any
* more, but is used by ll_rw_block. If we can get rid of it, we
* also get rid of some of the stuff above as well.
- *
- * Note: max_low_pfn and max_pfn reflect the number of _pages_ in
- * the system, not the maximum PFN.
*/
- max_low_pfn = max_low - PHYS_PFN_OFFSET;
- max_pfn = max_high - PHYS_PFN_OFFSET;
+ min_low_pfn = min;
+ max_low_pfn = max_low;
+ max_pfn = max_high;
}

static inline int free_area(unsigned long pfn, unsigned long end, char *s)
@@ -544,7 +542,7 @@ static void __init free_unused_memmap(struct meminfo *mi)
static void __init free_highpages(void)
{
#ifdef CONFIG_HIGHMEM
- unsigned long max_low = max_low_pfn + PHYS_PFN_OFFSET;
+ unsigned long max_low = max_low_pfn;
struct memblock_region *mem, *res;

/* set highmem page free */
--
1.7.9.5

2013-03-25 04:11:03

by Joonsoo Kim

[permalink] [raw]
Subject: [RFC PATCH 3/6] ARM, crashkernel: correct total_mem size in reserve_crashkernel()

There is some platforms which have highmem, so this equation
doesn't represent total_mem size properly.
In addition, max_low_pfn's meaning is different in other architecture and
it is scheduled to be changed, so remove related code to max_low_pfn.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 99ffe87..1149988 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -655,14 +655,6 @@ static int __init init_machine_late(void)
late_initcall(init_machine_late);

#ifdef CONFIG_KEXEC
-static inline unsigned long long get_total_mem(void)
-{
- unsigned long total;
-
- total = max_low_pfn - min_low_pfn;
- return total << PAGE_SHIFT;
-}
-
/**
* reserve_crashkernel() - reserves memory are for crash kernel
*
@@ -677,7 +669,7 @@ static void __init reserve_crashkernel(void)
unsigned long limit = 0;
int ret;

- total_mem = get_total_mem();
+ total_mem = memblock_phys_mem_size();
ret = parse_crashkernel(boot_command_line, total_mem,
&crash_size, &crash_base);
if (ret != 0 || crash_size == 0)
--
1.7.9.5

2013-03-25 09:48:32

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

On Mon, Mar 25, 2013 at 01:11:13PM +0900, Joonsoo Kim wrote:
> nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
> So we need proper value to max_low_pfn.
>
> But, there is some difficulty related to max_low_pfn. max_low_pfn is used
> for two meanings in various architectures. One is for number of pages
> in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
> as number of pages in lowmem. You can get more information in below link.
> http://lwn.net/Articles/543408/
> http://lwn.net/Articles/543424/
>
> As I investigated, architectures which use max_low_pfn as maximum pfn are
> more than others, so to change meaning of max_low_pfn to maximum pfn
> is preferable solution to me. This patch change max_low_pfn as maximum
> lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
> to this criteria.
>
> There is no real user for max_low_pfn except block/blk-setting.c and
> blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
> so this patch may not harm anything.

This will need some very rigorous testing before it can go into mainline.

2013-03-25 21:18:16

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFC PATCH 1/6] ARM, TCM: initialize TCM in paging_init(), instead of setup_arch()

On Mon, Mar 25, 2013 at 5:11 AM, Joonsoo Kim <[email protected]> wrote:

> tcm_init() call iotable_init() and it use early_alloc variants which
> do memblock allocation. Directly using memblock allocation after
> initializing bootmem should not permitted, because bootmem can't know
> where are additinally reserved.
> So move tcm_init() to a safe place before initalizing bootmem.
>
> Signed-off-by: Joonsoo Kim <[email protected]>

Tested-by: Linus Walleij <[email protected]>
(On the U300)

You should put this into Russell's patch tracker for his judgement.

Yours,
Linus Walleij

2013-04-01 08:28:40

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

Hello, Russell.

On Mon, Mar 25, 2013 at 09:48:16AM +0000, Russell King - ARM Linux wrote:
> On Mon, Mar 25, 2013 at 01:11:13PM +0900, Joonsoo Kim wrote:
> > nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
> > So we need proper value to max_low_pfn.
> >
> > But, there is some difficulty related to max_low_pfn. max_low_pfn is used
> > for two meanings in various architectures. One is for number of pages
> > in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
> > as number of pages in lowmem. You can get more information in below link.
> > http://lwn.net/Articles/543408/
> > http://lwn.net/Articles/543424/
> >
> > As I investigated, architectures which use max_low_pfn as maximum pfn are
> > more than others, so to change meaning of max_low_pfn to maximum pfn
> > is preferable solution to me. This patch change max_low_pfn as maximum
> > lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
> > to this criteria.
> >
> > There is no real user for max_low_pfn except block/blk-setting.c and
> > blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
> > so this patch may not harm anything.
>
> This will need some very rigorous testing before it can go into mainline.

There is no attention from others to this patchset. :)
Do you have any plan to test this?
What I can do best is just to run this patchset on my platform.

Would you guide me how to go into mainline for this patchset.

Thanks.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-04-22 08:21:00

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [RFC PATCH 0/6] ARM: use NO_BOOTMEM on default configuration

On Mon, Mar 25, 2013 at 01:11:08PM +0900, Joonsoo Kim wrote:
> Currently, ARM use traditional 'bootmem' allocator. It use a bitmap for
> managing memory space, so initialize a bitmap at first step. It is
> a needless overhead if we use 'nobootmem'. 'nobootmem' use a memblock
> allocator internally, so there is no additional initializing overhead.
> In addition, if we use 'nobootmem', we can save small amount of memory,
> because 'nobootmem' manage memory space as byte unit. However,
> 'bootmem' manage it as page unit, so some space is wasted,
> although it is very small. On my system, 20 KB memories can be saved. :)
> Using 'nobootmem' have another advantage. Before initializing 'bootmem'
> allocator, we use memblock allocator. If we use memblock allocator
> after initializing 'bootmem' by mistake, there is possible problem.
> Patch '1/6' is good example of this case. 'nobootmem' use memblock
> allocator internally, so these risk will be disappeared.
>
> There is one stopper to enable NO_BOOTMEM, it is max_low_pfn.
> nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
> So we need proper value to max_low_pfn.
> But, there is some difficulty related to max_low_pfn. max_low_pfn is used
> for two meanings in various architectures. One is for number of pages
> in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
> as number of pages in lowmem. You can get more information in below link.
> http://lwn.net/Articles/543408/
> http://lwn.net/Articles/543424/
>
> As I investigated, architectures which use max_low_pfn as maximum pfn are
> more than others, so IMHO, to change meaning of max_low_pfn to maximum pfn
> is preferable solution to me. This patchset change max_low_pfn as maximum
> lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
> to this criteria.
>
> AFAIK, there is no real user for max_low_pfn except block/blk-setting.c
> and blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
> so this patch may not harm anything. But, I'm not expert about this,
> so please let me know what I am missing.
>
> I did some working test on my android device and it worked. :)
> Feel free to give me some opinion about this patset.
> This patchset is based on v3.9-rc4.
>
> Thanks.
>
> Joonsoo Kim (6):
> ARM, TCM: initialize TCM in paging_init(), instead of setup_arch()
> ARM, crashkernel: use ___alloc_bootmem_node_nopanic() for reserving
> memory
> ARM, crashkernel: correct total_mem size in reserve_crashkernel()
> ARM, mm: don't do arm_bootmem_init() if CONFIG_NO_BOOTMEM
> ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem
> ARM, mm: enable NO_BOOTMEM for default ARM build
>
> arch/arm/Kconfig | 1 +
> arch/arm/kernel/setup.c | 22 ++++++++--------------
> arch/arm/kernel/tcm.c | 1 -
> arch/arm/kernel/tcm.h | 17 -----------------
> arch/arm/mm/init.c | 19 ++++++++++++-------
> arch/arm/mm/mmu.c | 2 ++
> arch/arm/mm/tcm.h | 17 +++++++++++++++++
> 7 files changed, 40 insertions(+), 39 deletions(-)
> delete mode 100644 arch/arm/kernel/tcm.h
> create mode 100644 arch/arm/mm/tcm.h

Hello, guys.

Could you review and comment this patchset?
[1/6] is already merged, but others wait for someone's review :)

Thanks.

>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-07-01 14:15:19

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

Joonsoo,

On Monday 25 March 2013 12:11 AM, Joonsoo Kim wrote:
> nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
> So we need proper value to max_low_pfn.
>
> But, there is some difficulty related to max_low_pfn. max_low_pfn is used
> for two meanings in various architectures. One is for number of pages
> in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
> as number of pages in lowmem. You can get more information in below link.
> http://lwn.net/Articles/543408/
> http://lwn.net/Articles/543424/
>
> As I investigated, architectures which use max_low_pfn as maximum pfn are
> more than others, so to change meaning of max_low_pfn to maximum pfn
> is preferable solution to me. This patch change max_low_pfn as maximum
> lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
> to this criteria.
>
> There is no real user for max_low_pfn except block/blk-setting.c and
> blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
> so this patch may not harm anything.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
>
I have been also carrying similar patch as yours in an attempt
to make LPAE kernel work on ARM. Your patch carries better
description, so will your version and include in my series
which I plan to post on the list after some more testing.
Will copy you. The changes are very similar to your series.

Regards,
Santosh

2013-07-01 17:10:03

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

On Mon, Jul 01, 2013 at 10:14:45AM -0400, Santosh Shilimkar wrote:
> I have been also carrying similar patch as yours in an attempt
> to make LPAE kernel work on ARM. Your patch carries better
> description, so will your version and include in my series
> which I plan to post on the list after some more testing.
> Will copy you. The changes are very similar to your series.

And will you try to investigate and/or address my concerns with this
change?

Consider this code in the block layer:

static int __init blk_settings_init(void)
{
blk_max_low_pfn = max_low_pfn - 1;
blk_max_pfn = max_pfn - 1;
return 0;
}

void blk_queue_bounce_limit(struct request_queue *q, u64 dma_mask)
{
unsigned long b_pfn = dma_mask >> PAGE_SHIFT;
...
if (b_pfn < blk_max_low_pfn)
dma = 1;
q->limits.bounce_pfn = b_pfn;
if (dma) {
init_emergency_isa_pool();
q->bounce_gfp = GFP_NOIO | GFP_DMA;
q->limits.bounce_pfn = b_pfn;
}
}

and this in SCSI:

u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
{
struct device *host_dev;
u64 bounce_limit = 0xffffffff;
...
host_dev = scsi_get_device(shost);
if (host_dev && host_dev->dma_mask)
bounce_limit = *host_dev->dma_mask;

return bounce_limit;
}

struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
request_fn_proc *request_fn)
{
...
blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));

Now, what happens when you have a device which can only address the first
64MB of system memory, which is at 3GB(physical), but you have 1GB of
system memory available both before changing max_low_pfn, and after
changing max_low_pfn.

Bear in mind that you will find that virtually all places in the kernel
set the device DMA mask to be "DMA_BIT_MASK(number_of_bits_driven)" and
not offset by the base of physical memory. In the above case, consider
what happens with a 24 bit DMA mask.

So... if we make this change, I would much prefer to also see a number
of other patches preceding this one:

(a) blk_queue_bounce_limit()'s parameter renamed from "dma_mask" to
"max_addr" or indeed just taking b_pfn directly.
(b) a helper: dma_max_pfn(dev) which converts a DMA bitmask of bits
to a b_pfn number which is just "dev->dma_mask >> PAGE_SHIFT" in the
generic case, but ARM can override this later.
(c) a patch changing max_low_pfn/max_pfn to include the physical offset
of memory, and providing an ARM version of this which adds in the
appropriate offset.

Here's an example dma_max_pfn() helper for the generic case:

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 48ef6f5..a083724 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -153,6 +153,13 @@ static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
return -EIO;
}

+#ifndef dma_max_pfn
+static inline unsigned long dma_max_pfn(struct device *dev)
+{
+ return dev->dma_mask ? (*dev->dma_mask >> PAGE_SHIFT) : 0;
+}
+#endif
+
static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t flag)
{

Arches can then override this with:

static inline unsigned long dma_max_pfn(struct device *dev)
{
...
}
#define dma_max_pfn dma_max_pfn

2013-07-02 01:28:19

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

On Mon, Jul 01, 2013 at 10:14:45AM -0400, Santosh Shilimkar wrote:
> Joonsoo,
>
> On Monday 25 March 2013 12:11 AM, Joonsoo Kim wrote:
> > nobootmem use max_low_pfn for computing boundary in free_all_bootmem()
> > So we need proper value to max_low_pfn.
> >
> > But, there is some difficulty related to max_low_pfn. max_low_pfn is used
> > for two meanings in various architectures. One is for number of pages
> > in lowmem and the other is for maximum lowmem pfn. Now, in ARM, it is used
> > as number of pages in lowmem. You can get more information in below link.
> > http://lwn.net/Articles/543408/
> > http://lwn.net/Articles/543424/
> >
> > As I investigated, architectures which use max_low_pfn as maximum pfn are
> > more than others, so to change meaning of max_low_pfn to maximum pfn
> > is preferable solution to me. This patch change max_low_pfn as maximum
> > lowmem pfn in ARM. In addition, min_low_pfn, max_pfn is assigned according
> > to this criteria.
> >
> > There is no real user for max_low_pfn except block/blk-setting.c and
> > blk-setting.c assume that max_low_pfn is maximum lowmem pfn,
> > so this patch may not harm anything.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
> >
> I have been also carrying similar patch as yours in an attempt
> to make LPAE kernel work on ARM. Your patch carries better
> description, so will your version and include in my series
> which I plan to post on the list after some more testing.
> Will copy you. The changes are very similar to your series.

Okay!

Thanks.
>
> Regards,
> Santosh
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-07-02 17:39:01

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC PATCH 5/6] ARM, mm: change meaning of max_low_pfn to maximum pfn for nobootmem

On Monday 01 July 2013 01:09 PM, Russell King - ARM Linux wrote:
> On Mon, Jul 01, 2013 at 10:14:45AM -0400, Santosh Shilimkar wrote:
>> I have been also carrying similar patch as yours in an attempt
>> to make LPAE kernel work on ARM. Your patch carries better
>> description, so will your version and include in my series
>> which I plan to post on the list after some more testing.
>> Will copy you. The changes are very similar to your series.
>
> And will you try to investigate and/or address my concerns with this
> change?
>
Sure. Will try to look and come back to you.

Regards,
Santosh