On 2019/3/10 下午5:40, Nikolay Borisov wrote:
>
>
> On 10.03.19 г. 11:34 ч., Qu Wenruo wrote:
>>
>>
>> On 2019/3/10 下午5:29, Nikolay Borisov wrote:
>>>
>>>
>>> On 10.03.19 г. 5:08 ч., Anand Jain wrote:
>>>>
>>>> I agree we need btrfs specific performance measurements and its
>>>> my list too.
>>>>
>>>> However my idea was to add it as a btrfs-progs subcommand such as
>>>>
>>>> btrfs inspect perf ...
>>>>
>>>> And implement by using the systemtap/perf/bpf/dtrace, as these
>>>> can tap the kernel functions from the useland using which we
>>>> can measure the time taken and no kernel changes will be required.
>>>> But yes we need to update the btrfs-progs if we rename the kernel
>>>> function, which I think is ok.
>>>>
>>>> I was too early trying this with bpf before, probably there are
>>>> more tools now to do that same thing.
>>>
>>> This is way too developer oriented to be included in the generic btrfs
>>> tools. Frankly bpf makes sense but only as a separate script being
>>> developed and possibly shared on github or whatnot so that other
>>> interested people can use it. However, integrating with btrfs-progs
>>> definitely seems the wrong thing to do.
>>>
>>> On the same note - I'm highly against this patchset landing in the kernel.
>>
>> Are you against the interface part or the btrfs specific perf part?
>>
>> For the first half, indeed the new sysfs is not good and I'm OK to move
>> to any more established interface.
>>
>> For the later half, if bfp/ftrace can do the same work, then I'm fine.
>> Otherwise the need is still here, both developer or experienced sys
>> admin will need this feature.
>
> I'm against this functionality being part of btrfs. What would be more
> useful is a set of bpf scripts which might very well be developed
> alongside the main project. They will be a lot more flexible also they
> will add overhead only when you really need them, whereas the current
> approach AFAIK (I might be wrong, correct me if so) always accumulates
> certain stats irrespective of whether the sys admin needs them or not?
Since this patchset is mostly an RFC, I skipped a lot things like mount
option to skip all such events.
But the bfp part is really a good direction for me to investigate.
If we can do the same work using more established facility, then I'm
completely fine to discard the patchset.
Thanks,
Qu
> At the very least you will sprinkle code such as :
>
> 'if (perf measurement enabled) ..... '
>
> I'd really we didn't add this clutter.
>
>>
>> Thanks,
>> Qu
>>
>>