On 22/07/2019 17:18, Rob Herring wrote: > On Mon, Jul 22, 2019 at 7:25 AM Robin Murphy <robin.murphy@xxxxxxx> wrote: >> >> On 22/07/2019 13:19, Steven Price wrote: >> [...] >>> Indeed, that case was just occurring to me too! How about: >>> >>> u64 next_seg = ALIGN(*start, PFN_4G); >>> >>> if (next_seg - *start <= PFN_16M) >>> *start = next_seg + 1; > > This could make start > end... But only in the case where (*end - *start < 16MB) - i.e. this region wasn't big enough. The generic code will deal with that and reject the hole. > It also doesn't handle not starting on a 4G boundary (or was that > condition check supposed to be included still). I was assuming we still had this before that code: if ((*start & PFN_4G_MASK) == 0) (*start)++; if ((*end & PFN_4G_MASK) == 0) (*end)--; And when bumping *start to next_seg, there's a +1 to deal with the 4GB boundary case. Steve >>> >>> *end = min(*end, ALIGN(*start, PFN_4G) - 1); >>> >>> So always allocate at the beginning, but skip past the next 4GB boundary >>> if there's less than 16MB left (or equal to avoid the 4GB boundary). >> >> Ah, there it is! I think it generalises even further by just changing >> the condition to "if (next_seg - *start < *end - next_seg)", although >> we'd need to ensure a signed comparison to cover the case where start >> and end are already in the same segment. > > IMO, relying on signed comparsion doesn't really improve the readability... > > Rob > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
