mbox series

[BUGFIX,V2,0/1] block, bfq: deschedule empty bfq_queues not referred by any process

Message ID 20191114093311.47877-1-paolo.valente@linaro.org
Headers show
Series block, bfq: deschedule empty bfq_queues not referred by any process | expand

Message

Paolo Valente Nov. 14, 2019, 9:33 a.m. UTC
Hi Jens,
change from V1: added check to correctly work only on bfq-queues
scheduled for service, and not on in-service bfq-queues (it makes no
sense, and it creates inconsistencies, to deschedule an in-service
bfq-queue).

Differently from V1, which was still under test when I submitted it,
this version has already been tested, by those who reported V1's
failures.

Thanks,
Paolo

Paolo Valente (1):
  block, bfq: deschedule empty bfq_queues not referred by any process

 block/bfq-iosched.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

--
2.20.1

Comments

Jens Axboe Nov. 14, 2019, 2:02 p.m. UTC | #1
On 11/14/19 2:33 AM, Paolo Valente wrote:
> Hi Jens,

> change from V1: added check to correctly work only on bfq-queues

> scheduled for service, and not on in-service bfq-queues (it makes no

> sense, and it creates inconsistencies, to deschedule an in-service

> bfq-queue).

> 

> Differently from V1, which was still under test when I submitted it,

> this version has already been tested, by those who reported V1's

> failures.


I'm a bit miffed that you'd send out a patch for an issue, this late
in the cycle, and then it not being tested at all. That's not very
confidence inspiring. I have applied this one, just letting you know
that that is not acceptable at all.

-- 
Jens Axboe
Paolo Valente Nov. 14, 2019, 3:14 p.m. UTC | #2
> Il giorno 14 nov 2019, alle ore 15:02, Jens Axboe <axboe@kernel.dk> ha scritto:

> 

> On 11/14/19 2:33 AM, Paolo Valente wrote:

>> Hi Jens,

>> change from V1: added check to correctly work only on bfq-queues

>> scheduled for service, and not on in-service bfq-queues (it makes no

>> sense, and it creates inconsistencies, to deschedule an in-service

>> bfq-queue).

>> 

>> Differently from V1, which was still under test when I submitted it,

>> this version has already been tested, by those who reported V1's

>> failures.

> 

> I'm a bit miffed that you'd send out a patch for an issue, this late

> in the cycle, and then it not being tested at all. That's not very

> confidence inspiring. I have applied this one, just letting you know

> that that is not acceptable at all.

> 


I'm sorry for irritating you.  Yet I don't fully get your point.  I
have sent this fix now, simply because this bug was found ten days
ago, and I've tried to fix it as soon as possible.  I did test my
patch before sending it.  As for public testing, how could Oleksandr
or any other user/dev have had a chance to test this patch if I had
not submitted it here?

Thanks,
Paolo

> -- 

> Jens Axboe

>
Jens Axboe Nov. 14, 2019, 3:19 p.m. UTC | #3
On 11/14/19 8:14 AM, Paolo Valente wrote:
> 

> 

>> Il giorno 14 nov 2019, alle ore 15:02, Jens Axboe <axboe@kernel.dk> ha scritto:

>>

>> On 11/14/19 2:33 AM, Paolo Valente wrote:

>>> Hi Jens,

>>> change from V1: added check to correctly work only on bfq-queues

>>> scheduled for service, and not on in-service bfq-queues (it makes no

>>> sense, and it creates inconsistencies, to deschedule an in-service

>>> bfq-queue).

>>>

>>> Differently from V1, which was still under test when I submitted it,

>>> this version has already been tested, by those who reported V1's

>>> failures.

>>

>> I'm a bit miffed that you'd send out a patch for an issue, this late

>> in the cycle, and then it not being tested at all. That's not very

>> confidence inspiring. I have applied this one, just letting you know

>> that that is not acceptable at all.

>>

> 

> I'm sorry for irritating you.  Yet I don't fully get your point.  I

> have sent this fix now, simply because this bug was found ten days

> ago, and I've tried to fix it as soon as possible.  I did test my

> patch before sending it.  As for public testing, how could Oleksandr

> or any other user/dev have had a chance to test this patch if I had

> not submitted it here?


If that's the case, then make it clear that you don't expect it to
be merged right now. As it stands, when you sent it out, all I know is
that it's an issue that's crashing current kernels, and we're winding
down this release. Hence there's a sense of urgency there, as we
could be releasing this kernel as soon as this weekend.

If you have a potential fix, but it isn't tested yet, then make that
clear by submitting it as an RFC. You'd say something like:

"This is a potential fix for X/Y/Z, let's wait for the original reporters
to verify this before including it."

And make that clear with RFC in the subject line. Your patch had none
of that, in fact it said:

[PATCH BUGFIX]

and the commit message had no references to this needing any further
testing.

-- 
Jens Axboe