lkml.org 
[lkml]   [2018]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [mm PATCH v5 3/7] mm: Implement new zone specific memblock iterator
    > > > +		unsigned long epfn = PFN_DOWN(epa);
    > > > + unsigned long spfn = PFN_UP(spa);
    > > > +
    > > > + /*
    > > > + * Verify the end is at least past the start of the zone and
    > > > + * that we have at least one PFN to initialize.
    > > > + */
    > > > + if (zone->zone_start_pfn < epfn && spfn < epfn) {
    > > > + /* if we went too far just stop searching */
    > > > + if (zone_end_pfn(zone) <= spfn)
    > > > + break;
    > >
    > > Set *idx = U64_MAX here, then break. This way after we are outside this
    > > while loop idx is always equals to U64_MAX.
    >
    > Actually I think what you are asking for is the logic that is outside
    > of the while loop we are breaking out of. So if you check at the end of
    > the function there is the bit of code with the comment "signal end of
    > iteration" where I end up setting *idx to ULLONG_MAX, *out_spfn to
    > ULONG_MAX, and *out_epfn to 0.
    >
    > The general idea I had with the function is that you could use either
    > the index or spfn < epfn checks to determine if you keep going or not.

    Yes, I meant to remove that *idx = U64_MAX after the loop, it is
    confusing to have a loop:

    while (*idx != U64_MAX) {
    ...
    }

    *idx = U64_MAX;


    So, it is better to set idx to U643_MAX inside the loop before the
    break.

    >
    > >
    > > > +
    > > > + if (out_spfn)
    > > > + *out_spfn = max(zone->zone_start_pfn, spfn);
    > > > + if (out_epfn)
    > > > + *out_epfn = min(zone_end_pfn(zone), epfn);
    > >
    > > Don't we need to verify after adjustment that out_spfn != out_epfn, so
    > > there is at least one PFN to initialize?
    >
    > We have a few checks that I believe prevent that. Before we get to this
    > point we have verified the following:
    > zone->zone_start < epfn
    > spfn < epfn
    >
    > The other check that should be helping to prevent that is the break
    > statement above that is forcing us to exit if spfn is somehow already
    > past the end of the zone, that essentially maps out:
    > spfn < zone_end_pfn(zone)
    >
    > So the only check we don't have is:
    > zone->zone_start < zone_end_pfn(zone)
    >
    > If I am not mistaken that is supposed to be a given is it not? I would
    > assume we don't have any zones that are completely empty or inverted
    > that would be called here do we?


    if (zone_end_pfn(zone) <= spfn) won't break

    Equal sign in <= here takes care of the case I was thinking. Yes, logic looks good.

    Thank you
    Pasha

    \
     
     \ /
      Last update: 2018-11-10 01:11    [W:2.299 / U:0.652 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site