Hi Anand, re: inflight calculation
On Thu, Feb 11, 2021 at 03:55:33PM +0000, Michal Rostecki wrote: > > It is better to have random workloads in the above three categories > > of configs. > > > > Apart from the above three configs, there is also > > all-non-rotational with hetero > > For example, ssd and nvme together both are non-rotational. > > And, > > all-rotational with hetero > > For example, rotational disks with different speeds. > > > > > > The inflight calculation is local to btrfs. If the device is busy due to > > external factors, it would not switch to the better performing device. > > > > Good point. Maybe I should try to use the part stats instead of storing > inflight locally in btrfs. I tried today to reuse the inflight calculation which is done for iostat. And I came to conclusion that it's impossible without exporting some methods from the block/ subsystem. The thing is that there are two methods of calculating inflight. Which one of them is used, depends on queue_is_mq(): https://github.com/kdave/btrfs-devel/blob/9d294a685fbcb256ce8c5f7fd88a7596d0f52a8a/block/genhd.c#L1163 And if that condition is true, I noticed that part_stats return 0, even though there are processed requests (I checked with fio inside VM). In general, those two methods of checking inflight are: 1) part_stats - which would be trivial to reuse, just a matter of one simple for_each_possible_cpu() loop with part_stat_local_read_cpu() https://github.com/kdave/btrfs-devel/blob/9d294a685fbcb256ce8c5f7fd88a7596d0f52a8a/block/genhd.c#L133-L146 2) blk_mq_in_flight() - which has a lot of code and unexported structs inside the block/ directory, double function callback; definitely not easy to reimplement easily in btrfs without copying dozens of lines https://github.com/kdave/btrfs-devel/blob/9d294a685fbcb256ce8c5f7fd88a7596d0f52a8a/block/blk-mq.c#L115-L123 Well, I tried copying the whole blk_mq_in_flight() function with all dependencies anyway, hard to do without causing modpost errors. So, to sum it up, I think that making 2) possible to reuse in btrfs would require at lest exporting the blk_mq_in_flight() function, therefore the change would have to go through linux-block tree. Which maybe would be a good thing to do in long term, not sure if it should block my patchset entirely. The question is if we are fine with inflight stats inside btrfs. Personally I think we sholdn't be concerned too much about it. The inflight counter in my patches is a percpu counted, used in places which already do some atomic operations. Thanks, Michal
