| Message ID | 20181112095632.69114-1-paolo.valente@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | unify the interface of the proportional-share policy in blkio/io | expand |
Forgot to CC Lennart, sorry. > Il giorno 12 nov 2018, alle ore 10:56, Paolo Valente <paolo.valente@linaro.org> ha scritto: > > Hi Jens, Tejun, all, > about nine months ago, we agreed on a solution for unifying the > interface of the proportional-share policy in blkio/io [1]. Angelo > and I finally completed it. Let me briefly recall the problem and the > solution. > > The current implementation of cgroups doesn't allow two or more > entities, e.g., I/O schedulers, to share the same files. So, if CFQ > creates its files for the proportional-share policy, such as, e.g, > weight files for blkio/io groups, BFQ cannot attach somehow to them. > Thus, to enable people to set group weights with BFQ, I resorted to > making BFQ create its own version of these common files, by prepending > a bfq prefix. > > Actually, no legacy code uses these different names, or is likely to > do so. Having these two sets of names is simply a source of > confusion, as pointed out also, e.g., by Lennart Poettering (CCed > here), and acknowledged by Tejun [2]. > > In [1] we agreed on a solution that solves this problem, by actually > making it possible to share cgroups files. Both writing to and > reading from a shared file trigger the appropriate operation for each > of the entities that share the file. In particular, in case of > reading, > - if all entities produce the same output, the this common output is > shown only once; > - if the outputs differ, then every per-entity output is shown, > preceded by the name of the entity that produced that output. > > With this solution, legacy code that, e.g., sets group weights, just > works, regardless of the I/O scheduler actually implementing > proportional share. > > But note that this extension is not restricted to only blkio/io. The > general group interface now enables files to be shared among multiple > entities of any kind. > > (I have also added a patch to fix some clerical errors in bfq doc, > which I found while making the latter consistent with the new > interface.) > > Thanks, > Paolo > > [1] https://lkml.org/lkml/2018/1/4/667 > [2] https://github.com/systemd/systemd/issues/7057 > > Angelo Ruocco (7): > kernfs: add function to find kernfs_node without increasing ref > counter > cgroup: link cftypes of the same subsystem with the same name > cgroup: add owner name to cftypes > block, bfq: align min and default weights with cfq > cgroup: make all functions of all cftypes be invoked > block, cfq: allow cgroup files to be shared > block, throttle: allow sharing cgroup statistic files > > Paolo Valente (5): > cgroup: add hook seq_show_cft with also the owning cftype as parameter > block, cgroup: pass cftype to functions that need to use it > block, bfq: use standard file names for the proportional-share policy > doc, bfq-iosched: fix a few clerical errors > doc, bfq-iosched: make it consistent with the new cgroup interface > > Documentation/block/bfq-iosched.txt | 31 +++-- > block/bfq-cgroup.c | 148 +++++++++++++------- > block/bfq-iosched.h | 4 +- > block/blk-cgroup.c | 22 +-- > block/blk-throttle.c | 24 ++-- > block/cfq-iosched.c | 105 +++++++++++---- > fs/kernfs/dir.c | 13 ++ > include/linux/blk-cgroup.h | 10 +- > include/linux/cgroup-defs.h | 14 +- > include/linux/cgroup.h | 13 ++ > include/linux/kernfs.h | 7 + > kernel/cgroup/cgroup.c | 262 +++++++++++++++++++++++++++++------- > 12 files changed, 483 insertions(+), 170 deletions(-) > > -- > 2.16.1
On 12.11.2018 10:56, Paolo Valente wrote: > Hi Jens, Tejun, all, > about nine months ago, we agreed on a solution for unifying the > interface of the proportional-share policy in blkio/io [1]. Angelo > and I finally completed it. Let me briefly recall the problem and the > solution. > > The current implementation of cgroups doesn't allow two or more > entities, e.g., I/O schedulers, to share the same files. So, if CFQ > creates its files for the proportional-share policy, such as, e.g, > weight files for blkio/io groups, BFQ cannot attach somehow to them. > Thus, to enable people to set group weights with BFQ, I resorted to > making BFQ create its own version of these common files, by prepending > a bfq prefix. > > Actually, no legacy code uses these different names, or is likely to > do so. Having these two sets of names is simply a source of > confusion, as pointed out also, e.g., by Lennart Poettering (CCed > here), and acknowledged by Tejun [2]. > > In [1] we agreed on a solution that solves this problem, by actually > making it possible to share cgroups files. Both writing to and > reading from a shared file trigger the appropriate operation for each > of the entities that share the file. In particular, in case of > reading, > - if all entities produce the same output, the this common output is > shown only once; > - if the outputs differ, then every per-entity output is shown, > preceded by the name of the entity that produced that output. > > With this solution, legacy code that, e.g., sets group weights, just > works, regardless of the I/O scheduler actually implementing > proportional share. > > But note that this extension is not restricted to only blkio/io. The > general group interface now enables files to be shared among multiple > entities of any kind. > > (I have also added a patch to fix some clerical errors in bfq doc, > which I found while making the latter consistent with the new > interface.) > > Thanks, > Paolo > > [1] https://lkml.org/lkml/2018/1/4/667 > [2] https://github.com/systemd/systemd/issues/7057 > > Angelo Ruocco (7): > kernfs: add function to find kernfs_node without increasing ref > counter > cgroup: link cftypes of the same subsystem with the same name > cgroup: add owner name to cftypes > block, bfq: align min and default weights with cfq > cgroup: make all functions of all cftypes be invoked > block, cfq: allow cgroup files to be shared > block, throttle: allow sharing cgroup statistic files > > Paolo Valente (5): > cgroup: add hook seq_show_cft with also the owning cftype as > parameter > block, cgroup: pass cftype to functions that need to use it > block, bfq: use standard file names for the proportional-share policy > doc, bfq-iosched: fix a few clerical errors > doc, bfq-iosched: make it consistent with the new cgroup interface > > Documentation/block/bfq-iosched.txt | 31 +++-- > block/bfq-cgroup.c | 148 +++++++++++++------- > block/bfq-iosched.h | 4 +- > block/blk-cgroup.c | 22 +-- > block/blk-throttle.c | 24 ++-- > block/cfq-iosched.c | 105 +++++++++++---- > fs/kernfs/dir.c | 13 ++ > include/linux/blk-cgroup.h | 10 +- > include/linux/cgroup-defs.h | 14 +- > include/linux/cgroup.h | 13 ++ > include/linux/kernfs.h | 7 + > kernel/cgroup/cgroup.c | 262 > +++++++++++++++++++++++++++++------- > 12 files changed, 483 insertions(+), 170 deletions(-) > > -- > 2.16.1 I thought all the legacy stuff including CFS et al. is going to be removed in v4.21 completely…
On 12.11.2018 11:00, Oleksandr Natalenko wrote: > I thought all the legacy stuff including CFS et al. is going to be > removed in v4.21 completely… Paolo, [1] and [2]. [1] http://git.kernel.dk/cgit/linux-block/log/?h=for-4.21/block [2] http://git.kernel.dk/cgit/linux-block/commit/?h=for-4.21/block&id=f382fb0bcef4c37dc049e9f6963e3baf204d815c
> Il giorno 12 nov 2018, alle ore 11:00, Oleksandr Natalenko <oleksandr@natalenko.name> ha scritto: > > On 12.11.2018 10:56, Paolo Valente wrote: >> Hi Jens, Tejun, all, >> about nine months ago, we agreed on a solution for unifying the >> interface of the proportional-share policy in blkio/io [1]. Angelo >> and I finally completed it. Let me briefly recall the problem and the >> solution. >> The current implementation of cgroups doesn't allow two or more >> entities, e.g., I/O schedulers, to share the same files. So, if CFQ >> creates its files for the proportional-share policy, such as, e.g, >> weight files for blkio/io groups, BFQ cannot attach somehow to them. >> Thus, to enable people to set group weights with BFQ, I resorted to >> making BFQ create its own version of these common files, by prepending >> a bfq prefix. >> Actually, no legacy code uses these different names, or is likely to >> do so. Having these two sets of names is simply a source of >> confusion, as pointed out also, e.g., by Lennart Poettering (CCed >> here), and acknowledged by Tejun [2]. >> In [1] we agreed on a solution that solves this problem, by actually >> making it possible to share cgroups files. Both writing to and >> reading from a shared file trigger the appropriate operation for each >> of the entities that share the file. In particular, in case of >> reading, >> - if all entities produce the same output, the this common output is >> shown only once; >> - if the outputs differ, then every per-entity output is shown, >> preceded by the name of the entity that produced that output. >> With this solution, legacy code that, e.g., sets group weights, just >> works, regardless of the I/O scheduler actually implementing >> proportional share. >> But note that this extension is not restricted to only blkio/io. The >> general group interface now enables files to be shared among multiple >> entities of any kind. >> (I have also added a patch to fix some clerical errors in bfq doc, >> which I found while making the latter consistent with the new >> interface.) >> Thanks, >> Paolo >> [1] https://lkml.org/lkml/2018/1/4/667 >> [2] https://github.com/systemd/systemd/issues/7057 >> Angelo Ruocco (7): >> kernfs: add function to find kernfs_node without increasing ref >> counter >> cgroup: link cftypes of the same subsystem with the same name >> cgroup: add owner name to cftypes >> block, bfq: align min and default weights with cfq >> cgroup: make all functions of all cftypes be invoked >> block, cfq: allow cgroup files to be shared >> block, throttle: allow sharing cgroup statistic files >> Paolo Valente (5): >> cgroup: add hook seq_show_cft with also the owning cftype as parameter >> block, cgroup: pass cftype to functions that need to use it >> block, bfq: use standard file names for the proportional-share policy >> doc, bfq-iosched: fix a few clerical errors >> doc, bfq-iosched: make it consistent with the new cgroup interface >> Documentation/block/bfq-iosched.txt | 31 +++-- >> block/bfq-cgroup.c | 148 +++++++++++++------- >> block/bfq-iosched.h | 4 +- >> block/blk-cgroup.c | 22 +-- >> block/blk-throttle.c | 24 ++-- >> block/cfq-iosched.c | 105 +++++++++++---- >> fs/kernfs/dir.c | 13 ++ >> include/linux/blk-cgroup.h | 10 +- >> include/linux/cgroup-defs.h | 14 +- >> include/linux/cgroup.h | 13 ++ >> include/linux/kernfs.h | 7 + >> kernel/cgroup/cgroup.c | 262 +++++++++++++++++++++++++++++------- >> 12 files changed, 483 insertions(+), 170 deletions(-) >> -- >> 2.16.1 > > I thought all the legacy stuff including CFS et al. is going to be removed in v4.21 completely… > Thanks for pointing this out. People with a lower kernel version than the future 4.21 just cannot and will not be able to use the proportional share policy on blk-mq (with legacy code), because of the name issue highlighted in this email. If this patch series gets accepted, a backport will solve the problem. In this respect, such a backport might even happen 'automatically', as most bfq commit seem to get backported to older, stable kernels. In addition, this extension - extends the whole cgroups interface, in a seamless and backward-compatible way, to prevent future issues like these; - solves a similar issue with throttle (which AFAIK won't go away with 4.21). Thanks, Paolo > -- > Oleksandr Natalenko (post-factum)
On 11/12/18 3:17 AM, Paolo Valente wrote: > > >> Il giorno 12 nov 2018, alle ore 11:00, Oleksandr Natalenko <oleksandr@natalenko.name> ha scritto: >> >> On 12.11.2018 10:56, Paolo Valente wrote: >>> Hi Jens, Tejun, all, >>> about nine months ago, we agreed on a solution for unifying the >>> interface of the proportional-share policy in blkio/io [1]. Angelo >>> and I finally completed it. Let me briefly recall the problem and the >>> solution. >>> The current implementation of cgroups doesn't allow two or more >>> entities, e.g., I/O schedulers, to share the same files. So, if CFQ >>> creates its files for the proportional-share policy, such as, e.g, >>> weight files for blkio/io groups, BFQ cannot attach somehow to them. >>> Thus, to enable people to set group weights with BFQ, I resorted to >>> making BFQ create its own version of these common files, by prepending >>> a bfq prefix. >>> Actually, no legacy code uses these different names, or is likely to >>> do so. Having these two sets of names is simply a source of >>> confusion, as pointed out also, e.g., by Lennart Poettering (CCed >>> here), and acknowledged by Tejun [2]. >>> In [1] we agreed on a solution that solves this problem, by actually >>> making it possible to share cgroups files. Both writing to and >>> reading from a shared file trigger the appropriate operation for each >>> of the entities that share the file. In particular, in case of >>> reading, >>> - if all entities produce the same output, the this common output is >>> shown only once; >>> - if the outputs differ, then every per-entity output is shown, >>> preceded by the name of the entity that produced that output. >>> With this solution, legacy code that, e.g., sets group weights, just >>> works, regardless of the I/O scheduler actually implementing >>> proportional share. >>> But note that this extension is not restricted to only blkio/io. The >>> general group interface now enables files to be shared among multiple >>> entities of any kind. >>> (I have also added a patch to fix some clerical errors in bfq doc, >>> which I found while making the latter consistent with the new >>> interface.) >>> Thanks, >>> Paolo >>> [1] https://lkml.org/lkml/2018/1/4/667 >>> [2] https://github.com/systemd/systemd/issues/7057 >>> Angelo Ruocco (7): >>> kernfs: add function to find kernfs_node without increasing ref >>> counter >>> cgroup: link cftypes of the same subsystem with the same name >>> cgroup: add owner name to cftypes >>> block, bfq: align min and default weights with cfq >>> cgroup: make all functions of all cftypes be invoked >>> block, cfq: allow cgroup files to be shared >>> block, throttle: allow sharing cgroup statistic files >>> Paolo Valente (5): >>> cgroup: add hook seq_show_cft with also the owning cftype as parameter >>> block, cgroup: pass cftype to functions that need to use it >>> block, bfq: use standard file names for the proportional-share policy >>> doc, bfq-iosched: fix a few clerical errors >>> doc, bfq-iosched: make it consistent with the new cgroup interface >>> Documentation/block/bfq-iosched.txt | 31 +++-- >>> block/bfq-cgroup.c | 148 +++++++++++++------- >>> block/bfq-iosched.h | 4 +- >>> block/blk-cgroup.c | 22 +-- >>> block/blk-throttle.c | 24 ++-- >>> block/cfq-iosched.c | 105 +++++++++++---- >>> fs/kernfs/dir.c | 13 ++ >>> include/linux/blk-cgroup.h | 10 +- >>> include/linux/cgroup-defs.h | 14 +- >>> include/linux/cgroup.h | 13 ++ >>> include/linux/kernfs.h | 7 + >>> kernel/cgroup/cgroup.c | 262 +++++++++++++++++++++++++++++------- >>> 12 files changed, 483 insertions(+), 170 deletions(-) >>> -- >>> 2.16.1 >> >> I thought all the legacy stuff including CFS et al. is going to be removed in v4.21 completely… >> > > Thanks for pointing this out. > > People with a lower kernel version than the future 4.21 just cannot > and will not be able to use the proportional share policy on blk-mq > (with legacy code), because of the name issue highlighted in this > email. If this patch series gets accepted, a backport will solve the > problem. In this respect, such a backport might even happen > 'automatically', as most bfq commit seem to get backported to older, > stable kernels. > > In addition, this extension > - extends the whole cgroups interface, in a seamless and > backward-compatible way, to prevent future issues like these; > - solves a similar issue with throttle (which AFAIK won't go away > with 4.21). There's no way this series can get accepted, since you've made the mistake of basing it on something that won't apply to the block tree for 4.21. I've outlined these rules before, but here they are again: 1) Patches destined for the CURRENT kernel version should be against my for-linus branch. That means that right now, any patches that should to into 4.20 should be against that. 2) Patches destined for the NEXT kernel version should be against my for-x.y/block branch, where x.y is the next version. As of right now, patches for 4.21 should be against my for-4.21/bloc branch. I'd encourage you to respin against that, particularly in this case since we've both got a lot of churn, and also removal of various items that you are patching here.
> Il giorno 12 nov 2018, alle ore 16:35, Jens Axboe <axboe@kernel.dk> ha scritto: > > On 11/12/18 3:17 AM, Paolo Valente wrote: >> >> >>> Il giorno 12 nov 2018, alle ore 11:00, Oleksandr Natalenko <oleksandr@natalenko.name> ha scritto: >>> >>> On 12.11.2018 10:56, Paolo Valente wrote: >>>> Hi Jens, Tejun, all, >>>> about nine months ago, we agreed on a solution for unifying the >>>> interface of the proportional-share policy in blkio/io [1]. Angelo >>>> and I finally completed it. Let me briefly recall the problem and the >>>> solution. >>>> The current implementation of cgroups doesn't allow two or more >>>> entities, e.g., I/O schedulers, to share the same files. So, if CFQ >>>> creates its files for the proportional-share policy, such as, e.g, >>>> weight files for blkio/io groups, BFQ cannot attach somehow to them. >>>> Thus, to enable people to set group weights with BFQ, I resorted to >>>> making BFQ create its own version of these common files, by prepending >>>> a bfq prefix. >>>> Actually, no legacy code uses these different names, or is likely to >>>> do so. Having these two sets of names is simply a source of >>>> confusion, as pointed out also, e.g., by Lennart Poettering (CCed >>>> here), and acknowledged by Tejun [2]. >>>> In [1] we agreed on a solution that solves this problem, by actually >>>> making it possible to share cgroups files. Both writing to and >>>> reading from a shared file trigger the appropriate operation for each >>>> of the entities that share the file. In particular, in case of >>>> reading, >>>> - if all entities produce the same output, the this common output is >>>> shown only once; >>>> - if the outputs differ, then every per-entity output is shown, >>>> preceded by the name of the entity that produced that output. >>>> With this solution, legacy code that, e.g., sets group weights, just >>>> works, regardless of the I/O scheduler actually implementing >>>> proportional share. >>>> But note that this extension is not restricted to only blkio/io. The >>>> general group interface now enables files to be shared among multiple >>>> entities of any kind. >>>> (I have also added a patch to fix some clerical errors in bfq doc, >>>> which I found while making the latter consistent with the new >>>> interface.) >>>> Thanks, >>>> Paolo >>>> [1] https://lkml.org/lkml/2018/1/4/667 >>>> [2] https://github.com/systemd/systemd/issues/7057 >>>> Angelo Ruocco (7): >>>> kernfs: add function to find kernfs_node without increasing ref >>>> counter >>>> cgroup: link cftypes of the same subsystem with the same name >>>> cgroup: add owner name to cftypes >>>> block, bfq: align min and default weights with cfq >>>> cgroup: make all functions of all cftypes be invoked >>>> block, cfq: allow cgroup files to be shared >>>> block, throttle: allow sharing cgroup statistic files >>>> Paolo Valente (5): >>>> cgroup: add hook seq_show_cft with also the owning cftype as parameter >>>> block, cgroup: pass cftype to functions that need to use it >>>> block, bfq: use standard file names for the proportional-share policy >>>> doc, bfq-iosched: fix a few clerical errors >>>> doc, bfq-iosched: make it consistent with the new cgroup interface >>>> Documentation/block/bfq-iosched.txt | 31 +++-- >>>> block/bfq-cgroup.c | 148 +++++++++++++------- >>>> block/bfq-iosched.h | 4 +- >>>> block/blk-cgroup.c | 22 +-- >>>> block/blk-throttle.c | 24 ++-- >>>> block/cfq-iosched.c | 105 +++++++++++---- >>>> fs/kernfs/dir.c | 13 ++ >>>> include/linux/blk-cgroup.h | 10 +- >>>> include/linux/cgroup-defs.h | 14 +- >>>> include/linux/cgroup.h | 13 ++ >>>> include/linux/kernfs.h | 7 + >>>> kernel/cgroup/cgroup.c | 262 +++++++++++++++++++++++++++++------- >>>> 12 files changed, 483 insertions(+), 170 deletions(-) >>>> -- >>>> 2.16.1 >>> >>> I thought all the legacy stuff including CFS et al. is going to be removed in v4.21 completely… >>> >> >> Thanks for pointing this out. >> >> People with a lower kernel version than the future 4.21 just cannot >> and will not be able to use the proportional share policy on blk-mq >> (with legacy code), because of the name issue highlighted in this >> email. If this patch series gets accepted, a backport will solve the >> problem. In this respect, such a backport might even happen >> 'automatically', as most bfq commit seem to get backported to older, >> stable kernels. >> >> In addition, this extension >> - extends the whole cgroups interface, in a seamless and >> backward-compatible way, to prevent future issues like these; >> - solves a similar issue with throttle (which AFAIK won't go away >> with 4.21). > > There's no way this series can get accepted, since you've made the > mistake of basing it on something that won't apply to the block > tree for 4.21. Of course, sorry :( We'll rebase V2. BTW, since this patch series is probably even more useful for older than for future kernels, might it make sense to also propose it for stable/longterm kernels (provided that such a possibility exists)? Thanks, Paolo > I've outlined these rules before, but here they are > again: > > 1) Patches destined for the CURRENT kernel version should be > against my for-linus branch. That means that right now, any > patches that should to into 4.20 should be against that. > > 2) Patches destined for the NEXT kernel version should be against > my for-x.y/block branch, where x.y is the next version. As of > right now, patches for 4.21 should be against my for-4.21/bloc > branch. > > I'd encourage you to respin against that, particularly in this case > since we've both got a lot of churn, and also removal of various > items that you are patching here. > > -- > Jens Axboe
On 11/12/18 8:45 AM, Paolo Valente wrote: > BTW, since this patch series is probably even more useful for older > than for future kernels, might it make sense to also propose it for > stable/longterm kernels (provided that such a possibility exists)? That just not how things work, we don't put different things in older/stable kernels, it's strictly backports of what we have in current/newer kernels. Hence it appears to be a dead end right now.
On Mon, Nov 12, 2018 at 08:48:35AM -0700, Jens Axboe wrote: > On 11/12/18 8:45 AM, Paolo Valente wrote: > > BTW, since this patch series is probably even more useful for older > > than for future kernels, might it make sense to also propose it for > > stable/longterm kernels (provided that such a possibility exists)? > > That just not how things work, we don't put different things in > older/stable kernels, it's strictly backports of what we have in > current/newer kernels. Hence it appears to be a dead end right now. > It may not be useful currently, but my plans are to do a scheduler agnostic proportional io controller next, so having these interfaces unified would be nice so I don't have to do a rqos.io.weight or something similar. Thanks, Josef
On 11/12/18 8:54 AM, Josef Bacik wrote: > On Mon, Nov 12, 2018 at 08:48:35AM -0700, Jens Axboe wrote: >> On 11/12/18 8:45 AM, Paolo Valente wrote: >>> BTW, since this patch series is probably even more useful for older >>> than for future kernels, might it make sense to also propose it for >>> stable/longterm kernels (provided that such a possibility exists)? >> >> That just not how things work, we don't put different things in >> older/stable kernels, it's strictly backports of what we have in >> current/newer kernels. Hence it appears to be a dead end right now. >> > > It may not be useful currently, but my plans are to do a scheduler agnostic > proportional io controller next, so having these interfaces unified would be > nice so I don't have to do a rqos.io.weight or something similar. Thanks, I'm not saying the work isn't useful, I'm saying that we can't go adding different interfaces to stable kernels than what we currently have in tip. I'm all for unified interfaces for this kind of thing, it's much better than having something that's specific to any given implementation.
Hi Jens, I have rebased the patchset against the for-4.21/block branch, but I can't test them properly because the compiling process has an error on a different file. In particular: include/net/xfrm.h:1465:3 error: unknown type 'spruct' include/net/xfrm.h:1465:30 error: expected ':', ',', ';', '}' or '__attribute__' before 'auth' To be clear, and so that you can check I haven't made some trivial mistakes: I have added/fetched the remote [1] and then simply rebased against the for-4.21/block branch. [1] git://git.kernel.dk/linux-block Angelo 2018-11-12 16:35 GMT+01:00, Jens Axboe <axboe@kernel.dk>: > On 11/12/18 3:17 AM, Paolo Valente wrote: >> >> >>> Il giorno 12 nov 2018, alle ore 11:00, Oleksandr Natalenko >>> <oleksandr@natalenko.name> ha scritto: >>> >>> On 12.11.2018 10:56, Paolo Valente wrote: >>>> Hi Jens, Tejun, all, >>>> about nine months ago, we agreed on a solution for unifying the >>>> interface of the proportional-share policy in blkio/io [1]. Angelo >>>> and I finally completed it. Let me briefly recall the problem and the >>>> solution. >>>> The current implementation of cgroups doesn't allow two or more >>>> entities, e.g., I/O schedulers, to share the same files. So, if CFQ >>>> creates its files for the proportional-share policy, such as, e.g, >>>> weight files for blkio/io groups, BFQ cannot attach somehow to them. >>>> Thus, to enable people to set group weights with BFQ, I resorted to >>>> making BFQ create its own version of these common files, by prepending >>>> a bfq prefix. >>>> Actually, no legacy code uses these different names, or is likely to >>>> do so. Having these two sets of names is simply a source of >>>> confusion, as pointed out also, e.g., by Lennart Poettering (CCed >>>> here), and acknowledged by Tejun [2]. >>>> In [1] we agreed on a solution that solves this problem, by actually >>>> making it possible to share cgroups files. Both writing to and >>>> reading from a shared file trigger the appropriate operation for each >>>> of the entities that share the file. In particular, in case of >>>> reading, >>>> - if all entities produce the same output, the this common output is >>>> shown only once; >>>> - if the outputs differ, then every per-entity output is shown, >>>> preceded by the name of the entity that produced that output. >>>> With this solution, legacy code that, e.g., sets group weights, just >>>> works, regardless of the I/O scheduler actually implementing >>>> proportional share. >>>> But note that this extension is not restricted to only blkio/io. The >>>> general group interface now enables files to be shared among multiple >>>> entities of any kind. >>>> (I have also added a patch to fix some clerical errors in bfq doc, >>>> which I found while making the latter consistent with the new >>>> interface.) >>>> Thanks, >>>> Paolo >>>> [1] https://lkml.org/lkml/2018/1/4/667 >>>> [2] https://github.com/systemd/systemd/issues/7057 >>>> Angelo Ruocco (7): >>>> kernfs: add function to find kernfs_node without increasing ref >>>> counter >>>> cgroup: link cftypes of the same subsystem with the same name >>>> cgroup: add owner name to cftypes >>>> block, bfq: align min and default weights with cfq >>>> cgroup: make all functions of all cftypes be invoked >>>> block, cfq: allow cgroup files to be shared >>>> block, throttle: allow sharing cgroup statistic files >>>> Paolo Valente (5): >>>> cgroup: add hook seq_show_cft with also the owning cftype as parameter >>>> block, cgroup: pass cftype to functions that need to use it >>>> block, bfq: use standard file names for the proportional-share policy >>>> doc, bfq-iosched: fix a few clerical errors >>>> doc, bfq-iosched: make it consistent with the new cgroup interface >>>> Documentation/block/bfq-iosched.txt | 31 +++-- >>>> block/bfq-cgroup.c | 148 +++++++++++++------- >>>> block/bfq-iosched.h | 4 +- >>>> block/blk-cgroup.c | 22 +-- >>>> block/blk-throttle.c | 24 ++-- >>>> block/cfq-iosched.c | 105 +++++++++++---- >>>> fs/kernfs/dir.c | 13 ++ >>>> include/linux/blk-cgroup.h | 10 +- >>>> include/linux/cgroup-defs.h | 14 +- >>>> include/linux/cgroup.h | 13 ++ >>>> include/linux/kernfs.h | 7 + >>>> kernel/cgroup/cgroup.c | 262 >>>> +++++++++++++++++++++++++++++------- >>>> 12 files changed, 483 insertions(+), 170 deletions(-) >>>> -- >>>> 2.16.1 >>> >>> I thought all the legacy stuff including CFS et al. is going to be >>> removed in v4.21 completely… >>> >> >> Thanks for pointing this out. >> >> People with a lower kernel version than the future 4.21 just cannot >> and will not be able to use the proportional share policy on blk-mq >> (with legacy code), because of the name issue highlighted in this >> email. If this patch series gets accepted, a backport will solve the >> problem. In this respect, such a backport might even happen >> 'automatically', as most bfq commit seem to get backported to older, >> stable kernels. >> >> In addition, this extension >> - extends the whole cgroups interface, in a seamless and >> backward-compatible way, to prevent future issues like these; >> - solves a similar issue with throttle (which AFAIK won't go away >> with 4.21). > > There's no way this series can get accepted, since you've made the > mistake of basing it on something that won't apply to the block > tree for 4.21. I've outlined these rules before, but here they are > again: > > 1) Patches destined for the CURRENT kernel version should be > against my for-linus branch. That means that right now, any > patches that should to into 4.20 should be against that. > > 2) Patches destined for the NEXT kernel version should be against > my for-x.y/block branch, where x.y is the next version. As of > right now, patches for 4.21 should be against my for-4.21/bloc > branch. > > I'd encourage you to respin against that, particularly in this case > since we've both got a lot of churn, and also removal of various > items that you are patching here. > > -- > Jens Axboe > >
Hi. On 15.11.2018 16:33, Angelo Ruocco wrote: > I have realized that I didn't clearly explain my actions. I did do a > rebase, but that is not the cause of the problem I reported. I mean, > this is enough to trigger the issue: > > git add remote block [1] > git fetch block > git checkout for-4.21/block > > make > > [1] git://git.kernel.dk/linux-block [1] Checking this: http://git.kernel.dk/cgit/linux-block/tree/include/net/xfrm.h?h=for-4.21/block#n1465 and `git blame`, I don't see any `spruct` there. Is your storage OK? Or, maybe, that was some accidental edit?
On 11/15/18 4:54 AM, Angelo Ruocco wrote: > Hi Jens, > > I have rebased the patchset against the for-4.21/block branch, but I > can't test them properly because the compiling process has an error on > a different file. In particular: > > include/net/xfrm.h:1465:3 error: unknown type 'spruct' > include/net/xfrm.h:1465:30 error: expected ':', ',', ';', '}' or > '__attribute__' before 'auth' > > To be clear, and so that you can check I haven't made some trivial > mistakes: I have added/fetched the remote [1] and then simply rebased > against the for-4.21/block branch. I think you have memory issues, a 'p' is just one bit away from a 't'.
2018-11-15 17:30 GMT+01:00, Jens Axboe <axboe@kernel.dk>: > On 11/15/18 4:54 AM, Angelo Ruocco wrote: >> Hi Jens, >> >> I have rebased the patchset against the for-4.21/block branch, but I >> can't test them properly because the compiling process has an error on >> a different file. In particular: >> >> include/net/xfrm.h:1465:3 error: unknown type 'spruct' >> include/net/xfrm.h:1465:30 error: expected ':', ',', ';', '}' or >> '__attribute__' before 'auth' >> >> To be clear, and so that you can check I haven't made some trivial >> mistakes: I have added/fetched the remote [1] and then simply rebased >> against the for-4.21/block branch. > > I think you have memory issues, a 'p' is just one bit away from a 't'. That was indeed the problem. Thank you, Angelo > > -- > Jens Axboe >
Hi Jens, Tejun, all, about nine months ago, we agreed on a solution for unifying the interface of the proportional-share policy in blkio/io [1]. Angelo and I finally completed it. Let me briefly recall the problem and the solution. The current implementation of cgroups doesn't allow two or more entities, e.g., I/O schedulers, to share the same files. So, if CFQ creates its files for the proportional-share policy, such as, e.g, weight files for blkio/io groups, BFQ cannot attach somehow to them. Thus, to enable people to set group weights with BFQ, I resorted to making BFQ create its own version of these common files, by prepending a bfq prefix. Actually, no legacy code uses these different names, or is likely to do so. Having these two sets of names is simply a source of confusion, as pointed out also, e.g., by Lennart Poettering (CCed here), and acknowledged by Tejun [2]. In [1] we agreed on a solution that solves this problem, by actually making it possible to share cgroups files. Both writing to and reading from a shared file trigger the appropriate operation for each of the entities that share the file. In particular, in case of reading, - if all entities produce the same output, the this common output is shown only once; - if the outputs differ, then every per-entity output is shown, preceded by the name of the entity that produced that output. With this solution, legacy code that, e.g., sets group weights, just works, regardless of the I/O scheduler actually implementing proportional share. But note that this extension is not restricted to only blkio/io. The general group interface now enables files to be shared among multiple entities of any kind. (I have also added a patch to fix some clerical errors in bfq doc, which I found while making the latter consistent with the new interface.) Thanks, Paolo [1] https://lkml.org/lkml/2018/1/4/667 [2] https://github.com/systemd/systemd/issues/7057 Angelo Ruocco (7): kernfs: add function to find kernfs_node without increasing ref counter cgroup: link cftypes of the same subsystem with the same name cgroup: add owner name to cftypes block, bfq: align min and default weights with cfq cgroup: make all functions of all cftypes be invoked block, cfq: allow cgroup files to be shared block, throttle: allow sharing cgroup statistic files Paolo Valente (5): cgroup: add hook seq_show_cft with also the owning cftype as parameter block, cgroup: pass cftype to functions that need to use it block, bfq: use standard file names for the proportional-share policy doc, bfq-iosched: fix a few clerical errors doc, bfq-iosched: make it consistent with the new cgroup interface Documentation/block/bfq-iosched.txt | 31 +++-- block/bfq-cgroup.c | 148 +++++++++++++------- block/bfq-iosched.h | 4 +- block/blk-cgroup.c | 22 +-- block/blk-throttle.c | 24 ++-- block/cfq-iosched.c | 105 +++++++++++---- fs/kernfs/dir.c | 13 ++ include/linux/blk-cgroup.h | 10 +- include/linux/cgroup-defs.h | 14 +- include/linux/cgroup.h | 13 ++ include/linux/kernfs.h | 7 + kernel/cgroup/cgroup.c | 262 +++++++++++++++++++++++++++++------- 12 files changed, 483 insertions(+), 170 deletions(-) -- 2.16.1