lmb_remove_region - bad decrementing...

lmb_remove_region - bad decrementing...

[Siva Prasad]

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 =3D r; i < rgn->cnt - 1; i++) {
                rgn->region[i].base =3D rgn->region[i + 1].base;
                rgn->region[i].size =3D 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?

Thanks
Siva

Re: lmb_remove_region - bad decrementing...

[Michael Ellerman]

[-- 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 --]