Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 221 bytes --]

Hi,

As we have memleak.py in bcc tools, and it provides better info
including the calling stack, I'm wondering if we should replace current
eb leakage check with bcc based one.

Any idea on this?

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Nikolay Borisov]

On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
> Hi,
> 
> As we have memleak.py in bcc tools, and it provides better info
> including the calling stack, I'm wondering if we should replace current
> eb leakage check with bcc based one.
> 
> Any idea on this?

Why do you want to change it, given that the existing one is only
activate if btrfs is compiled with debug enabled and is "seamless"? What
are we going to gain by introducing yet another tool or replacing an
existing infrastructure with bcc (or whatever) based one?

> 
> Thanks,
> Qu
> 

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Qu Wenruo]

On 2019/4/10 下午2:51, Nikolay Borisov wrote:
>
>
> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>> Hi,
>>
>> As we have memleak.py in bcc tools, and it provides better info
>> including the calling stack, I'm wondering if we should replace current
>> eb leakage check with bcc based one.
>>
>> Any idea on this?
>
> Why do you want to change it, given that the existing one is only
> activate if btrfs is compiled with debug enabled and is "seamless"?

Well, I don't want to change that part, but to enhance it by external tools.

E.g, better call stack to mimic valgrind output. So when one developer
caused some leakage,  he or she can locate the problem easier.

Personally speaking, the external tool may be just an good enhancement,
but may not get much usage.

Thanks,
Qu

 What
> are we going to gain by introducing yet another tool or replacing an
> existing infrastructure with bcc (or whatever) based one?
>
>>
>> Thanks,
>> Qu
>>

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Nikolay Borisov]

On 10.04.19 г. 9:58 ч., Qu Wenruo wrote:
> 
> 
> On 2019/4/10 下午2:51, Nikolay Borisov wrote:
>>
>>
>> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>>> Hi,
>>>
>>> As we have memleak.py in bcc tools, and it provides better info
>>> including the calling stack, I'm wondering if we should replace current
>>> eb leakage check with bcc based one.
>>>
>>> Any idea on this?
>>
>> Why do you want to change it, given that the existing one is only
>> activate if btrfs is compiled with debug enabled and is "seamless"?
> 
> Well, I don't want to change that part, but to enhance it by external tools.
> 
> E.g, better call stack to mimic valgrind output. So when one developer
> caused some leakage,  he or she can locate the problem easier.
> 
> Personally speaking, the external tool may be just an good enhancement,
> but may not get much usage.

How do you envision this bcc-based tool to work? Create a probe hooked
up to eb alloc function which records allocated eb's, a probe on eb free
function and finally have another probe at module unload to check that
all ebs are freed?

> 
> Thanks,
> Qu
> 
>  What
>> are we going to gain by introducing yet another tool or replacing an
>> existing infrastructure with bcc (or whatever) based one?
>>
>>>
>>> Thanks,
>>> Qu
>>>
> 

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Qu Wenruo]

On 2019/4/10 下午3:01, Nikolay Borisov wrote:
>
>
> On 10.04.19 г. 9:58 ч., Qu Wenruo wrote:
>>
>>
>> On 2019/4/10 下午2:51, Nikolay Borisov wrote:
>>>
>>>
>>> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>>>> Hi,
>>>>
>>>> As we have memleak.py in bcc tools, and it provides better info
>>>> including the calling stack, I'm wondering if we should replace current
>>>> eb leakage check with bcc based one.
>>>>
>>>> Any idea on this?
>>>
>>> Why do you want to change it, given that the existing one is only
>>> activate if btrfs is compiled with debug enabled and is "seamless"?
>>
>> Well, I don't want to change that part, but to enhance it by external tools.
>>
>> E.g, better call stack to mimic valgrind output. So when one developer
>> caused some leakage,  he or she can locate the problem easier.
>>
>> Personally speaking, the external tool may be just an good enhancement,
>> but may not get much usage.
>
> How do you envision this bcc-based tool to work? Create a probe hooked
> up to eb alloc function which records allocated eb's, a probe on eb free
> function and finally have another probe at module unload to check that
> all ebs are freed?

hook to catch free_extent_buffer(), get_extent_buffer() (does the
function just get removed?) and alloc_extent_buffer(), passing all the
@bytenr and stack to user space.

That's all needed to be done in kernel space.

User space catch the bytenr and stack to match the refs increase and
decrease.
No hook is needed at modules unload.

Each refs inc/dec will trigger a check. If one eb get refs == 0, then
it's removed from the list/dict/whatever the structure is called in
python, along with all its previous call stacks.

If dec on an non-existing one, output the stack, this should be the same
behavior of current kernel.

If any eb still has refs >0, output all the allocation stack, this the
enhanced behavior.

It's users' response to ensure the tool is loaded/terminated at proper
timing, e.g. loaded before mounting the fs of interest and terminated
after the fs is unmounted.

Thanks,
Qu

>
>>
>> Thanks,
>> Qu
>>
>>  What
>>> are we going to gain by introducing yet another tool or replacing an
>>> existing infrastructure with bcc (or whatever) based one?
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Nikolay Borisov]

On 10.04.19 г. 10:11 ч., Qu Wenruo wrote:
> 
> 
> On 2019/4/10 下午3:01, Nikolay Borisov wrote:
>>
>>
>> On 10.04.19 г. 9:58 ч., Qu Wenruo wrote:
>>>
>>>
>>> On 2019/4/10 下午2:51, Nikolay Borisov wrote:
>>>>
>>>>
>>>> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>>>>> Hi,
>>>>>
>>>>> As we have memleak.py in bcc tools, and it provides better info
>>>>> including the calling stack, I'm wondering if we should replace current
>>>>> eb leakage check with bcc based one.
>>>>>
>>>>> Any idea on this?
>>>>
>>>> Why do you want to change it, given that the existing one is only
>>>> activate if btrfs is compiled with debug enabled and is "seamless"?
>>>
>>> Well, I don't want to change that part, but to enhance it by external tools.
>>>
>>> E.g, better call stack to mimic valgrind output. So when one developer
>>> caused some leakage,  he or she can locate the problem easier.
>>>
>>> Personally speaking, the external tool may be just an good enhancement,
>>> but may not get much usage.
>>
>> How do you envision this bcc-based tool to work? Create a probe hooked
>> up to eb alloc function which records allocated eb's, a probe on eb free
>> function and finally have another probe at module unload to check that
>> all ebs are freed?
> 
> hook to catch free_extent_buffer(), get_extent_buffer() (does the
> function just get removed?) and alloc_extent_buffer(), passing all the
> @bytenr and stack to user space.
> 
> That's all needed to be done in kernel space.
> 
> User space catch the bytenr and stack to match the refs increase and
> decrease.
> No hook is needed at modules unload.
> 
> Each refs inc/dec will trigger a check. If one eb get refs == 0, then
> it's removed from the list/dict/whatever the structure is called in
> python, along with all its previous call stacks.
> 
> If dec on an non-existing one, output the stack, this should be the same
> behavior of current kernel.
> 
> If any eb still has refs >0, output all the allocation stack, this the
> enhanced behavior.
> 
> It's users' response to ensure the tool is loaded/terminated at proper
> timing, e.g. loaded before mounting the fs of interest and terminated
> after the fs is unmounted.

Such tool might indeed be useful, however it's going to be just a 3rd
party tool. As such it's up to you whether you want to develop it and
use or not and share it on github. However, the in-kernel infrastructure
should stay untouched (although it's rather crude). Subsequently we
might think about removing it.

> 
> Thanks,
> Qu
> 
>>
>>>
>>> Thanks,
>>> Qu
>>>
>>>  What
>>>> are we going to gain by introducing yet another tool or replacing an
>>>> existing infrastructure with bcc (or whatever) based one?
>>>>
>>>>>
>>>>> Thanks,
>>>>> Qu
>>>>>
>>>
> 

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Qu Wenruo]

