> Il giorno 06 nov 2017, alle ore 17:39, Jens Axboe <axboe@xxxxxxxxx> ha scritto: > > On 11/06/2017 09:37 AM, Tejun Heo wrote: >> On Mon, Nov 06, 2017 at 05:33:45PM +0100, Paolo Valente wrote: >>> >>>> Il giorno 06 nov 2017, alle ore 17:30, Tejun Heo <tj@xxxxxxxxxx> ha scritto: >>>> >>>> Hello, >>>> >>>> On Mon, Nov 06, 2017 at 05:26:38PM +0100, Paolo Valente wrote: >>>>> Yes. DEBUG_BLK_CGROUP will just serve to keep the for-debugging >>>>> subset switched off even when the dynamic parameter is on. >>>>> >>>>> We'll wait for possible counter-arguments from Tejun, and then start >>>>> to work on what you propose. >>>> >>>> So, I think the only *really* required ones are io_serviced and the >>>> corresponding byte counts, which are tracked by blk-cgroup core >>>> anyway. Everything else can be hidden behind an obviously debug boot >>>> param, say, bfq.__DEBUG__extra_stats or whatever. >>>> >>> >>> ok, can we change it, or do you want us to change it, for cfq too? >> >> I don't have a strong opinion there but it coudl be more hassle than >> its worth given how long the stats have been out there. idk. >> >> Please also note that you can still allow the extra stats to be >> enabled run-time through /sys/kernel/module moduleparams and gate them >> with the static_branch. No idea whether making it that way is >> worthwhile tho. Creating / removing the files dynamically is >> supported but given that some stats need to be tracking all the time >> to be correct (like # in flights), it seems kinda silly. > > Yeah, I'd keep them enable-only through some boot/module parameter. > Folks using this can just reboot to turn them back off. > Ok to change our solution that way. One question though: if this switch must be system-wide (as I seem to understand), and not only bfq-specific, is a per-scheduler-module parameter ok? > Bonus points for doing the same for CFQ. > Prepare the points ... :) I'm unsure we can finish all this by the next merge window, so, to not miss the chance of the merge window for the other patches we have already almost finished, in the next days we'll submit only patches to: 1) Hide behind DEBUG_BLK_CGROUP all the parameters that Tejun said can be hidden behind it (only for bfq, as agreed) 2) Move the execution of stat update functions outside the scheduler lock, which does increase parallelism, and provides a boost of about 30% of sustainable throughput when stats need to be update. Because of the patch in item 1), this boost will provide most benefits when DEBUG_BLK_CGROUP is set, and when also the boot/module parameter is set, after we will have implemented that too. Thanks, Paolo > -- > Jens Axboe
