* Re: [PATCH RFC 4/5] rculist: Remove hlist_for_each_entry_rcu_notrace since no users
From: Miguel Ojeda @ 2019-05-26 16:20 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan,
Linux Doc Mailing List, linuxppc-dev, Mathieu Desnoyers,
Michael Ellerman, Paul E. McKenney, Paul Mackerras, rcu,
Steven Rostedt
In-Reply-To: <20190524234933.5133-5-joel@joelfernandes.org>
On Sat, May 25, 2019 at 1:50 AM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> The series removes all users of the API and with this patch, the API
> itself.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> .clang-format | 1 -
Ack for clang-format, and thanks for removing it there too! :-)
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v12 03/13] dt-bindings: Add doc for the Ingenic TCU drivers
From: Paul Cercueil @ 2019-05-25 19:13 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Jonathan Corbet, Daniel Lezcano, Thomas Gleixner,
Michael Turquette, Stephen Boyd, Jason Cooper, Marc Zyngier,
Lee Jones, Mathieu Malaterre, linux-kernel, devicetree,
linux-mips, linux-doc, linux-clk, od
In-Reply-To: <20190524202103.GA15650@bogus>
Le ven. 24 mai 2019 à 22:21, Rob Herring <robh@kernel.org> a écrit :
> On Tue, May 21, 2019 at 04:51:31PM +0200, Paul Cercueil wrote:
>> Add documentation about how to properly use the Ingenic TCU
>> (Timer/Counter Unit) drivers from devicetree.
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>
>> Notes:
>> v4: New patch in this series. Corresponds to V2 patches 3-4-5
>> with
>> added content.
>>
>> v5: - Edited PWM/watchdog DT bindings documentation to point to
>> the new
>> document.
>> - Moved main document to
>> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>> - Updated documentation to reflect the new devicetree bindings.
>>
>> v6: - Removed PWM/watchdog documentation files as asked by
>> upstream
>> - Removed doc about properties that should be implicit
>> - Removed doc about ingenic,timer-channel /
>> ingenic,clocksource-channel as they are gone
>> - Fix WDT clock name in the binding doc
>> - Fix lengths of register areas in watchdog/pwm nodes
>>
>> v7: No change
>>
>> v8: - Fix address of the PWM node
>> - Added doc about system timer and clocksource children nodes
>>
>> v9: - Remove doc about system timer and clocksource children
>> nodes...
>> - Add doc about ingenic,pwm-channels-mask property
>>
>> v10: No change
>>
>> v11: Fix info about default value of ingenic,pwm-channels-mask
>>
>> v12: Drop sub-nodes for now; they will be introduced in a
>> follow-up
>> patchset.
>
> Why? I believe I acked them.
The patchset was too big, and I've already been trying to get it
upstream for
more than one year now. So I cut it in half in hope that it'll be
easier to
upstream it that way.
>>
>> .../devicetree/bindings/timer/ingenic,tcu.txt | 59
>> +++++++++++++++++++
>> 1 file changed, 59 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>> b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>> new file mode 100644
>> index 000000000000..d101cd72c9b0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>> @@ -0,0 +1,59 @@
>> +Ingenic JZ47xx SoCs Timer/Counter Unit devicetree bindings
>> +==========================================================
>> +
>> +For a description of the TCU hardware and drivers, have a look at
>> +Documentation/mips/ingenic-tcu.txt.
>> +
>> +Required properties:
>> +
>> +- compatible: Must be one of:
>> + * "ingenic,jz4740-tcu"
>> + * "ingenic,jz4725b-tcu"
>> + * "ingenic,jz4770-tcu"
>> +- reg: Should be the offset/length value corresponding to the TCU
>> registers
>> +- clocks: List of phandle & clock specifiers for clocks external
>> to the TCU.
>> + The "pclk", "rtc" and "ext" clocks should be provided. The "tcu"
>> clock
>> + should be provided if the SoC has it.
>> +- clock-names: List of name strings for the external clocks.
>> +- #clock-cells: Should be <1>;
>> + Clock consumers specify this argument to identify a clock. The
>> valid values
>> + may be found in <dt-bindings/clock/ingenic,tcu.h>.
>> +- interrupt-controller : Identifies the node as an interrupt
>> controller
>> +- #interrupt-cells : Specifies the number of cells needed to
>> encode an
>> + interrupt source. The value should be 1.
>> +- interrupt-parent : phandle of the interrupt controller.
>
> Drop this 'interrupt-parent' is implied and could be in a parent node.
>
>> +- interrupts : Specifies the interrupt the controller is connected
>> to.
>> +
>> +Optional properties:
>> +
>> +- ingenic,pwm-channels-mask: Bitmask of TCU channels reserved for
>> PWM use.
>> + Default value is 0xfc.
>> +
>> +
>> +Example
>> +==========================================================
>> +
>> +#include <dt-bindings/clock/jz4770-cgu.h>
>> +
>> +/ {
>> + tcu: timer@10002000 {
>> + compatible = "ingenic,jz4770-tcu";
>> + reg = <0x10002000 0x1000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x0 0x10002000 0x1000>;
>> +
>> + #clock-cells = <1>;
>> +
>> + clocks = <&cgu JZ4770_CLK_RTC
>> + &cgu JZ4770_CLK_EXT
>> + &cgu JZ4770_CLK_PCLK>;
>> + clock-names = "rtc", "ext", "pclk";
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> +
>> + interrupt-parent = <&intc>;
>> + interrupts = <27 26 25>;
>> + };
>> +};
>> --
>> 2.21.0.593.g511ec345e18
>>
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-25 18:18 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190525181407.GA220326@google.com>
On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
[snip]
> > That aside, if we are going to change the name of an API that is
> > used 160 places throughout the tree, we would need to have a pretty
> > good justification. Without such a justification, it will just look
> > like pointless churn to the various developers and maintainers on the
> > receiving end of the patches.
>
> Actually, the API name change is not something I want to do, it is Steven
> suggestion. My suggestion is let us just delete _raw_notrace and just use the
> _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> issue since _raw doesn't do such checking at the moment anyway.. (if possible
> check my cover letter again for details/motivation of this series).
Come to think of it, if we/I succeed in adding lockdep checking in _raw, then
we can just keep the current APIs and not delete anything. And we can have
_raw_notrace skip the lockdep checks. The sparse check question would still
be an open one though, since _raw doesn't do sparse checks at the moment
unlike _raw_notrace as Steve pointed.
Thanks,
- Joel
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-25 18:14 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190525155035.GE28207@linux.ibm.com>
On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > On Sat, 25 May 2019 04:14:44 -0400
> > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > >
> > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > that check?
> > > >
> > > > This is true.
> > > >
> > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > sparse checking version of _raw if need-be. The other option could be to
> > > > always do sparse checking for _raw however that used to be the case and got
> > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > >
> > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> >
> > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > kernel :-/.
> >
> > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> >
> > I wonder what Paul thinks about the matter as well.
>
> My thought is that it is likely that a goodly number of the current uses
> of _raw should really be some form of _check, with lockdep expressions
> spelled out. Not that working out what exactly those lockdep expressions
> should be is necessarily a trivial undertaking. ;-)
Yes, currently where I am a bit stuck is the rcu_dereference_raw()
cannot possibly know what SRCU domain it is under, so lockdep cannot check if
an SRCU lock is held without the user also passing along the SRCU domain. I
am trying to change lockdep to see if it can check if *any* srcu domain lock
is held (regardless of which one) and complain if none are. This is at least
better than no check at all.
However, I think it gets tricky for mutexes. If you have something like:
mutex_lock(some_mutex);
p = rcu_dereference_raw(gp);
mutex_unlock(some_mutex);
This might be a perfectly valid invocation of _raw, however my checks (patch
is still cooking) trigger a lockdep warning becase _raw cannot know that this
is Ok. lockdep thinks it is not in a reader section. This then gets into the
territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
which sucks because its yet another API. To circumvent this issue, can we
just have callers of rcu_dereference_raw ensure that they call
rcu_read_lock() if they are protecting dereferences by a mutex? That would
make things a lot easier and also may be Ok since rcu_read_lock is quite
cheap.
> That aside, if we are going to change the name of an API that is
> used 160 places throughout the tree, we would need to have a pretty
> good justification. Without such a justification, it will just look
> like pointless churn to the various developers and maintainers on the
> receiving end of the patches.
Actually, the API name change is not something I want to do, it is Steven
suggestion. My suggestion is let us just delete _raw_notrace and just use the
_raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
issue since _raw doesn't do such checking at the moment anyway.. (if possible
check my cover letter again for details/motivation of this series).
thanks!
- Joel
> Thanx, Paul
>
> > thanks, Steven!
> >
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Paul E. McKenney @ 2019-05-25 15:50 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190525141954.GA176647@google.com>
On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > On Sat, 25 May 2019 04:14:44 -0400
> > Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > that check?
> > >
> > > This is true.
> > >
> > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > just for sparse checking? The API naming is also confusing. I was expecting
> > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > sparse checking version of _raw if need-be. The other option could be to
> > > always do sparse checking for _raw however that used to be the case and got
> > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> >
> > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
>
> That would also mean changing 160 usages of _raw to _raw_nocheck in the
> kernel :-/.
>
> The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> rcu_check_sparse directly in the calling code for those and eliminate the APIs?
>
> I wonder what Paul thinks about the matter as well.
My thought is that it is likely that a goodly number of the current uses
of _raw should really be some form of _check, with lockdep expressions
spelled out. Not that working out what exactly those lockdep expressions
should be is necessarily a trivial undertaking. ;-)
That aside, if we are going to change the name of an API that is
used 160 places throughout the tree, we would need to have a pretty
good justification. Without such a justification, it will just look
like pointless churn to the various developers and maintainers on the
receiving end of the patches.
Thanx, Paul
> thanks, Steven!
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-25 14:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu
In-Reply-To: <20190525070826.16f76ee7@gandalf.local.home>
On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> On Sat, 25 May 2019 04:14:44 -0400
> Joel Fernandes <joel@joelfernandes.org> wrote:
>
> > > I guess the difference between the _raw_notrace and just _raw variants
> > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > that check?
> >
> > This is true.
> >
> > Since the users of _raw_notrace are very few, is it worth keeping this API
> > just for sparse checking? The API naming is also confusing. I was expecting
> > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > want to nuke _raw_notrace as done in this series and later we can introduce a
> > sparse checking version of _raw if need-be. The other option could be to
> > always do sparse checking for _raw however that used to be the case and got
> > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
>
> What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
That would also mean changing 160 usages of _raw to _raw_nocheck in the
kernel :-/.
The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
rcu_check_sparse directly in the calling code for those and eliminate the APIs?
I wonder what Paul thinks about the matter as well.
thanks, Steven!
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Steven Rostedt @ 2019-05-25 11:08 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu
In-Reply-To: <20190525081444.GC197789@google.com>
On Sat, 25 May 2019 04:14:44 -0400
Joel Fernandes <joel@joelfernandes.org> wrote:
> > I guess the difference between the _raw_notrace and just _raw variants
> > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > that check?
>
> This is true.
>
> Since the users of _raw_notrace are very few, is it worth keeping this API
> just for sparse checking? The API naming is also confusing. I was expecting
> _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> want to nuke _raw_notrace as done in this series and later we can introduce a
> sparse checking version of _raw if need-be. The other option could be to
> always do sparse checking for _raw however that used to be the case and got
> changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
-- Steve
^ permalink raw reply
* Re: [PATCH] doc/rcu: Correct field_count field naming in examples
From: Joel Fernandes @ 2019-05-25 10:07 UTC (permalink / raw)
To: Paul E. McKenney
Cc: LKML, rcu, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
Lai Jiangshan, Jonathan Corbet, open list:DOCUMENTATION
In-Reply-To: <20190508181638.GY3923@linux.ibm.com>
On Wed, May 8, 2019 at 9:16 PM Paul E. McKenney <paulmck@linux.ibm.com> wrote:
[snip]
> > > And this example code predates v2.6.12. ;-)
> > >
> > > So good eyes, but I believe that this really does reflect the ancient
> > > code...
> > >
> > > On the other hand, would you have ideas for more modern replacement
> > > examples?
> >
> > There are 3 cases I can see in listRCU.txt:
> > (1) action taken outside of read_lock (can tolerate stale data), no in-place update.
> > this is the best possible usage of RCU.
> > (2) action taken outside of read_lock, in-place updates
> > this is good as long as not too many in-place updates.
> > involves copying creating new list node and replacing the
> > node being updated with it.
> > (3) cannot tolerate stale data: here a deleted or obsolete flag can be used
> > protected by a per-entry lock. reader
> > aborts if object is stale.
> >
> > Any replacement example must make satisfy (3) too?
>
> It would be OK to have a separate example for (3). It would of course
> be nicer to have one example for all three, but not all -that- important.
>
> > The only example for (3) that I know of is sysvipc sempahores which you also
> > mentioned in the paper. Looking through this code, it hasn't changed
> > conceptually and it could be a fit for an example (ipc_valid_object() checks
> > for whether the object is stale).
>
> That is indeed the classic canonical example. ;-)
FWIW just want to mention, it seems to me the ptrace task list
traversal could be a great example of "mark obsolete objects" and is
simple so I could just use that.
Neil talks about it in his article here:
https://lwn.net/Articles/610972/ . In "Group 3: Transform the way the
list is walked"
Cheers,
- Joel
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-25 8:14 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu
In-Reply-To: <20190524232458.4bcf4eb4@gandalf.local.home>
On Fri, May 24, 2019 at 11:24:58PM -0400, Steven Rostedt wrote:
> On Fri, 24 May 2019 19:49:28 -0400
> "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote:
>
> > The series removes users of the following APIs, and the APIs themselves, since
> > the regular non - _notrace variants don't do any tracing anyway.
> > * hlist_for_each_entry_rcu_notrace
> > * rcu_dereference_raw_notrace
> >
>
> I guess the difference between the _raw_notrace and just _raw variants
> is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> that check?
This is true.
Since the users of _raw_notrace are very few, is it worth keeping this API
just for sparse checking? The API naming is also confusing. I was expecting
_raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
want to nuke _raw_notrace as done in this series and later we can introduce a
sparse checking version of _raw if need-be. The other option could be to
always do sparse checking for _raw however that used to be the case and got
changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
thanks a lot,
- Joel
>
> -- Steve
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Steven Rostedt @ 2019-05-25 3:24 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
On Fri, 24 May 2019 19:49:28 -0400
"Joel Fernandes (Google)" <joel@joelfernandes.org> wrote:
> The series removes users of the following APIs, and the APIs themselves, since
> the regular non - _notrace variants don't do any tracing anyway.
> * hlist_for_each_entry_rcu_notrace
> * rcu_dereference_raw_notrace
>
I guess the difference between the _raw_notrace and just _raw variants
is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
that check?
-- Steve
^ permalink raw reply
* [PATCH RFC 5/5] rcu: Remove rcu_dereference_raw_notrace since no users
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
The series removes all users of the API and with this patch, the API
itself. Also fix documentation.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
Documentation/RCU/Design/Requirements/Requirements.html | 6 +++---
include/linux/rcupdate.h | 9 ---------
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
index 5a9238a2883c..9727278893e6 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.html
+++ b/Documentation/RCU/Design/Requirements/Requirements.html
@@ -2512,9 +2512,9 @@ disabled across the entire RCU read-side critical section.
<p>
It is possible to use tracing on RCU code, but tracing itself
uses RCU.
-For this reason, <tt>rcu_dereference_raw_notrace()</tt>
-is provided for use by tracing, which avoids the destructive
-recursion that could otherwise ensue.
+This is the other reason for using, <tt>rcu_dereference_raw()</tt>,
+for use by tracing, which avoids the destructive recursion that could
+otherwise ensue.
This API is also used by virtualization in some architectures,
where RCU readers execute in environments in which tracing
cannot be used.
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 922bb6848813..f917a27fc115 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -472,15 +472,6 @@ static inline void rcu_preempt_sleep_check(void) { }
__rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
__rcu)
-/*
- * The tracing infrastructure traces RCU (we want that), but unfortunately
- * some of the RCU checks causes tracing to lock up the system.
- *
- * The no-tracing version of rcu_dereference_raw() must not call
- * rcu_read_lock_held().
- */
-#define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
-
/**
* rcu_dereference_protected() - fetch RCU pointer when updates prevented
* @p: The pointer to read, prior to dereferencing
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH RFC 3/5] hashtable: Use the regular hlist_for_each_entry_rcu API
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
hlist_for_each_entry_rcu already does not do any tracing. This series
removes the notrace variant of it, so let us just use the regular API.
In a future patch, we can also remove the
hash_for_each_possible_rcu_notrace API that this patch touches.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
include/linux/hashtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h
index 417d2c4bc60d..47fa7b673c1b 100644
--- a/include/linux/hashtable.h
+++ b/include/linux/hashtable.h
@@ -189,7 +189,7 @@ static inline void hash_del_rcu(struct hlist_node *node)
* not do any RCU debugging or tracing.
*/
#define hash_for_each_possible_rcu_notrace(name, obj, member, key) \
- hlist_for_each_entry_rcu_notrace(obj, \
+ hlist_for_each_entry_rcu(obj, \
&name[hash_min(key, HASH_BITS(name))], member)
/**
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH RFC 1/5] powerpc: Use regular rcu_dereference_raw API
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
rcu_dereference_raw already does not do any tracing. There is no need to
use the _notrace variant of it and this series removes that API, so let us
use the regular variant here.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 21b1ed5df888..c15c9bbf0206 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -546,7 +546,7 @@ static inline void note_hpte_modification(struct kvm *kvm,
*/
static inline struct kvm_memslots *kvm_memslots_raw(struct kvm *kvm)
{
- return rcu_dereference_raw_notrace(kvm->memslots[0]);
+ return rcu_dereference_raw(kvm->memslots[0]);
}
extern void kvmppc_mmu_debugfs_init(struct kvm *kvm);
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH RFC 4/5] rculist: Remove hlist_for_each_entry_rcu_notrace since no users
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
The series removes all users of the API and with this patch, the API
itself.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
.clang-format | 1 -
include/linux/rculist.h | 20 --------------------
2 files changed, 21 deletions(-)
diff --git a/.clang-format b/.clang-format
index 2ffd69afc1a8..aa935923f5cb 100644
--- a/.clang-format
+++ b/.clang-format
@@ -287,7 +287,6 @@ ForEachMacros:
- 'hlist_for_each_entry_from_rcu'
- 'hlist_for_each_entry_rcu'
- 'hlist_for_each_entry_rcu_bh'
- - 'hlist_for_each_entry_rcu_notrace'
- 'hlist_for_each_entry_safe'
- '__hlist_for_each_rcu'
- 'hlist_for_each_safe'
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index e91ec9ddcd30..0d3d77cf4f07 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -628,26 +628,6 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
&(pos)->member)), typeof(*(pos)), member))
-/**
- * hlist_for_each_entry_rcu_notrace - iterate over rcu list of given type (for tracing)
- * @pos: the type * to use as a loop cursor.
- * @head: the head for your list.
- * @member: the name of the hlist_node within the struct.
- *
- * This list-traversal primitive may safely run concurrently with
- * the _rcu list-mutation primitives such as hlist_add_head_rcu()
- * as long as the traversal is guarded by rcu_read_lock().
- *
- * This is the same as hlist_for_each_entry_rcu() except that it does
- * not do any RCU debugging or tracing.
- */
-#define hlist_for_each_entry_rcu_notrace(pos, head, member) \
- for (pos = hlist_entry_safe (rcu_dereference_raw_notrace(hlist_first_rcu(head)),\
- typeof(*(pos)), member); \
- pos; \
- pos = hlist_entry_safe(rcu_dereference_raw_notrace(hlist_next_rcu(\
- &(pos)->member)), typeof(*(pos)), member))
-
/**
* hlist_for_each_entry_rcu_bh - iterate over rcu list of given type
* @pos: the type * to use as a loop cursor.
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH RFC 2/5] trace: Use regular rcu_dereference_raw API
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
In-Reply-To: <20190524234933.5133-1-joel@joelfernandes.org>
rcu_dereference_raw already does not do any tracing. There is no need to
use the _notrace variant of it and this series removes that API, so let us
use the regular variant here.
While at it, also replace the only user of
hlist_for_each_entry_rcu_notrace (which indirectly uses the
rcu_dereference_raw_notrace API) with hlist_for_each_entry_rcu which
also does not do any tracing.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/trace/ftrace.c | 4 ++--
kernel/trace/ftrace_internal.h | 8 ++++----
kernel/trace/trace.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index b920358dd8f7..f7d5f0ee69de 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -706,7 +706,7 @@ ftrace_find_profiled_func(struct ftrace_profile_stat *stat, unsigned long ip)
if (hlist_empty(hhd))
return NULL;
- hlist_for_each_entry_rcu_notrace(rec, hhd, node) {
+ hlist_for_each_entry_rcu(rec, hhd, node) {
if (rec->ip == ip)
return rec;
}
@@ -1135,7 +1135,7 @@ __ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
key = ftrace_hash_key(hash, ip);
hhd = &hash->buckets[key];
- hlist_for_each_entry_rcu_notrace(entry, hhd, hlist) {
+ hlist_for_each_entry_rcu(entry, hhd, hlist) {
if (entry->ip == ip)
return entry;
}
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 0515a2096f90..e3530a284f46 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -6,22 +6,22 @@
/*
* Traverse the ftrace_global_list, invoking all entries. The reason that we
- * can use rcu_dereference_raw_notrace() is that elements removed from this list
+ * can use rcu_dereference_raw() is that elements removed from this list
* are simply leaked, so there is no need to interact with a grace-period
- * mechanism. The rcu_dereference_raw_notrace() calls are needed to handle
+ * mechanism. The rcu_dereference_raw() calls are needed to handle
* concurrent insertions into the ftrace_global_list.
*
* Silly Alpha and silly pointer-speculation compiler optimizations!
*/
#define do_for_each_ftrace_op(op, list) \
- op = rcu_dereference_raw_notrace(list); \
+ op = rcu_dereference_raw(list); \
do
/*
* Optimized for just a single item in the list (as that is the normal case).
*/
#define while_for_each_ftrace_op(op) \
- while (likely(op = rcu_dereference_raw_notrace((op)->next)) && \
+ while (likely(op = rcu_dereference_raw((op)->next)) && \
unlikely((op) != &ftrace_list_end))
extern struct ftrace_ops __rcu *ftrace_ops_list;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ec439999f387..cb8d696d9cde 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2638,10 +2638,10 @@ static void ftrace_exports(struct ring_buffer_event *event)
preempt_disable_notrace();
- export = rcu_dereference_raw_notrace(ftrace_exports_list);
+ export = rcu_dereference_raw(ftrace_exports_list);
while (export) {
trace_process_export(export, event);
- export = rcu_dereference_raw_notrace(export->next);
+ export = rcu_dereference_raw(export->next);
}
preempt_enable_notrace();
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes (Google) @ 2019-05-24 23:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul E. McKenney, Paul Mackerras, rcu, Steven Rostedt
The series removes users of the following APIs, and the APIs themselves, since
the regular non - _notrace variants don't do any tracing anyway.
* hlist_for_each_entry_rcu_notrace
* rcu_dereference_raw_notrace
Joel Fernandes (Google) (5):
powerpc: Use regular rcu_dereference_raw API
trace: Use regular rcu_dereference_raw API
hashtable: Use the regular hlist_for_each_entry_rcu API
rculist: Remove hlist_for_each_entry_rcu_notrace since no users
rcu: Remove rcu_dereference_raw_notrace since no users
.clang-format | 1 -
.../RCU/Design/Requirements/Requirements.html | 6 +++---
arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
include/linux/hashtable.h | 2 +-
include/linux/rculist.h | 20 -------------------
include/linux/rcupdate.h | 9 ---------
kernel/trace/ftrace.c | 4 ++--
kernel/trace/ftrace_internal.h | 8 ++++----
kernel/trace/trace.c | 4 ++--
9 files changed, 13 insertions(+), 43 deletions(-)
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Peter Oskolkov @ 2019-05-24 22:07 UTC (permalink / raw)
To: Dave Chiluk
Cc: Phil Auld, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAC=E7cXVrGRKMNkJPhd4fJi7wgdYk=YcXPV7B8GVNL5M69BarQ@mail.gmail.com>
On Fri, May 24, 2019 at 2:35 PM Dave Chiluk <chiluk+linux@indeed.com> wrote:
>
> On Fri, May 24, 2019 at 11:28 AM Peter Oskolkov <posk@posk.io> wrote:
> >
> > On Fri, May 24, 2019 at 8:15 AM Dave Chiluk <chiluk+linux@indeed.com> wrote:
> > >
> > > On Fri, May 24, 2019 at 9:32 AM Phil Auld <pauld@redhat.com> wrote:
> > > > On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
> > >
> > > > > If the machine runs at/close to capacity, won't the overallocation
> > > > > of the quota to bursty tasks necessarily negatively impact every other
> > > > > task? Should the "unused" quota be available only on idle CPUs?
> > > > > (Or maybe this is the behavior achieved here, and only the comment and
> > > > > the commit message should be fixed...)
> > > > >
> > > >
> > > > It's bounded by the amount left unused from the previous period. So
> > > > theoretically a process could use almost twice its quota. But then it
> > > > would have nothing left over in the next period. To repeat it would have
> > > > to not use any that next period. Over a longer number of periods it's the
> > > > same amount of CPU usage.
> > > >
> > > > I think that is more fair than throttling a process that has never used
> > > > its full quota.
> > > >
> > > > And it removes complexity.
> > > >
> > > > Cheers,
> > > > Phil
> > >
> > > Actually it's not even that bad. The overallocation of quota to a
> > > bursty task in a period is limited to at most one slice per cpu, and
> > > that slice must not have been used in the previous periods. The slice
> > > size is set with /proc/sys/kernel/sched_cfs_bandwidth_slice_us and
> > > defaults to 5ms. If a bursty task goes from underutilizing quota to
> > > using it's entire quota, it will not be able to burst in the
> > > subsequent periods. Therefore in an absolute worst case contrived
> > > scenario, a bursty task can add at most 5ms to the latency of other
> > > threads on the same CPU. I think this worst case 5ms tradeoff is
> > > entirely worth it.
> > >
> > > This does mean that a theoretically a poorly written massively
> > > threaded application on an 80 core box, that spreads itself onto 80
> > > cpu run queues, can overutilize it's quota in a period by at most 5ms
> > > * 80 CPUs in a sincle period (slice * number of runqueues the
> > > application is running on). But that means that each of those threads
> > > would have had to not be use their quota in a previous period, and it
> > > also means that the application would have to be carefully written to
> > > exacerbate this behavior.
> > >
> > > Additionally if cpu bound threads underutilize a slice of their quota
> > > in a period due to the cfs choosing a bursty task to run, they should
> > > theoretically be able to make it up in the following periods when the
> > > bursty task is unable to "burst".
> >
> > OK, so it is indeed possible that CPU bound threads will underutilize a slice
> > of their quota in a period as a result of this patch. This should probably
> > be clearly stated in the code comments and in the commit message.
> >
> > In addition, I believe that although many workloads will indeed be
> > indifferent to getting their fair share "later", some latency-sensitive
> > workloads will definitely be negatively affected by this temporary
> > CPU quota stealing by bursty antagonists. So there should probably be
> > a way to limit this behavior; for example, by making it tunable
> > per cgroup.
> >
> This patch restores the behavior that existed from at least
> v3.16..v4.18, and the current Redhat 7 kernels. So you are kind of
> championing a moot point as no one has noticed this "bursting"
> behavior in over 5 years. By removing this slice expiration
> altogether we restore the behavior and also solve the root issue of
> 'commit 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift
> condition")'.
>
> Since 512ac999d275, many people are now noticing the slice expiration
> and very displeased with the behavior change.
> see: https://github.com/kubernetes/kubernetes/issues/67577
>
> I would love to hear if you know of a single complaint during that 5
> year time window, where someone noticed this bursting and reported
> that it negatively affected their application.
Linux CPU scheduling tail latency is a well-known issue and a major
pain point in some workloads:
https://www.google.com/search?q=linux+cpu+scheduling+tail+latency
Even assuming that nobody noticed this particular cause
of CPU scheduling latencies, it does not mean the problem should be waved
away. At least it should be documented, if at this point it decided that
it is difficult to address it in a meaningful way. And, preferably, a way
to address the issue later on should be discussed and hopefully agreed to.
>
> As for the documentation, I thought about documenting the possible
> adverse side-effect, but I didn't feel it was worthwhile since no one
> had noticed that corner case in the 5 years. I also could not figure
> out a concise way of describing the slight corner case issue without
> overly complicating the documentation. I felt that adding that corner
> case would have detracted rather than added to the documentation's
> usefulness. As far as commenting in code, considering most of this
> commit removes lines, there's not a really great place for it. That's
> why I did my best to describe the behavior in the documentation.
> Smart people can draw conclusions from there as you have done. Please
> keep in mind that this "bursting" is again limited to a single slice
> on each per-cpu run-queue.
>
> Thank you,
> Dave
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Dave Chiluk @ 2019-05-24 21:35 UTC (permalink / raw)
To: Peter Oskolkov
Cc: Phil Auld, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAFTs51Vm258CkDXi_Jj_cGOMotTvhdYR_VW8aUwAUvgistZOFQ@mail.gmail.com>
On Fri, May 24, 2019 at 11:28 AM Peter Oskolkov <posk@posk.io> wrote:
>
> On Fri, May 24, 2019 at 8:15 AM Dave Chiluk <chiluk+linux@indeed.com> wrote:
> >
> > On Fri, May 24, 2019 at 9:32 AM Phil Auld <pauld@redhat.com> wrote:
> > > On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
> >
> > > > If the machine runs at/close to capacity, won't the overallocation
> > > > of the quota to bursty tasks necessarily negatively impact every other
> > > > task? Should the "unused" quota be available only on idle CPUs?
> > > > (Or maybe this is the behavior achieved here, and only the comment and
> > > > the commit message should be fixed...)
> > > >
> > >
> > > It's bounded by the amount left unused from the previous period. So
> > > theoretically a process could use almost twice its quota. But then it
> > > would have nothing left over in the next period. To repeat it would have
> > > to not use any that next period. Over a longer number of periods it's the
> > > same amount of CPU usage.
> > >
> > > I think that is more fair than throttling a process that has never used
> > > its full quota.
> > >
> > > And it removes complexity.
> > >
> > > Cheers,
> > > Phil
> >
> > Actually it's not even that bad. The overallocation of quota to a
> > bursty task in a period is limited to at most one slice per cpu, and
> > that slice must not have been used in the previous periods. The slice
> > size is set with /proc/sys/kernel/sched_cfs_bandwidth_slice_us and
> > defaults to 5ms. If a bursty task goes from underutilizing quota to
> > using it's entire quota, it will not be able to burst in the
> > subsequent periods. Therefore in an absolute worst case contrived
> > scenario, a bursty task can add at most 5ms to the latency of other
> > threads on the same CPU. I think this worst case 5ms tradeoff is
> > entirely worth it.
> >
> > This does mean that a theoretically a poorly written massively
> > threaded application on an 80 core box, that spreads itself onto 80
> > cpu run queues, can overutilize it's quota in a period by at most 5ms
> > * 80 CPUs in a sincle period (slice * number of runqueues the
> > application is running on). But that means that each of those threads
> > would have had to not be use their quota in a previous period, and it
> > also means that the application would have to be carefully written to
> > exacerbate this behavior.
> >
> > Additionally if cpu bound threads underutilize a slice of their quota
> > in a period due to the cfs choosing a bursty task to run, they should
> > theoretically be able to make it up in the following periods when the
> > bursty task is unable to "burst".
>
> OK, so it is indeed possible that CPU bound threads will underutilize a slice
> of their quota in a period as a result of this patch. This should probably
> be clearly stated in the code comments and in the commit message.
>
> In addition, I believe that although many workloads will indeed be
> indifferent to getting their fair share "later", some latency-sensitive
> workloads will definitely be negatively affected by this temporary
> CPU quota stealing by bursty antagonists. So there should probably be
> a way to limit this behavior; for example, by making it tunable
> per cgroup.
>
This patch restores the behavior that existed from at least
v3.16..v4.18, and the current Redhat 7 kernels. So you are kind of
championing a moot point as no one has noticed this "bursting"
behavior in over 5 years. By removing this slice expiration
altogether we restore the behavior and also solve the root issue of
'commit 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift
condition")'.
Since 512ac999d275, many people are now noticing the slice expiration
and very displeased with the behavior change.
see: https://github.com/kubernetes/kubernetes/issues/67577
I would love to hear if you know of a single complaint during that 5
year time window, where someone noticed this bursting and reported
that it negatively affected their application.
As for the documentation, I thought about documenting the possible
adverse side-effect, but I didn't feel it was worthwhile since no one
had noticed that corner case in the 5 years. I also could not figure
out a concise way of describing the slight corner case issue without
overly complicating the documentation. I felt that adding that corner
case would have detracted rather than added to the documentation's
usefulness. As far as commenting in code, considering most of this
commit removes lines, there's not a really great place for it. That's
why I did my best to describe the behavior in the documentation.
Smart people can draw conclusions from there as you have done. Please
keep in mind that this "bursting" is again limited to a single slice
on each per-cpu run-queue.
Thank you,
Dave
^ permalink raw reply
* Re: [PATCH v12 03/13] dt-bindings: Add doc for the Ingenic TCU drivers
From: Rob Herring @ 2019-05-24 20:21 UTC (permalink / raw)
To: Paul Cercueil
Cc: Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Jonathan Corbet, Daniel Lezcano, Thomas Gleixner,
Michael Turquette, Stephen Boyd, Jason Cooper, Marc Zyngier,
Lee Jones, Mathieu Malaterre, linux-kernel, devicetree,
linux-mips, linux-doc, linux-clk, od
In-Reply-To: <20190521145141.9813-4-paul@crapouillou.net>
On Tue, May 21, 2019 at 04:51:31PM +0200, Paul Cercueil wrote:
> Add documentation about how to properly use the Ingenic TCU
> (Timer/Counter Unit) drivers from devicetree.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>
> Notes:
> v4: New patch in this series. Corresponds to V2 patches 3-4-5 with
> added content.
>
> v5: - Edited PWM/watchdog DT bindings documentation to point to the new
> document.
> - Moved main document to
> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
> - Updated documentation to reflect the new devicetree bindings.
>
> v6: - Removed PWM/watchdog documentation files as asked by upstream
> - Removed doc about properties that should be implicit
> - Removed doc about ingenic,timer-channel /
> ingenic,clocksource-channel as they are gone
> - Fix WDT clock name in the binding doc
> - Fix lengths of register areas in watchdog/pwm nodes
>
> v7: No change
>
> v8: - Fix address of the PWM node
> - Added doc about system timer and clocksource children nodes
>
> v9: - Remove doc about system timer and clocksource children
> nodes...
> - Add doc about ingenic,pwm-channels-mask property
>
> v10: No change
>
> v11: Fix info about default value of ingenic,pwm-channels-mask
>
> v12: Drop sub-nodes for now; they will be introduced in a follow-up
> patchset.
Why? I believe I acked them.
>
> .../devicetree/bindings/timer/ingenic,tcu.txt | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/ingenic,tcu.txt b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
> new file mode 100644
> index 000000000000..d101cd72c9b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
> @@ -0,0 +1,59 @@
> +Ingenic JZ47xx SoCs Timer/Counter Unit devicetree bindings
> +==========================================================
> +
> +For a description of the TCU hardware and drivers, have a look at
> +Documentation/mips/ingenic-tcu.txt.
> +
> +Required properties:
> +
> +- compatible: Must be one of:
> + * "ingenic,jz4740-tcu"
> + * "ingenic,jz4725b-tcu"
> + * "ingenic,jz4770-tcu"
> +- reg: Should be the offset/length value corresponding to the TCU registers
> +- clocks: List of phandle & clock specifiers for clocks external to the TCU.
> + The "pclk", "rtc" and "ext" clocks should be provided. The "tcu" clock
> + should be provided if the SoC has it.
> +- clock-names: List of name strings for the external clocks.
> +- #clock-cells: Should be <1>;
> + Clock consumers specify this argument to identify a clock. The valid values
> + may be found in <dt-bindings/clock/ingenic,tcu.h>.
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The value should be 1.
> +- interrupt-parent : phandle of the interrupt controller.
Drop this 'interrupt-parent' is implied and could be in a parent node.
> +- interrupts : Specifies the interrupt the controller is connected to.
> +
> +Optional properties:
> +
> +- ingenic,pwm-channels-mask: Bitmask of TCU channels reserved for PWM use.
> + Default value is 0xfc.
> +
> +
> +Example
> +==========================================================
> +
> +#include <dt-bindings/clock/jz4770-cgu.h>
> +
> +/ {
> + tcu: timer@10002000 {
> + compatible = "ingenic,jz4770-tcu";
> + reg = <0x10002000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x10002000 0x1000>;
> +
> + #clock-cells = <1>;
> +
> + clocks = <&cgu JZ4770_CLK_RTC
> + &cgu JZ4770_CLK_EXT
> + &cgu JZ4770_CLK_PCLK>;
> + clock-names = "rtc", "ext", "pclk";
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interrupt-parent = <&intc>;
> + interrupts = <27 26 25>;
> + };
> +};
> --
> 2.21.0.593.g511ec345e18
>
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Peter Oskolkov @ 2019-05-24 16:28 UTC (permalink / raw)
To: Dave Chiluk
Cc: Phil Auld, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAC=E7cXxsyMLw1PR+8QchTH8FYL7WX6_8LBVdqueR1yjW+VVkQ@mail.gmail.com>
On Fri, May 24, 2019 at 8:15 AM Dave Chiluk <chiluk+linux@indeed.com> wrote:
>
> On Fri, May 24, 2019 at 9:32 AM Phil Auld <pauld@redhat.com> wrote:
> > On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
>
> > > If the machine runs at/close to capacity, won't the overallocation
> > > of the quota to bursty tasks necessarily negatively impact every other
> > > task? Should the "unused" quota be available only on idle CPUs?
> > > (Or maybe this is the behavior achieved here, and only the comment and
> > > the commit message should be fixed...)
> > >
> >
> > It's bounded by the amount left unused from the previous period. So
> > theoretically a process could use almost twice its quota. But then it
> > would have nothing left over in the next period. To repeat it would have
> > to not use any that next period. Over a longer number of periods it's the
> > same amount of CPU usage.
> >
> > I think that is more fair than throttling a process that has never used
> > its full quota.
> >
> > And it removes complexity.
> >
> > Cheers,
> > Phil
>
> Actually it's not even that bad. The overallocation of quota to a
> bursty task in a period is limited to at most one slice per cpu, and
> that slice must not have been used in the previous periods. The slice
> size is set with /proc/sys/kernel/sched_cfs_bandwidth_slice_us and
> defaults to 5ms. If a bursty task goes from underutilizing quota to
> using it's entire quota, it will not be able to burst in the
> subsequent periods. Therefore in an absolute worst case contrived
> scenario, a bursty task can add at most 5ms to the latency of other
> threads on the same CPU. I think this worst case 5ms tradeoff is
> entirely worth it.
>
> This does mean that a theoretically a poorly written massively
> threaded application on an 80 core box, that spreads itself onto 80
> cpu run queues, can overutilize it's quota in a period by at most 5ms
> * 80 CPUs in a sincle period (slice * number of runqueues the
> application is running on). But that means that each of those threads
> would have had to not be use their quota in a previous period, and it
> also means that the application would have to be carefully written to
> exacerbate this behavior.
>
> Additionally if cpu bound threads underutilize a slice of their quota
> in a period due to the cfs choosing a bursty task to run, they should
> theoretically be able to make it up in the following periods when the
> bursty task is unable to "burst".
OK, so it is indeed possible that CPU bound threads will underutilize a slice
of their quota in a period as a result of this patch. This should probably
be clearly stated in the code comments and in the commit message.
In addition, I believe that although many workloads will indeed be
indifferent to getting their fair share "later", some latency-sensitive
workloads will definitely be negatively affected by this temporary
CPU quota stealing by bursty antagonists. So there should probably be
a way to limit this behavior; for example, by making it tunable
per cgroup.
>
> Please be careful here quota and slice are being treated differently.
> Quota does not roll-over between periods, only slices of quota that
> has already been allocated to per cpu run queues. If you allocate
> 100ms of quota per period to an application, but it only spreads onto
> 3 cpu run queues that means it can in the worst case use 3 x slice
> size = 15ms in periods following underutilization.
>
> So why does this matter. Well applications that use thread pools
> *(*cough* java *cough*) with lots of tiny little worker threads, tend
> to spread themselves out onto a lot of run queues. These worker
> threads grab quota slices in order to run, then rarely use all of
> their slice (1 or 2ms out of the 5ms). This results in those worker
> threads starving the main application of quota, and then expiring the
> remainder of that quota slice on the per-cpu. Going back to my
> earlier 100ms quota / 80 cpu example. That means only
> 100ms/cfs_bandwidth_slice_us(5ms) = 20 slices are available in a
> period. So only 20 out of these 80 cpus ever get a slice allocated to
> them. By allowing these per-cpu run queues to use their remaining
> slice in following periods these worker threads do not need to be
> allocated additional slice, and thereby the main threads are actually
> able to use the allocated cpu quota.
>
> This can be experienced by running fibtest available at
> https://github.com/indeedeng/fibtest/.
> $ runfibtest 1
> runs a single fast thread taskset to cpu 0
> $ runfibtest 8
> Runs a single fast thread taskset to cpu 0, and 7 slow threads taskset
> to cpus 1-7. This run is expected to show less iterations, but the
> worse problem is that the cpu usage is far less than the 500ms that it
> should have received.
>
> Thanks for the engagement on this,
> Dave Chiluk
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Phil Auld @ 2019-05-24 15:59 UTC (permalink / raw)
To: Dave Chiluk
Cc: Peter Oskolkov, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAC=E7cXxsyMLw1PR+8QchTH8FYL7WX6_8LBVdqueR1yjW+VVkQ@mail.gmail.com>
On Fri, May 24, 2019 at 10:14:36AM -0500 Dave Chiluk wrote:
> On Fri, May 24, 2019 at 9:32 AM Phil Auld <pauld@redhat.com> wrote:
> > On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
>
> > > If the machine runs at/close to capacity, won't the overallocation
> > > of the quota to bursty tasks necessarily negatively impact every other
> > > task? Should the "unused" quota be available only on idle CPUs?
> > > (Or maybe this is the behavior achieved here, and only the comment and
> > > the commit message should be fixed...)
> > >
> >
> > It's bounded by the amount left unused from the previous period. So
> > theoretically a process could use almost twice its quota. But then it
> > would have nothing left over in the next period. To repeat it would have
> > to not use any that next period. Over a longer number of periods it's the
> > same amount of CPU usage.
> >
> > I think that is more fair than throttling a process that has never used
> > its full quota.
> >
> > And it removes complexity.
> >
> > Cheers,
> > Phil
>
> Actually it's not even that bad. The overallocation of quota to a
> bursty task in a period is limited to at most one slice per cpu, and
> that slice must not have been used in the previous periods. The slice
> size is set with /proc/sys/kernel/sched_cfs_bandwidth_slice_us and
> defaults to 5ms. If a bursty task goes from underutilizing quota to
> using it's entire quota, it will not be able to burst in the
> subsequent periods. Therefore in an absolute worst case contrived
> scenario, a bursty task can add at most 5ms to the latency of other
> threads on the same CPU. I think this worst case 5ms tradeoff is
> entirely worth it.
>
> This does mean that a theoretically a poorly written massively
> threaded application on an 80 core box, that spreads itself onto 80
> cpu run queues, can overutilize it's quota in a period by at most 5ms
> * 80 CPUs in a sincle period (slice * number of runqueues the
> application is running on). But that means that each of those threads
> would have had to not be use their quota in a previous period, and it
> also means that the application would have to be carefully written to
> exacerbate this behavior.
>
> Additionally if cpu bound threads underutilize a slice of their quota
> in a period due to the cfs choosing a bursty task to run, they should
> theoretically be able to make it up in the following periods when the
> bursty task is unable to "burst".
>
> Please be careful here quota and slice are being treated differently.
> Quota does not roll-over between periods, only slices of quota that
> has already been allocated to per cpu run queues. If you allocate
> 100ms of quota per period to an application, but it only spreads onto
> 3 cpu run queues that means it can in the worst case use 3 x slice
> size = 15ms in periods following underutilization.
>
> So why does this matter. Well applications that use thread pools
> *(*cough* java *cough*) with lots of tiny little worker threads, tend
> to spread themselves out onto a lot of run queues. These worker
> threads grab quota slices in order to run, then rarely use all of
> their slice (1 or 2ms out of the 5ms). This results in those worker
> threads starving the main application of quota, and then expiring the
> remainder of that quota slice on the per-cpu. Going back to my
> earlier 100ms quota / 80 cpu example. That means only
> 100ms/cfs_bandwidth_slice_us(5ms) = 20 slices are available in a
> period. So only 20 out of these 80 cpus ever get a slice allocated to
> them. By allowing these per-cpu run queues to use their remaining
> slice in following periods these worker threads do not need to be
> allocated additional slice, and thereby the main threads are actually
> able to use the allocated cpu quota.
>
> This can be experienced by running fibtest available at
> https://github.com/indeedeng/fibtest/.
> $ runfibtest 1
> runs a single fast thread taskset to cpu 0
> $ runfibtest 8
> Runs a single fast thread taskset to cpu 0, and 7 slow threads taskset
> to cpus 1-7. This run is expected to show less iterations, but the
> worse problem is that the cpu usage is far less than the 500ms that it
> should have received.
>
> Thanks for the engagement on this,
> Dave Chiluk
Thanks for the clarification. This is an even better explanation.
Fwiw, I ran some of my cfs throttling tests with this, none of which are
designed to measure or hit this particular issue. They are more focused
on starvation and hard lockups that I've hit. But I did not see any issues
or oddities with this patch.
Cheers,
Phil
--
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Dave Chiluk @ 2019-05-24 15:14 UTC (permalink / raw)
To: Phil Auld
Cc: Peter Oskolkov, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <20190524143204.GB4684@lorien.usersys.redhat.com>
On Fri, May 24, 2019 at 9:32 AM Phil Auld <pauld@redhat.com> wrote:
> On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
> > If the machine runs at/close to capacity, won't the overallocation
> > of the quota to bursty tasks necessarily negatively impact every other
> > task? Should the "unused" quota be available only on idle CPUs?
> > (Or maybe this is the behavior achieved here, and only the comment and
> > the commit message should be fixed...)
> >
>
> It's bounded by the amount left unused from the previous period. So
> theoretically a process could use almost twice its quota. But then it
> would have nothing left over in the next period. To repeat it would have
> to not use any that next period. Over a longer number of periods it's the
> same amount of CPU usage.
>
> I think that is more fair than throttling a process that has never used
> its full quota.
>
> And it removes complexity.
>
> Cheers,
> Phil
Actually it's not even that bad. The overallocation of quota to a
bursty task in a period is limited to at most one slice per cpu, and
that slice must not have been used in the previous periods. The slice
size is set with /proc/sys/kernel/sched_cfs_bandwidth_slice_us and
defaults to 5ms. If a bursty task goes from underutilizing quota to
using it's entire quota, it will not be able to burst in the
subsequent periods. Therefore in an absolute worst case contrived
scenario, a bursty task can add at most 5ms to the latency of other
threads on the same CPU. I think this worst case 5ms tradeoff is
entirely worth it.
This does mean that a theoretically a poorly written massively
threaded application on an 80 core box, that spreads itself onto 80
cpu run queues, can overutilize it's quota in a period by at most 5ms
* 80 CPUs in a sincle period (slice * number of runqueues the
application is running on). But that means that each of those threads
would have had to not be use their quota in a previous period, and it
also means that the application would have to be carefully written to
exacerbate this behavior.
Additionally if cpu bound threads underutilize a slice of their quota
in a period due to the cfs choosing a bursty task to run, they should
theoretically be able to make it up in the following periods when the
bursty task is unable to "burst".
Please be careful here quota and slice are being treated differently.
Quota does not roll-over between periods, only slices of quota that
has already been allocated to per cpu run queues. If you allocate
100ms of quota per period to an application, but it only spreads onto
3 cpu run queues that means it can in the worst case use 3 x slice
size = 15ms in periods following underutilization.
So why does this matter. Well applications that use thread pools
*(*cough* java *cough*) with lots of tiny little worker threads, tend
to spread themselves out onto a lot of run queues. These worker
threads grab quota slices in order to run, then rarely use all of
their slice (1 or 2ms out of the 5ms). This results in those worker
threads starving the main application of quota, and then expiring the
remainder of that quota slice on the per-cpu. Going back to my
earlier 100ms quota / 80 cpu example. That means only
100ms/cfs_bandwidth_slice_us(5ms) = 20 slices are available in a
period. So only 20 out of these 80 cpus ever get a slice allocated to
them. By allowing these per-cpu run queues to use their remaining
slice in following periods these worker threads do not need to be
allocated additional slice, and thereby the main threads are actually
able to use the allocated cpu quota.
This can be experienced by running fibtest available at
https://github.com/indeedeng/fibtest/.
$ runfibtest 1
runs a single fast thread taskset to cpu 0
$ runfibtest 8
Runs a single fast thread taskset to cpu 0, and 7 slow threads taskset
to cpus 1-7. This run is expected to show less iterations, but the
worse problem is that the cpu usage is far less than the 500ms that it
should have received.
Thanks for the engagement on this,
Dave Chiluk
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Phil Auld @ 2019-05-24 14:32 UTC (permalink / raw)
To: Peter Oskolkov
Cc: Dave Chiluk, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc
In-Reply-To: <CAFTs51W0KdK4nw6wydn2HjNYvFRC8DYMmVeKX9FAe+4YUGEAZg@mail.gmail.com>
On Thu, May 23, 2019 at 02:01:58PM -0700 Peter Oskolkov wrote:
> On Thu, May 23, 2019 at 11:44 AM Dave Chiluk <chiluk+linux@indeed.com> wrote:
> >
> > It has been observed, that highly-threaded, non-cpu-bound applications
> > running under cpu.cfs_quota_us constraints can hit a high percentage of
> > periods throttled while simultaneously not consuming the allocated
> > amount of quota. This use case is typical of user-interactive non-cpu
> > bound applications, such as those running in kubernetes or mesos when
> > run on multiple cpu cores.
> >
> > This has been root caused to threads being allocated per cpu bandwidth
> > slices, and then not fully using that slice within the period. At which
> > point the slice and quota expires. This expiration of unused slice
> > results in applications not being able to utilize the quota for which
> > they are allocated.
> >
> > The expiration of per-cpu slices was recently fixed by
> > 'commit 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift
> > condition")'. Prior to that it appears that this has been broken since
> > at least 'commit 51f2176d74ac ("sched/fair: Fix unlocked reads of some
> > cfs_b->quota/period")' which was introduced in v3.16-rc1 in 2014. That
> > added the following conditional which resulted in slices never being
> > expired.
> >
> > if (cfs_rq->runtime_expires != cfs_b->runtime_expires) {
> > /* extend local deadline, drift is bounded above by 2 ticks */
> > cfs_rq->runtime_expires += TICK_NSEC;
> >
> > Because this was broken for nearly 5 years, and has recently been fixed
> > and is now being noticed by many users running kubernetes
> > (https://github.com/kubernetes/kubernetes/issues/67577) it is my opinion
> > that the mechanisms around expiring runtime should be removed
> > altogether.
> >
> > This allows only per-cpu slices to live longer than the period boundary.
> > This allows threads on runqueues that do not use much CPU to continue to
> > use their remaining slice over a longer period of time than
> > cpu.cfs_period_us. However, this helps prevents the above condition of
> > hitting throttling while also not fully utilizing your cpu quota.
> >
> > This theoretically allows a machine to use slightly more than it's
> > allotted quota in some periods. This overflow would be bounded by the
> > remaining per-cpu slice that was left un-used in the previous period.
> > For CPU bound tasks this will change nothing, as they should
> > theoretically fully utilize all of their quota and slices in each
> > period. For user-interactive tasks as described above this provides a
> > much better user/application experience as their cpu utilization will
> > more closely match the amount they requested when they hit throttling.
> >
> > This greatly improves performance of high-thread-count, non-cpu bound
> > applications with low cfs_quota_us allocation on high-core-count
> > machines. In the case of an artificial testcase, this performance
> > discrepancy has been observed to be almost 30x performance improvement,
> > while still maintaining correct cpu quota restrictions albeit over
> > longer time intervals than cpu.cfs_period_us.
>
> If the machine runs at/close to capacity, won't the overallocation
> of the quota to bursty tasks necessarily negatively impact every other
> task? Should the "unused" quota be available only on idle CPUs?
> (Or maybe this is the behavior achieved here, and only the comment and
> the commit message should be fixed...)
>
It's bounded by the amount left unused from the previous period. So
theoretically a process could use almost twice its quota. But then it
would have nothing left over in the next period. To repeat it would have
to not use any that next period. Over a longer number of periods it's the
same amount of CPU usage.
I think that is more fair than throttling a process that has never used
its full quota.
And it removes complexity.
Cheers,
Phil
> > That testcase is
> > available at https://github.com/indeedeng/fibtest.
> >
> > Fixes: 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift condition")
> > Signed-off-by: Dave Chiluk <chiluk+linux@indeed.com>
> > ---
> > Documentation/scheduler/sched-bwc.txt | 29 +++++++++++---
> > kernel/sched/fair.c | 71 +++--------------------------------
> > kernel/sched/sched.h | 4 --
> > 3 files changed, 29 insertions(+), 75 deletions(-)
> >
> > diff --git a/Documentation/scheduler/sched-bwc.txt b/Documentation/scheduler/sched-bwc.txt
> > index f6b1873..4ded8ae 100644
> > --- a/Documentation/scheduler/sched-bwc.txt
> > +++ b/Documentation/scheduler/sched-bwc.txt
> > @@ -8,16 +8,33 @@ CFS bandwidth control is a CONFIG_FAIR_GROUP_SCHED extension which allows the
> > specification of the maximum CPU bandwidth available to a group or hierarchy.
> >
> > The bandwidth allowed for a group is specified using a quota and period. Within
> > -each given "period" (microseconds), a group is allowed to consume only up to
> > -"quota" microseconds of CPU time. When the CPU bandwidth consumption of a
> > -group exceeds this limit (for that period), the tasks belonging to its
> > -hierarchy will be throttled and are not allowed to run again until the next
> > -period.
> > +each given "period" (microseconds), a task group is allocated up to "quota"
> > +microseconds of CPU time. When the CPU bandwidth consumption of a group
> > +exceeds this limit (for that period), the tasks belonging to its hierarchy will
> > +be throttled and are not allowed to run again until the next period.
> >
> > A group's unused runtime is globally tracked, being refreshed with quota units
> > above at each period boundary. As threads consume this bandwidth it is
> > transferred to cpu-local "silos" on a demand basis. The amount transferred
> > -within each of these updates is tunable and described as the "slice".
> > +within each of these updates is tunable and described as the "slice". Slices
> > +that are allocated to cpu-local silos do not expire at the end of the period,
> > +but unallocated quota does. This doesn't affect cpu-bound applications as they
> > +by definition consume all of their bandwidth in each each period.
> > +
> > +However for highly-threaded user-interactive/non-cpu bound applications this
> > +non-expiration nuance allows applications to burst past their quota limits
> > +equal to the amount of unused slice per cpu that the task group is running on.
> > +This slight burst requires that quota had gone unused in previous periods.
> > +Additionally this burst amount is limited to the size of a slice for every cpu
> > +a task group is run on. As a result, this mechanism still strictly limits the
> > +task group to quota average usage over a longer time windows. This provides
> > +better more predictable user experience for highly threaded applications with
> > +small quota limits on high core count machines. It also eliminates the
> > +propensity to throttle these applications while simultanously using less than
> > +quota amounts of cpu. Another way to say this, is that by allowing the unused
> > +portion of a slice to be used in following periods we have decreased the
> > +possibility of wasting unused quota on cpu-local silos that don't need much cpu
> > +time.
> >
> > Management
> > ----------
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index f35930f..a675c69 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4295,8 +4295,6 @@ void __refill_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b)
> >
> > now = sched_clock_cpu(smp_processor_id());
> > cfs_b->runtime = cfs_b->quota;
> > - cfs_b->runtime_expires = now + ktime_to_ns(cfs_b->period);
> > - cfs_b->expires_seq++;
> > }
> >
> > static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
> > @@ -4318,8 +4316,7 @@ static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> > {
> > struct task_group *tg = cfs_rq->tg;
> > struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(tg);
> > - u64 amount = 0, min_amount, expires;
> > - int expires_seq;
> > + u64 amount = 0, min_amount;
> >
> > /* note: this is a positive sum as runtime_remaining <= 0 */
> > min_amount = sched_cfs_bandwidth_slice() - cfs_rq->runtime_remaining;
> > @@ -4336,61 +4333,17 @@ static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> > cfs_b->idle = 0;
> > }
> > }
> > - expires_seq = cfs_b->expires_seq;
> > - expires = cfs_b->runtime_expires;
> > raw_spin_unlock(&cfs_b->lock);
> >
> > cfs_rq->runtime_remaining += amount;
> > - /*
> > - * we may have advanced our local expiration to account for allowed
> > - * spread between our sched_clock and the one on which runtime was
> > - * issued.
> > - */
> > - if (cfs_rq->expires_seq != expires_seq) {
> > - cfs_rq->expires_seq = expires_seq;
> > - cfs_rq->runtime_expires = expires;
> > - }
> >
> > return cfs_rq->runtime_remaining > 0;
> > }
> >
> > -/*
> > - * Note: This depends on the synchronization provided by sched_clock and the
> > - * fact that rq->clock snapshots this value.
> > - */
> > -static void expire_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> > -{
> > - struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
> > -
> > - /* if the deadline is ahead of our clock, nothing to do */
> > - if (likely((s64)(rq_clock(rq_of(cfs_rq)) - cfs_rq->runtime_expires) < 0))
> > - return;
> > -
> > - if (cfs_rq->runtime_remaining < 0)
> > - return;
> > -
> > - /*
> > - * If the local deadline has passed we have to consider the
> > - * possibility that our sched_clock is 'fast' and the global deadline
> > - * has not truly expired.
> > - *
> > - * Fortunately we can check determine whether this the case by checking
> > - * whether the global deadline(cfs_b->expires_seq) has advanced.
> > - */
> > - if (cfs_rq->expires_seq == cfs_b->expires_seq) {
> > - /* extend local deadline, drift is bounded above by 2 ticks */
> > - cfs_rq->runtime_expires += TICK_NSEC;
> > - } else {
> > - /* global deadline is ahead, expiration has passed */
> > - cfs_rq->runtime_remaining = 0;
> > - }
> > -}
> > -
> > static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
> > {
> > /* dock delta_exec before expiring quota (as it could span periods) */
> > cfs_rq->runtime_remaining -= delta_exec;
> > - expire_cfs_rq_runtime(cfs_rq);
> >
> > if (likely(cfs_rq->runtime_remaining > 0))
> > return;
> > @@ -4581,8 +4534,7 @@ void unthrottle_cfs_rq(struct cfs_rq *cfs_rq)
> > resched_curr(rq);
> > }
> >
> > -static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> > - u64 remaining, u64 expires)
> > +static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b, u64 remaining)
> > {
> > struct cfs_rq *cfs_rq;
> > u64 runtime;
> > @@ -4604,7 +4556,6 @@ static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> > remaining -= runtime;
> >
> > cfs_rq->runtime_remaining += runtime;
> > - cfs_rq->runtime_expires = expires;
> >
> > /* we check whether we're throttled above */
> > if (cfs_rq->runtime_remaining > 0)
> > @@ -4629,7 +4580,7 @@ static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> > */
> > static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, unsigned long flags)
> > {
> > - u64 runtime, runtime_expires;
> > + u64 runtime;
> > int throttled;
> >
> > /* no need to continue the timer with no bandwidth constraint */
> > @@ -4657,8 +4608,6 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, u
> > /* account preceding periods in which throttling occurred */
> > cfs_b->nr_throttled += overrun;
> >
> > - runtime_expires = cfs_b->runtime_expires;
> > -
> > /*
> > * This check is repeated as we are holding onto the new bandwidth while
> > * we unthrottle. This can potentially race with an unthrottled group
> > @@ -4671,8 +4620,7 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, u
> > cfs_b->distribute_running = 1;
> > raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
> > /* we can't nest cfs_b->lock while distributing bandwidth */
> > - runtime = distribute_cfs_runtime(cfs_b, runtime,
> > - runtime_expires);
> > + runtime = distribute_cfs_runtime(cfs_b, runtime);
> > raw_spin_lock_irqsave(&cfs_b->lock, flags);
> >
> > cfs_b->distribute_running = 0;
> > @@ -4749,8 +4697,7 @@ static void __return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> > return;
> >
> > raw_spin_lock(&cfs_b->lock);
> > - if (cfs_b->quota != RUNTIME_INF &&
> > - cfs_rq->runtime_expires == cfs_b->runtime_expires) {
> > + if (cfs_b->quota != RUNTIME_INF) {
> > cfs_b->runtime += slack_runtime;
> >
> > /* we are under rq->lock, defer unthrottling using a timer */
> > @@ -4783,7 +4730,6 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> > {
> > u64 runtime = 0, slice = sched_cfs_bandwidth_slice();
> > unsigned long flags;
> > - u64 expires;
> >
> > /* confirm we're still not at a refresh boundary */
> > raw_spin_lock_irqsave(&cfs_b->lock, flags);
> > @@ -4800,7 +4746,6 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> > if (cfs_b->quota != RUNTIME_INF && cfs_b->runtime > slice)
> > runtime = cfs_b->runtime;
> >
> > - expires = cfs_b->runtime_expires;
> > if (runtime)
> > cfs_b->distribute_running = 1;
> >
> > @@ -4809,11 +4754,9 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> > if (!runtime)
> > return;
> >
> > - runtime = distribute_cfs_runtime(cfs_b, runtime, expires);
> > + runtime = distribute_cfs_runtime(cfs_b, runtime);
> >
> > raw_spin_lock_irqsave(&cfs_b->lock, flags);
> > - if (expires == cfs_b->runtime_expires)
> > - lsub_positive(&cfs_b->runtime, runtime);
> > cfs_b->distribute_running = 0;
> > raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
> > }
> > @@ -4969,8 +4912,6 @@ void start_cfs_bandwidth(struct cfs_bandwidth *cfs_b)
> >
> > cfs_b->period_active = 1;
> > overrun = hrtimer_forward_now(&cfs_b->period_timer, cfs_b->period);
> > - cfs_b->runtime_expires += (overrun + 1) * ktime_to_ns(cfs_b->period);
> > - cfs_b->expires_seq++;
> > hrtimer_start_expires(&cfs_b->period_timer, HRTIMER_MODE_ABS_PINNED);
> > }
> >
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index b52ed1a..0c0ed23 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -341,8 +341,6 @@ struct cfs_bandwidth {
> > u64 quota;
> > u64 runtime;
> > s64 hierarchical_quota;
> > - u64 runtime_expires;
> > - int expires_seq;
> >
> > short idle;
> > short period_active;
> > @@ -562,8 +560,6 @@ struct cfs_rq {
> >
> > #ifdef CONFIG_CFS_BANDWIDTH
> > int runtime_enabled;
> > - int expires_seq;
> > - u64 runtime_expires;
> > s64 runtime_remaining;
> >
> > u64 throttled_clock;
> > --
> > 1.8.3.1
> >
--
^ permalink raw reply
* Re: [PATCH] Documentation: gpio: remove duplicated lines
From: Linus Walleij @ 2019-05-24 11:27 UTC (permalink / raw)
To: Luca Ceresoli
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Jonathan Corbet,
linux-kernel@vger.kernel.org, Linux Doc Mailing List
In-Reply-To: <20190510090339.17211-1-luca@lucaceresoli.net>
On Fri, May 10, 2019 at 11:03 AM Luca Ceresoli <luca@lucaceresoli.net> wrote:
> The 'default (active high)' lines are repeated twice. Avoid people stare at
> their screens looking for differences.
>
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Patch applied with Bartosz's ACK.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Peter Zijlstra @ 2019-05-24 8:55 UTC (permalink / raw)
To: Dave Chiluk
Cc: Ingo Molnar, cgroups, linux-kernel, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall, Paul Turner
In-Reply-To: <1558637087-20283-2-git-send-email-chiluk+linux@indeed.com>
(it always helps to Cc the people who actually wrote the code)
Ben, can you have a look at this?
On Thu, May 23, 2019 at 01:44:47PM -0500, Dave Chiluk wrote:
> It has been observed, that highly-threaded, non-cpu-bound applications
> running under cpu.cfs_quota_us constraints can hit a high percentage of
> periods throttled while simultaneously not consuming the allocated
> amount of quota. This use case is typical of user-interactive non-cpu
> bound applications, such as those running in kubernetes or mesos when
> run on multiple cpu cores.
>
> This has been root caused to threads being allocated per cpu bandwidth
> slices, and then not fully using that slice within the period. At which
> point the slice and quota expires. This expiration of unused slice
> results in applications not being able to utilize the quota for which
> they are allocated.
>
> The expiration of per-cpu slices was recently fixed by
> 'commit 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift
> condition")'. Prior to that it appears that this has been broken since
> at least 'commit 51f2176d74ac ("sched/fair: Fix unlocked reads of some
> cfs_b->quota/period")' which was introduced in v3.16-rc1 in 2014. That
> added the following conditional which resulted in slices never being
> expired.
>
> if (cfs_rq->runtime_expires != cfs_b->runtime_expires) {
> /* extend local deadline, drift is bounded above by 2 ticks */
> cfs_rq->runtime_expires += TICK_NSEC;
>
> Because this was broken for nearly 5 years, and has recently been fixed
> and is now being noticed by many users running kubernetes
> (https://github.com/kubernetes/kubernetes/issues/67577) it is my opinion
> that the mechanisms around expiring runtime should be removed
> altogether.
>
> This allows only per-cpu slices to live longer than the period boundary.
> This allows threads on runqueues that do not use much CPU to continue to
> use their remaining slice over a longer period of time than
> cpu.cfs_period_us. However, this helps prevents the above condition of
> hitting throttling while also not fully utilizing your cpu quota.
>
> This theoretically allows a machine to use slightly more than it's
> allotted quota in some periods. This overflow would be bounded by the
> remaining per-cpu slice that was left un-used in the previous period.
> For CPU bound tasks this will change nothing, as they should
> theoretically fully utilize all of their quota and slices in each
> period. For user-interactive tasks as described above this provides a
> much better user/application experience as their cpu utilization will
> more closely match the amount they requested when they hit throttling.
>
> This greatly improves performance of high-thread-count, non-cpu bound
> applications with low cfs_quota_us allocation on high-core-count
> machines. In the case of an artificial testcase, this performance
> discrepancy has been observed to be almost 30x performance improvement,
> while still maintaining correct cpu quota restrictions albeit over
> longer time intervals than cpu.cfs_period_us. That testcase is
> available at https://github.com/indeedeng/fibtest.
>
> Fixes: 512ac999d275 ("sched/fair: Fix bandwidth timer clock drift condition")
> Signed-off-by: Dave Chiluk <chiluk+linux@indeed.com>
> ---
> Documentation/scheduler/sched-bwc.txt | 29 +++++++++++---
> kernel/sched/fair.c | 71 +++--------------------------------
> kernel/sched/sched.h | 4 --
> 3 files changed, 29 insertions(+), 75 deletions(-)
>
> diff --git a/Documentation/scheduler/sched-bwc.txt b/Documentation/scheduler/sched-bwc.txt
> index f6b1873..4ded8ae 100644
> --- a/Documentation/scheduler/sched-bwc.txt
> +++ b/Documentation/scheduler/sched-bwc.txt
> @@ -8,16 +8,33 @@ CFS bandwidth control is a CONFIG_FAIR_GROUP_SCHED extension which allows the
> specification of the maximum CPU bandwidth available to a group or hierarchy.
>
> The bandwidth allowed for a group is specified using a quota and period. Within
> -each given "period" (microseconds), a group is allowed to consume only up to
> -"quota" microseconds of CPU time. When the CPU bandwidth consumption of a
> -group exceeds this limit (for that period), the tasks belonging to its
> -hierarchy will be throttled and are not allowed to run again until the next
> -period.
> +each given "period" (microseconds), a task group is allocated up to "quota"
> +microseconds of CPU time. When the CPU bandwidth consumption of a group
> +exceeds this limit (for that period), the tasks belonging to its hierarchy will
> +be throttled and are not allowed to run again until the next period.
>
> A group's unused runtime is globally tracked, being refreshed with quota units
> above at each period boundary. As threads consume this bandwidth it is
> transferred to cpu-local "silos" on a demand basis. The amount transferred
> -within each of these updates is tunable and described as the "slice".
> +within each of these updates is tunable and described as the "slice". Slices
> +that are allocated to cpu-local silos do not expire at the end of the period,
> +but unallocated quota does. This doesn't affect cpu-bound applications as they
> +by definition consume all of their bandwidth in each each period.
> +
> +However for highly-threaded user-interactive/non-cpu bound applications this
> +non-expiration nuance allows applications to burst past their quota limits
> +equal to the amount of unused slice per cpu that the task group is running on.
> +This slight burst requires that quota had gone unused in previous periods.
> +Additionally this burst amount is limited to the size of a slice for every cpu
> +a task group is run on. As a result, this mechanism still strictly limits the
> +task group to quota average usage over a longer time windows. This provides
> +better more predictable user experience for highly threaded applications with
> +small quota limits on high core count machines. It also eliminates the
> +propensity to throttle these applications while simultanously using less than
> +quota amounts of cpu. Another way to say this, is that by allowing the unused
> +portion of a slice to be used in following periods we have decreased the
> +possibility of wasting unused quota on cpu-local silos that don't need much cpu
> +time.
>
> Management
> ----------
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index f35930f..a675c69 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4295,8 +4295,6 @@ void __refill_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b)
>
> now = sched_clock_cpu(smp_processor_id());
> cfs_b->runtime = cfs_b->quota;
> - cfs_b->runtime_expires = now + ktime_to_ns(cfs_b->period);
> - cfs_b->expires_seq++;
> }
>
> static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
> @@ -4318,8 +4316,7 @@ static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> {
> struct task_group *tg = cfs_rq->tg;
> struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(tg);
> - u64 amount = 0, min_amount, expires;
> - int expires_seq;
> + u64 amount = 0, min_amount;
>
> /* note: this is a positive sum as runtime_remaining <= 0 */
> min_amount = sched_cfs_bandwidth_slice() - cfs_rq->runtime_remaining;
> @@ -4336,61 +4333,17 @@ static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> cfs_b->idle = 0;
> }
> }
> - expires_seq = cfs_b->expires_seq;
> - expires = cfs_b->runtime_expires;
> raw_spin_unlock(&cfs_b->lock);
>
> cfs_rq->runtime_remaining += amount;
> - /*
> - * we may have advanced our local expiration to account for allowed
> - * spread between our sched_clock and the one on which runtime was
> - * issued.
> - */
> - if (cfs_rq->expires_seq != expires_seq) {
> - cfs_rq->expires_seq = expires_seq;
> - cfs_rq->runtime_expires = expires;
> - }
>
> return cfs_rq->runtime_remaining > 0;
> }
>
> -/*
> - * Note: This depends on the synchronization provided by sched_clock and the
> - * fact that rq->clock snapshots this value.
> - */
> -static void expire_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> -{
> - struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
> -
> - /* if the deadline is ahead of our clock, nothing to do */
> - if (likely((s64)(rq_clock(rq_of(cfs_rq)) - cfs_rq->runtime_expires) < 0))
> - return;
> -
> - if (cfs_rq->runtime_remaining < 0)
> - return;
> -
> - /*
> - * If the local deadline has passed we have to consider the
> - * possibility that our sched_clock is 'fast' and the global deadline
> - * has not truly expired.
> - *
> - * Fortunately we can check determine whether this the case by checking
> - * whether the global deadline(cfs_b->expires_seq) has advanced.
> - */
> - if (cfs_rq->expires_seq == cfs_b->expires_seq) {
> - /* extend local deadline, drift is bounded above by 2 ticks */
> - cfs_rq->runtime_expires += TICK_NSEC;
> - } else {
> - /* global deadline is ahead, expiration has passed */
> - cfs_rq->runtime_remaining = 0;
> - }
> -}
> -
> static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
> {
> /* dock delta_exec before expiring quota (as it could span periods) */
> cfs_rq->runtime_remaining -= delta_exec;
> - expire_cfs_rq_runtime(cfs_rq);
>
> if (likely(cfs_rq->runtime_remaining > 0))
> return;
> @@ -4581,8 +4534,7 @@ void unthrottle_cfs_rq(struct cfs_rq *cfs_rq)
> resched_curr(rq);
> }
>
> -static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> - u64 remaining, u64 expires)
> +static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b, u64 remaining)
> {
> struct cfs_rq *cfs_rq;
> u64 runtime;
> @@ -4604,7 +4556,6 @@ static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> remaining -= runtime;
>
> cfs_rq->runtime_remaining += runtime;
> - cfs_rq->runtime_expires = expires;
>
> /* we check whether we're throttled above */
> if (cfs_rq->runtime_remaining > 0)
> @@ -4629,7 +4580,7 @@ static u64 distribute_cfs_runtime(struct cfs_bandwidth *cfs_b,
> */
> static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, unsigned long flags)
> {
> - u64 runtime, runtime_expires;
> + u64 runtime;
> int throttled;
>
> /* no need to continue the timer with no bandwidth constraint */
> @@ -4657,8 +4608,6 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, u
> /* account preceding periods in which throttling occurred */
> cfs_b->nr_throttled += overrun;
>
> - runtime_expires = cfs_b->runtime_expires;
> -
> /*
> * This check is repeated as we are holding onto the new bandwidth while
> * we unthrottle. This can potentially race with an unthrottled group
> @@ -4671,8 +4620,7 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, u
> cfs_b->distribute_running = 1;
> raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
> /* we can't nest cfs_b->lock while distributing bandwidth */
> - runtime = distribute_cfs_runtime(cfs_b, runtime,
> - runtime_expires);
> + runtime = distribute_cfs_runtime(cfs_b, runtime);
> raw_spin_lock_irqsave(&cfs_b->lock, flags);
>
> cfs_b->distribute_running = 0;
> @@ -4749,8 +4697,7 @@ static void __return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
> return;
>
> raw_spin_lock(&cfs_b->lock);
> - if (cfs_b->quota != RUNTIME_INF &&
> - cfs_rq->runtime_expires == cfs_b->runtime_expires) {
> + if (cfs_b->quota != RUNTIME_INF) {
> cfs_b->runtime += slack_runtime;
>
> /* we are under rq->lock, defer unthrottling using a timer */
> @@ -4783,7 +4730,6 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> {
> u64 runtime = 0, slice = sched_cfs_bandwidth_slice();
> unsigned long flags;
> - u64 expires;
>
> /* confirm we're still not at a refresh boundary */
> raw_spin_lock_irqsave(&cfs_b->lock, flags);
> @@ -4800,7 +4746,6 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> if (cfs_b->quota != RUNTIME_INF && cfs_b->runtime > slice)
> runtime = cfs_b->runtime;
>
> - expires = cfs_b->runtime_expires;
> if (runtime)
> cfs_b->distribute_running = 1;
>
> @@ -4809,11 +4754,9 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> if (!runtime)
> return;
>
> - runtime = distribute_cfs_runtime(cfs_b, runtime, expires);
> + runtime = distribute_cfs_runtime(cfs_b, runtime);
>
> raw_spin_lock_irqsave(&cfs_b->lock, flags);
> - if (expires == cfs_b->runtime_expires)
> - lsub_positive(&cfs_b->runtime, runtime);
> cfs_b->distribute_running = 0;
> raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
> }
> @@ -4969,8 +4912,6 @@ void start_cfs_bandwidth(struct cfs_bandwidth *cfs_b)
>
> cfs_b->period_active = 1;
> overrun = hrtimer_forward_now(&cfs_b->period_timer, cfs_b->period);
> - cfs_b->runtime_expires += (overrun + 1) * ktime_to_ns(cfs_b->period);
> - cfs_b->expires_seq++;
> hrtimer_start_expires(&cfs_b->period_timer, HRTIMER_MODE_ABS_PINNED);
> }
>
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index b52ed1a..0c0ed23 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -341,8 +341,6 @@ struct cfs_bandwidth {
> u64 quota;
> u64 runtime;
> s64 hierarchical_quota;
> - u64 runtime_expires;
> - int expires_seq;
>
> short idle;
> short period_active;
> @@ -562,8 +560,6 @@ struct cfs_rq {
>
> #ifdef CONFIG_CFS_BANDWIDTH
> int runtime_enabled;
> - int expires_seq;
> - u64 runtime_expires;
> s64 runtime_remaining;
>
> u64 throttled_clock;
> --
> 1.8.3.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox