On Mon, Aug 17, 2009 at 06:47:18PM +0100, Russell King - ARM Linux wrote:
> On Mon, Aug 17, 2009 at 11:03:44PM +0530, Rabin Vincent wrote:
> > On Sat, Aug 15, 2009 at 07:17:11PM +0100, Russell King - ARM Linux wrote:
> > > +#ifndef CONFIG_SPARSEMEM
> > > +int pfn_valid(unsigned long pfn)
> > > +{
> > > + struct meminfo *mi = &meminfo;
> > > + unsigned int mid, left = 0, right = mi->nr_banks;
> > > +
> > > + while ((mid = (right - left) / 2) > 0) {
> > > + struct membank *bank = &mi->bank[mid];
> > > +
> > > + if (pfn < bank_pfn_start(bank))
> > > + right = mid;
> > > + else if (pfn >= bank_pfn_end(bank))
> > > + left = mid + 1;
> > > + else
> > > + return 1;
> > > + }
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(pfn_valid);
> > > +#endif
> >
> > The above function will ignore the first bank. I don't think that was
> > intended.
>
> Why do you say that?
>
> Let's start with 4 membank entries - 0 1 2 3.
>
> 0 covers 0-10, 1 covers 20-30, 2 covers 40-50 and 3 covers 60-70.
>
> Let's run through the steps looking for 5:
>
> Initally, left = 0, right = 4, so mid=2. 5 < 40? Yes, so right becomes 2.
>
> left = 0, right = 2, so mid=1. 5 < 20? Yes, so right becomes 1.
>
> left = 0, right = 1, so mid=0. 5 < 0? No. 5 >= 10? No. so return 1.
>
> This doesn't look to me like it ignores the first bank. I think you're
> wrong. Please provide me with your proof ASAP because right now it's
> pending for Linus.
When mid == 0, the loop exits because the condition you have is :
while ((mid = (right - left) / 2) > 0) {
^^^^^^
Thus bank[0] will never be checked.
Rabin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
