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
>>
>>

Reply via email to