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