[PATCH 0/4] perf maps/symbols: Various assert fixes

James Clark posted 4 patches 1 week, 4 days ago
tools/perf/util/maps.c   | 14 ++++++------
tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++----------------
2 files changed, 36 insertions(+), 27 deletions(-)
[PATCH 0/4] perf maps/symbols: Various assert fixes
Posted by James Clark 1 week, 4 days ago
A few different asserts are hit when running perf report on minimal
Arm systems when kcore is used, or the .debug/ info can't be loaded or
/boot isn't mounted etc.

These result in some less common paths being hit for resolving symbols
and things are done in an order that breaks some assumptions. I'm not
sure if we could do something to make the tests pick this up, but maybe
not easily if it would involve mocking the filesystem or even a specific
kernel. I tried a few different variations of --kcore and --vmlinux
arguments but ultimately I could only reproduce these issues by running
on specific kernels and root filesystems.

James Clark (4):
  perf symbols: Remove map from list before updating addresses
  perf maps: Re-use __maps__free_maps_by_name()
  perf symbols: Update kcore map before merging in remaining symbols
  perf symbols: Fix ownership of string in dso__load_vmlinux()

 tools/perf/util/maps.c   | 14 ++++++------
 tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 27 deletions(-)

-- 
2.34.1
Re: [PATCH 0/4] perf maps/symbols: Various assert fixes
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, May 07, 2024 at 03:12:04PM +0100, James Clark wrote:
> A few different asserts are hit when running perf report on minimal
> Arm systems when kcore is used, or the .debug/ info can't be loaded or
> /boot isn't mounted etc.
> 
> These result in some less common paths being hit for resolving symbols
> and things are done in an order that breaks some assumptions. I'm not
> sure if we could do something to make the tests pick this up, but maybe
> not easily if it would involve mocking the filesystem or even a specific
> kernel. I tried a few different variations of --kcore and --vmlinux
> arguments but ultimately I could only reproduce these issues by running
> on specific kernels and root filesystems.

Please consider adding Fixes tags so that we can help the work of
backporters/stable?

- Arnaldo
 
> James Clark (4):
>   perf symbols: Remove map from list before updating addresses
>   perf maps: Re-use __maps__free_maps_by_name()
>   perf symbols: Update kcore map before merging in remaining symbols
>   perf symbols: Fix ownership of string in dso__load_vmlinux()
> 
>  tools/perf/util/maps.c   | 14 ++++++------
>  tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++----------------
>  2 files changed, 36 insertions(+), 27 deletions(-)
> 
> -- 
> 2.34.1
Re: [PATCH 0/4] perf maps/symbols: Various assert fixes
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, May 07, 2024 at 12:11:42PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, May 07, 2024 at 03:12:04PM +0100, James Clark wrote:
> > A few different asserts are hit when running perf report on minimal
> > Arm systems when kcore is used, or the .debug/ info can't be loaded or
> > /boot isn't mounted etc.
> > 
> > These result in some less common paths being hit for resolving symbols
> > and things are done in an order that breaks some assumptions. I'm not
> > sure if we could do something to make the tests pick this up, but maybe
> > not easily if it would involve mocking the filesystem or even a specific
> > kernel. I tried a few different variations of --kcore and --vmlinux
> > arguments but ultimately I could only reproduce these issues by running
> > on specific kernels and root filesystems.
> 
> Please consider adding Fixes tags so that we can help the work of
> backporters/stable?

Sorry, you already did it for the last two patches in the series, so you
couldn't find easily what were the csets that introduced the problems in
the first two patches?

- Arnaldo
>  
> > James Clark (4):
> >   perf symbols: Remove map from list before updating addresses
> >   perf maps: Re-use __maps__free_maps_by_name()
> >   perf symbols: Update kcore map before merging in remaining symbols
> >   perf symbols: Fix ownership of string in dso__load_vmlinux()
> > 
> >  tools/perf/util/maps.c   | 14 ++++++------
> >  tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++----------------
> >  2 files changed, 36 insertions(+), 27 deletions(-)
> > 
> > -- 
> > 2.34.1
Re: [PATCH 0/4] perf maps/symbols: Various assert fixes
Posted by James Clark 1 week, 4 days ago

On 07/05/2024 16:12, Arnaldo Carvalho de Melo wrote:
> On Tue, May 07, 2024 at 12:11:42PM -0300, Arnaldo Carvalho de Melo wrote:
>> On Tue, May 07, 2024 at 03:12:04PM +0100, James Clark wrote:
>>> A few different asserts are hit when running perf report on minimal
>>> Arm systems when kcore is used, or the .debug/ info can't be loaded or
>>> /boot isn't mounted etc.
>>>
>>> These result in some less common paths being hit for resolving symbols
>>> and things are done in an order that breaks some assumptions. I'm not
>>> sure if we could do something to make the tests pick this up, but maybe
>>> not easily if it would involve mocking the filesystem or even a specific
>>> kernel. I tried a few different variations of --kcore and --vmlinux
>>> arguments but ultimately I could only reproduce these issues by running
>>> on specific kernels and root filesystems.
>>
>> Please consider adding Fixes tags so that we can help the work of
>> backporters/stable?
> 
> Sorry, you already did it for the last two patches in the series, so you
> couldn't find easily what were the csets that introduced the problems in
> the first two patches?
> 
> - Arnaldo

1,3 and 4 are all fixes and have tags. 2 is just a tidyup so I didn't
add the tag. I probably should have explained that in the cover letter.

>>  
>>> James Clark (4):
>>>   perf symbols: Remove map from list before updating addresses
>>>   perf maps: Re-use __maps__free_maps_by_name()
>>>   perf symbols: Update kcore map before merging in remaining symbols
>>>   perf symbols: Fix ownership of string in dso__load_vmlinux()
>>>
>>>  tools/perf/util/maps.c   | 14 ++++++------
>>>  tools/perf/util/symbol.c | 49 ++++++++++++++++++++++++----------------
>>>  2 files changed, 36 insertions(+), 27 deletions(-)
>>>
>>> -- 
>>> 2.34.1
>