On 24/06/19 10:06, Peter Xu wrote: > Well, if with such an error we'd better fix it right away in this > patch... :) > > Let me wait for some more comments, I'll touch that up too if I need a > repost.
Looks good to me, except for one minor issue in this patch. But do not attribute this one to me, it's basically all code from you. > +static uint64_t vtd_get_next_mask(uint64_t start, uint64_t size, int gaw) > +{ > + /* Tries to find smallest mask from start first */ > + uint64_t rmask = start & -start, max_mask = 1ULL << gaw; > + > + assert(size && gaw > 0 && gaw < 64); > + > + /* Zero start, or too big */ > + if (!rmask || rmask > max_mask) { > + rmask = max_mask; > + } Perhaps simpler: uint64_t max_mask = 1ULL << gaw; uint64_t alignment = start ? start & -start : max_mask; size = MIN(size, max_mask); if (alignment <= size) { /* Increase the alignment of start */ return alignment; } else { /* Find the largest page mask from size */ return 1ULL << (63 - clz64(size)); } Also please rename it to get_naturally_aligned_size. Paolo