linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Simek <monstr@monstr.eu>
To: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Yinghai Lu <yinghai@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Sam Ravnborg <sam@ravnborg.org>,
	linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: memblock and bootmem problems if start + size = 4GB
Date: Thu, 29 Dec 2011 17:46:18 +0100	[thread overview]
Message-ID: <4EFC995A.5090904@monstr.eu> (raw)
In-Reply-To: <20111229155836.GB3516@google.com>

Tejun Heo wrote:
> Hello,
> 
> On Tue, Dec 20, 2011 at 10:19:18AM +0100, Michal Simek wrote:
>>> Yeah, that's an inherent problem in using [) ranges but I think
>>> chopping off the last page probably is simpler and more robust
>>> solution.  Currently, memblock_add_region() would simply ignore if
>>> address range overflows but making it just ignore the last page is
>>> several lines of addition.  Wouldn't that be effective enough while
>>> staying very simple?
>> The main problem is with PFN_DOWN/UP macros and it is in __init section.
>> The result will be definitely u32 type (for 32bit archs) anyway and seems to me
>> better solution than ignoring the last page.
> 
> Other than being able to use one more 4k page, is there any other
> benefit? Maybe others had different experiences but in my exprience
> trying to extend range coverages - be it stack top/end pointers,
> address ranges or whatnot - using [] ranges or special flag usually
> ended up adding complexity while adding almost nothing tangible.

First of all I don't like to use your term "extend range coverages".
We don't want to extend any ranges - we just wanted to place memory to the end
of address space and be able to work with. It is limitation which should be fixed somehow.
And I would expect that PFN_XX(base + size) will be in u32 range.

Probably the best solution will be to use PFN macro in one place and do not covert
addresses in common code.

+ change parameters in bootmem code because some arch do
free_bootmem_node(..., PFN_PHYS(), ...)
and
reserve_bootmem_node(..., PFN_PHYS(), ...)

and then in that functions(free/reseve_bootmem_code) are used PFN_DOWN/PFN_UP macros.
If alignment is handled by architecture code (which I believe is) then should be possible to change parameters.

For example:
void __init free_bootmem_node(pg_data_t *pgdat, unsigned long start_pfn,
			      unsigned long end_pfn)

int __init reserve_bootmem_node(pg_data_t *pgdat, unsigned long start_pfn,
				 unsigned long end_pfn, int flags)

Is there any reason to use use physical addresses instead of pfns in bootmem code?

 >  On
> extreme cases, people even carry separate valid flag to use %NULL as
> valid address, which is pretty silly, IMHO.  So, unless there's some
> benefit that I'm missing, I still think it's an overkill.  It's more
> complex and difficult to test and verify.  Why bother for a single
> page?

Where do you think this page should be placed? In common code or in architecture memory
code where one page from the top of 4G should be subtract?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-12-29 16:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-19 13:58 memblock and bootmem problems if start + size = 4GB Michal Simek
2011-12-19 16:28 ` Tejun Heo
2011-12-20  9:19   ` Michal Simek
2011-12-29 13:44     ` Michal Simek
2011-12-29 15:58     ` Tejun Heo
2011-12-29 16:46       ` Michal Simek [this message]
2011-12-29 17:07         ` Tejun Heo
2011-12-30  7:58           ` Michal Simek
2011-12-30 17:45             ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EFC995A.5090904@monstr.eu \
    --to=monstr@monstr.eu \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=sam@ravnborg.org \
    --cc=tj@kernel.org \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).