Em Fri, Sep 20, 2019 at 09:46:15PM +0000, Steve MacLean escreveu:
> >> after->start = map->end;
> >> + after->pgoff = pos->map_ip(pos, map->end);
> >
> > So is this equivalent to what __split_vma() does in the kernel, i.e.:
> >
> > if (new_below)
> > new->vm_end = addr;
> > else {
> > new->vm_start = addr;
> > new->vm_pgoff += ((addr - vma->vm_start) >> PAGE_SHIFT);
> > }
> >
> > where new->vm_pgoff starts equal to the vm_pgoff of the mmap being split?
>
> It is roughly equivalent. The pgoff in struct map is stored in bytes not in
> pages, so it doesn't include the shift.
>
> An earlier version of this patch used:
> after->start = map->end;
> + after->pgoff += map->end - pos->start;
>
> Instead of the newer Functionally equivalent:
> after->start = map->end;
> + after->pgoff = pos->map_ip(pos, map->end);
>
> I preferred the latter form as it made more sense with the assertion that the
> mapping of map->end should match in pos and after.
Sorry for the delay in continuing with this discussion, I was at
Plumbers in Lisbon and then some vacations, etc. Also I was hoping
someone else would jump here and provide some Reviewed-by tag, etc :-)
So, if they are equivalent then I think its better to use code that
ressembles the kernel as much as possible, so that when in doubt we can
compare the tools/perf calcs with how the kernel does it, filtering out
things like the PAGE_SHIFT, can we go that way?
Also do you have some reproducer, if you have one then we can try and
have this as a 'perf test' entry, bolting some more checks into
tools/perf/tests/perf-record.c or using it as a start for a test that
stresses this code.
This is not a prerequisite for having your fix on, but would help
checking that perf doesn't regresses in this area.
- Arnaldo
