[WIP BRANCH] cgroups support in bfq-mq WIP branch

[WIP BRANCH] cgroups support in bfq-mq WIP branch

[Paolo Valente]

Hi,
I've just completed cgroups support, and I'd like to highlight the
main blk-mq issue that I have found along the way.  I have pushed the
commit that completes the support for cgroups to the usual WIP branch
[1].  Before moving to this issue, I have preliminary question about
the scheduler name, since I'm about to start preparing the patch
series for submission.  So far, I have used bfq-mq as a temporary
name.  Are we fine with it, or should I change it, for example, to
just bfq?  Jens?

I've found a sort of circular dependency in blk-mq, related to
scheduler initialization.  To describe both the issue and how I've
addressed it, I'm pasting the message of the new commit.

    This commit completes cgroups support for bfq-mq. In particular, it =
deals with
    a sort of circular dependency introduced in blk-mq: the function
    blkcg_activate_policy, invoked during scheduler initialization, =
triggers the
    invocation of the has_work scheduler hook (before the init function =
is
    finished). To adress this issue, this commit moves the invocation of
    blkcg_activate_policy after the initialization of all the fields =
that could be
    initialized before invoking blkcg_activate_policy itself. This =
enables has_work
    to correctly return false, and thus to prevent the blk-mq stack from =
invoking
    further scheduler hooks before the init function is finished.

Thanks,
Paolo

[1] https://github.com/Algodev-github/bfq-mq=

Re: [WIP BRANCH] cgroups support in bfq-mq WIP branch

[Jens Axboe]

On 02/25/2017 10:44 AM, Paolo Valente wrote:
> Hi,
> I've just completed cgroups support, and I'd like to highlight the
> main blk-mq issue that I have found along the way.  I have pushed the
> commit that completes the support for cgroups to the usual WIP branch
> [1].  Before moving to this issue, I have preliminary question about
> the scheduler name, since I'm about to start preparing the patch
> series for submission.  So far, I have used bfq-mq as a temporary
> name.  Are we fine with it, or should I change it, for example, to
> just bfq?  Jens?

Just call it 'bfq', that doesn't conflict with anything that's
in the kernel already.

> I've found a sort of circular dependency in blk-mq, related to
> scheduler initialization.  To describe both the issue and how I've
> addressed it, I'm pasting the message of the new commit.

Rebase your patches on top of Linus current master, some of them
will need to change and some can be dropped.

And disentangle it completely from the old bfq, I don't want to see
nasty stuff like includes of .c files with prior defines modifying
behavior of functions.

When that's done, get it posted for review asap. I would imagine
we will go through a few postings and review cycles, and if we're
targeting 4.12 with this, then we should get the ball rolling
on that side.

-- 
Jens Axboe

Re: [WIP BRANCH] cgroups support in bfq-mq WIP branch

[Paolo Valente]

> Il giorno 25 feb 2017, alle ore 19:52, Jens Axboe <axboe@kernel.dk> ha =
scritto:
>=20
> On 02/25/2017 10:44 AM, Paolo Valente wrote:
>> Hi,
>> I've just completed cgroups support, and I'd like to highlight the
>> main blk-mq issue that I have found along the way.  I have pushed the
>> commit that completes the support for cgroups to the usual WIP branch
>> [1].  Before moving to this issue, I have preliminary question about
>> the scheduler name, since I'm about to start preparing the patch
>> series for submission.  So far, I have used bfq-mq as a temporary
>> name.  Are we fine with it, or should I change it, for example, to
>> just bfq?  Jens?
>=20
> Just call it 'bfq', that doesn't conflict with anything that's
> in the kernel already.
>=20

ok

>> I've found a sort of circular dependency in blk-mq, related to
>> scheduler initialization.  To describe both the issue and how I've
>> addressed it, I'm pasting the message of the new commit.
>=20
> Rebase your patches on top of Linus current master, some of them
> will need to change and some can be dropped.
>=20

Done, but the last deadlock issue shows up again :( To help you get
context, I'm going to reply to the email in which your sent the patch =
that
solved it.


> And disentangle it completely from the old bfq, I don't want to see
> nasty stuff like includes of .c files with prior defines modifying
> behavior of functions.
>=20

Of course.

> When that's done, get it posted for review asap. I would imagine
> we will go through a few postings and review cycles, and if we're
> targeting 4.12 with this, then we should get the ball rolling
> on that side.
>=20

I was about to to submit, but bumped into the above regression.

Thanks,
Paolo

> --=20
> Jens Axboe
>=20

Re: [WIP BRANCH] cgroups support in bfq-mq WIP branch

[Jens Axboe]

On 03/02/2017 03:15 AM, Paolo Valente wrote:
> 
>> Il giorno 25 feb 2017, alle ore 19:52, Jens Axboe <axboe@kernel.dk> ha scritto:
>>
>> On 02/25/2017 10:44 AM, Paolo Valente wrote:
>>> Hi,
>>> I've just completed cgroups support, and I'd like to highlight the
>>> main blk-mq issue that I have found along the way.  I have pushed the
>>> commit that completes the support for cgroups to the usual WIP branch
>>> [1].  Before moving to this issue, I have preliminary question about
>>> the scheduler name, since I'm about to start preparing the patch
>>> series for submission.  So far, I have used bfq-mq as a temporary
>>> name.  Are we fine with it, or should I change it, for example, to
>>> just bfq?  Jens?
>>
>> Just call it 'bfq', that doesn't conflict with anything that's
>> in the kernel already.
>>
> 
> ok
> 
>>> I've found a sort of circular dependency in blk-mq, related to
>>> scheduler initialization.  To describe both the issue and how I've
>>> addressed it, I'm pasting the message of the new commit.
>>
>> Rebase your patches on top of Linus current master, some of them
>> will need to change and some can be dropped.
>>
> 
> Done, but the last deadlock issue shows up again :( To help you get
> context, I'm going to reply to the email in which your sent the patch that
> solved it.

OK, I got that sent to you. When you have tested it, just add it as
a prep patch in your series. If it works for you, then let me know
and I'll add your Tested-by: to that patch and post it for more
thorough review.

-- 
Jens Axboe