[lkml]   [2019]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] perf map: fix overlapped map handling
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

 \ /
  Last update: 2019-09-27 17:36    [W:0.091 / U:6.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site