On 2019/4/10 下午3:16, Nikolay Borisov wrote:
>
>
> On 10.04.19 г. 10:11 ч., Qu Wenruo wrote:
>>
>>
>> On 2019/4/10 下午3:01, Nikolay Borisov wrote:
>>>
>>>
>>> On 10.04.19 г. 9:58 ч., Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2019/4/10 下午2:51, Nikolay Borisov wrote:
>>>>>
>>>>>
>>>>> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>>>>>> Hi,
>>>>>>
>>>>>> As we have memleak.py in bcc tools, and it provides better info
>>>>>> including the calling stack, I'm wondering if we should replace current
>>>>>> eb leakage check with bcc based one.
>>>>>>
>>>>>> Any idea on this?
>>>>>
>>>>> Why do you want to change it, given that the existing one is only
>>>>> activate if btrfs is compiled with debug enabled and is "seamless"?
>>>>
>>>> Well, I don't want to change that part, but to enhance it by external tools.
>>>>
>>>> E.g, better call stack to mimic valgrind output. So when one developer
>>>> caused some leakage,  he or she can locate the problem easier.
>>>>
>>>> Personally speaking, the external tool may be just an good enhancement,
>>>> but may not get much usage.
>>>
>>> How do you envision this bcc-based tool to work? Create a probe hooked
>>> up to eb alloc function which records allocated eb's, a probe on eb free
>>> function and finally have another probe at module unload to check that
>>> all ebs are freed?
>>
>> hook to catch free_extent_buffer(), get_extent_buffer() (does the
>> function just get removed?) and alloc_extent_buffer(), passing all the
>> @bytenr and stack to user space.
>>
>> That's all needed to be done in kernel space.
>>
>> User space catch the bytenr and stack to match the refs increase and
>> decrease.
>> No hook is needed at modules unload.
>>
>> Each refs inc/dec will trigger a check. If one eb get refs == 0, then
>> it's removed from the list/dict/whatever the structure is called in
>> python, along with all its previous call stacks.
>>
>> If dec on an non-existing one, output the stack, this should be the same
>> behavior of current kernel.
>>
>> If any eb still has refs >0, output all the allocation stack, this the
>> enhanced behavior.
>>
>> It's users' response to ensure the tool is loaded/terminated at proper
>> timing, e.g. loaded before mounting the fs of interest and terminated
>> after the fs is unmounted.
>
> Such tool might indeed be useful, however it's going to be just a 3rd
> party tool. As such it's up to you whether you want to develop it and
> use or not and share it on github. However, the in-kernel infrastructure
> should stay untouched (although it's rather crude). Subsequently we
> might think about removing it.

Good, I'll just try to craft the tool, and see if the community wants it.

Thanks,
Qu
>
>>
>> Thanks,
>> Qu
>>
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>  What
>>>>> are we going to gain by introducing yet another tool or replacing an
>>>>> existing infrastructure with bcc (or whatever) based one?
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Qu
>>>>>>
>>>>
>>

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[Chris Mason]

On 10 Apr 2019, at 2:51, Nikolay Borisov wrote:

> On 10.04.19 г. 9:34 ч., Qu Wenruo wrote:
>> Hi,
>>
>> As we have memleak.py in bcc tools, and it provides better info
>> including the calling stack, I'm wondering if we should replace 
>> current
>> eb leakage check with bcc based one.
>>
>> Any idea on this?
>
> Why do you want to change it, given that the existing one is only
> activate if btrfs is compiled with debug enabled and is "seamless"? 
> What
> are we going to gain by introducing yet another tool or replacing an
> existing infrastructure with bcc (or whatever) based one?

bcc/bpf are able to give much more information about which call chain 
allocated the extent buffer, but unfortunately almost all eb's come from 
the same place.  But, the bpf version could remember every call stack 
that did an inc/dec on the reference counter, which does make it 
dramatically easier to track down leaks.

With all of that said, the real reason I love the extent buffer leak 
code is that when chasing down a leak, I could rmmod btrfs, have the 
leak detector print a helpful warning, and then cleanup the slab cache 
so that I didn't have to reboot the test machine.

bpf code on top could definitely make it faster to find the cause of 
leaks, but I think the slab cleanup is worth keeping around.

-chris

Re: Is it a good idea to replace current eb leakage detector with bcc/ebpf based one?

[David Sterba]

On Wed, Apr 10, 2019 at 02:34:10PM +0800, Qu Wenruo wrote:
> As we have memleak.py in bcc tools, and it provides better info
> including the calling stack, I'm wondering if we should replace current
> eb leakage check with bcc based one.

The built-in checks have the advantage that they're built in. So no
additional tools or frameworks are needed, either provided by distro or
downloaded and built. So for some low-level and crude sanity checks we
still want something that ships with the code and can be run as part of
normal development & testing loop.

Providing additional enhanced output is of course welcome, that's useful
when one is actually looking for a specific problem or gathering traces
for analysis.

So I think we want both, ie. no 'replace'.