Re: lmb_remove_region - bad decrementing...

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]

On Mon, 2007-03-19 at 13:00 -0700, Siva Prasad wrote:
> Hi,
> 
> This is based on 2.6.19 kernel.
> 
> In the file mm/lmb.c, we have function lmb_remove_region. In this
> function we are decrementing the region count even if the condition
> fails and we don't necessarily remove the region.
> 
> For example ...
> 
> static void __init lmb_remove_region(struct lmb_region *rgn, unsigned
> long r)
> {
>         unsigned long i;
> 
>         for (i = r; i < rgn->cnt - 1; i++) {
>                 rgn->region[i].base = rgn->region[i + 1].base;
>                 rgn->region[i].size = rgn->region[i + 1].size;
>         }
>         rgn->cnt--;
> }
> 
> Shound't the region count be decremented only if r > rgn->cnt -1. When
> we initially setup lmb, cnt will be 1, and as part of other functions,
> this gets called with out even increasing this number. So, region does
> not get removed, but cnt does get decremented by 1.
> 
> At least a quick fix like this "if (rgn->cnt > 1) rgn->cnt--;" should
> serve for now.
> 
> Same thing happens at routine lmb_enfore_memory_limit. Actually
> lmb_remove_region gets called from here and the problem starts. So, "i"
> should not be decremented here, if the initial condition fails.
> 
> What do you guys think?

I agree that's a bug in lmb_remove_region(), but I can't see how it's
ever triggered.

I can't see how lmb_enforce_memory_limit() would hit the bug either. Can
you give more detail?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]