lkml.org 
[lkml]   [2008]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: make reserve_bootmem can crossed the nodes
On Thu, Mar 13, 2008 at 7:50 PM, Johannes Weiner <hannes@saeurebad.de> wrote:
>
> Hi,
>
> "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
>
> > On Thu, Mar 13, 2008 at 5:13 PM, Johannes Weiner <hannes@saeurebad.de> wrote:
> >> Hi,
> >>
> >> "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> >>
> >
> >> > static void __init free_bootmem_core(bootmem_data_t *bdata, unsigned long addr,
> >> > @@ -407,6 +432,11 @@ unsigned long __init init_bootmem_node(p
> >> > void __init reserve_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
> >> > unsigned long size, int flags)
> >> > {
> >> > + int ret;
> >> > +
> >> > + ret = can_reserve_bootmem_core(pgdat->bdata, physaddr, size, flags);
> >> > + if (ret < 0)
> >> > + return;
> >> > reserve_bootmem_core(pgdat->bdata, physaddr, size, flags);
> >>
> >> I don't get it. Sorry. What is the purpose of
> >> can_reserve_bootmem_core()? It does exactly what reserve_bootmem_core
> >> does besides actually setting the bits. All the pre-checking you wanted
> >> to have out of the way is repeated again in reserve_bootmem_core()
> >> (well, almost all).
> >
> > can_reserve_bootmem_core is check if there is some pages is reserved
> > already with
> >> > + if (flags & BOOTMEM_EXCLUSIVE)
> >> > + return -EBUSY;
> >
> > so it will avoid the restoring later.
>
> Yes, I understood that. But you skipped the lower part of my email:
>
> Your current state now is _not_ that you have one function that
> prechecks the range and another function that reserves it! You have
> _two_ functions checking the range and the second reserving it.

Yes

>
> Why double-check most of the things? If you want to have a pre-check
> function, _move_ all the pre-checks into another function, not
> copy-paste them.

for cross the nodes

>
> And is the condition of trying to reserve a range twice, the second time
> exclusively, so common that it is worth iterating twice over the nodes
> (once for checking, once for reserving) instead of just unwinding the
> reservation if it fails in between?
>
> On something else: is there a bug when a memory range is reserved with
> BOOTMEM_EXCLUSIVE and then again without this flag? The second call
> does not return an error then.

Yes. only provide one direction protection.
so always make sure crash_kernel reserve_bootmem as the last. that is doable.

YH


\
 
 \ /
  Last update: 2008-03-14 04:11    [W:0.047 / U:42.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site