On Sat, Feb 09, 2019 at 03:07:49PM +0100, Andrea Righi wrote: > This is an attempt to mitigate the priority inversion problem of a > high-priority blkcg issuing a sync() and being forced to wait the > completion of all the writeback I/O generated by any other low-priority > blkcg, causing massive latencies to processes that shouldn't be > I/O-throttled at all. > > The idea is to save a list of blkcg's that are waiting for writeback: > every time a sync() is executed the current blkcg is added to the list. > > Then, when I/O is throttled, if there's a blkcg waiting for writeback > different than the current blkcg, no throttling is applied (we can > probably refine this logic later, i.e., a better policy could be to > adjust the throttling I/O rate using the blkcg with the highest speed > from the list of waiters - priority inheritance, kinda). > > This topic has been discussed here: > https://lwn.net/ml/cgroups/20190118103127.325-1-righi.andrea@xxxxxxxxx/ > > But we didn't come up with any definitive solution. > > This patch is not a definitive solution either, but it's an attempt to > continue addressing this issue and handling the priority inversion > problem with sync() in a better way. > > Signed-off-by: Andrea Righi <righi.andrea@xxxxxxxxx> Talked with Tejun about this some and we agreed the following is probably the best way forward 1) Track the submitter of the wb work to the writeback code. 2) Sync() defaults to the root cg, and and it writes all the things as the root cg. 3) Add a flag to the cgroups that would make sync()'ers in that group only be allowed to write out things that belong to its group. This way we avoid the priority inversion of having things like systemd or random logged in user doing sync() and having to wait, and we keep low prio cgroups from causing big IO storms by syncing out stuff and getting upgraded to root priority just to avoid the inversion. Obviously by default we want this flag to be off since its such a big change, but people/setups really worried about this behavior (Facebook for instance would likely use this flag) can go ahead and set it and be sure we're getting good isolation and still avoiding the priority inversion associated with running sync from a high priority context. Thanks, Josef
