On Mon, Mar 30, 2015 at 09:47:01PM +0530, Viresh Kumar wrote:
> And all I get it is 8256 bytes, with or without the change.
Duh, rounded up to cacheline boundary ;-)
Trades two 4 byte holes at the start for a bigger 'hole' at the end.
struct tvec_base {
spinlock_t lock;
On 30 March 2015 at 19:29, Peter Zijlstra wrote:
> Yeah, so that _should_ not trigger (obviously), and while I agree with
> the sentiment of sanity checks, I'm not sure its worth keeping that
> variable around just for that.
I read it as I can remove it then ? :)
> Anyway, while I'm looking at
On Mon, 30 Mar 2015, Michal Hocko wrote:
> Why cannot we do something like refresh_cpu_vm_stats from the IRQ
> context? Especially the first zone stat part. The per-cpu pagesets is
> more costly and it would need a special treatment, alright. A simple
> way would be to splice the lists from the
On Mon, Mar 30, 2015 at 05:08:18PM +0200, Michal Hocko wrote:
> On Sat 28-03-15 10:53:22, Peter Zijlstra wrote:
> [...]
> > Alternatively the thing hocko suggests is an utter fail too. You cannot
> > stuff that into hardirq context, that's insane.
>
> I guess you are referring to
>
On Sat 28-03-15 10:53:22, Peter Zijlstra wrote:
[...]
> Alternatively the thing hocko suggests is an utter fail too. You cannot
> stuff that into hardirq context, that's insane.
I guess you are referring to
http://article.gmane.org/gmane.linux.kernel.mm/127569, right?
Why cannot we do something
On Mon, Mar 30, 2015 at 06:44:22PM +0530, Viresh Kumar wrote:
> On 30 March 2015 at 18:17, Peter Zijlstra wrote:
> > No, I means something else with that. We can remove the
> > tvec_base::running_timer field. Everything that uses that can use
> > tbase_running() AFAICT.
>
> Okay, there is one
On 30 March 2015 at 18:17, Peter Zijlstra wrote:
> No, I means something else with that. We can remove the
> tvec_base::running_timer field. Everything that uses that can use
> tbase_running() AFAICT.
Okay, there is one instance which still needs it.
migrate_timers():
On Mon, Mar 30, 2015 at 05:32:16PM +0530, Viresh Kumar wrote:
> On 29 March 2015 at 15:54, Peter Zijlstra wrote:
> > What I didn't say, but had thought of is that __run_timer() should skip
> > any timer that has RUNNING set -- for obvious reasons :-)
> Below is copied from your first reply, and
On 29 March 2015 at 15:54, Peter Zijlstra wrote:
> On Sat, Mar 28, 2015 at 02:44:57PM +0100, Peter Zijlstra wrote:
>> > Now there are few issues I see here (Sorry if they are all imaginary):
>> > - In case a timer re-arms itself from its handler and is migrated from CPU
>> > A to B, what
>> >
On Mon, Mar 30, 2015 at 05:32:16PM +0530, Viresh Kumar wrote:
On 29 March 2015 at 15:54, Peter Zijlstra pet...@infradead.org wrote:
What I didn't say, but had thought of is that __run_timer() should skip
any timer that has RUNNING set -- for obvious reasons :-)
Below is copied from your
On 30 March 2015 at 18:17, Peter Zijlstra pet...@infradead.org wrote:
No, I means something else with that. We can remove the
tvec_base::running_timer field. Everything that uses that can use
tbase_running() AFAICT.
Okay, there is one instance which still needs it.
migrate_timers():
On Mon, Mar 30, 2015 at 06:44:22PM +0530, Viresh Kumar wrote:
On 30 March 2015 at 18:17, Peter Zijlstra pet...@infradead.org wrote:
No, I means something else with that. We can remove the
tvec_base::running_timer field. Everything that uses that can use
tbase_running() AFAICT.
Okay,
On 29 March 2015 at 15:54, Peter Zijlstra pet...@infradead.org wrote:
On Sat, Mar 28, 2015 at 02:44:57PM +0100, Peter Zijlstra wrote:
Now there are few issues I see here (Sorry if they are all imaginary):
- In case a timer re-arms itself from its handler and is migrated from CPU
A to B,
On Sat 28-03-15 10:53:22, Peter Zijlstra wrote:
[...]
Alternatively the thing hocko suggests is an utter fail too. You cannot
stuff that into hardirq context, that's insane.
I guess you are referring to
http://article.gmane.org/gmane.linux.kernel.mm/127569, right?
Why cannot we do something
On Mon, Mar 30, 2015 at 05:08:18PM +0200, Michal Hocko wrote:
On Sat 28-03-15 10:53:22, Peter Zijlstra wrote:
[...]
Alternatively the thing hocko suggests is an utter fail too. You cannot
stuff that into hardirq context, that's insane.
I guess you are referring to
On Mon, 30 Mar 2015, Michal Hocko wrote:
Why cannot we do something like refresh_cpu_vm_stats from the IRQ
context? Especially the first zone stat part. The per-cpu pagesets is
more costly and it would need a special treatment, alright. A simple
way would be to splice the lists from the
On Mon, Mar 30, 2015 at 09:47:01PM +0530, Viresh Kumar wrote:
And all I get it is 8256 bytes, with or without the change.
Duh, rounded up to cacheline boundary ;-)
Trades two 4 byte holes at the start for a bigger 'hole' at the end.
struct tvec_base {
spinlock_t lock;
On 30 March 2015 at 19:29, Peter Zijlstra pet...@infradead.org wrote:
Yeah, so that _should_ not trigger (obviously), and while I agree with
the sentiment of sanity checks, I'm not sure its worth keeping that
variable around just for that.
I read it as I can remove it then ? :)
Anyway, while
On Sun, Mar 29, 2015 at 05:31:32PM +0530, Viresh Kumar wrote:
> Warning:
>
> config: blackfin-allyesconfig (attached as .config)
> reproduce:
> wget
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
>
On 28 March 2015 at 19:14, Peter Zijlstra wrote:
> Yeah, something like the below (at the very end) should ensure the thing
> is cacheline aligned, that should give us a fair few bits.
> ---
> kernel/time/timer.c | 36
> 1 file changed, 8 insertions(+), 28
On Sat, Mar 28, 2015 at 02:44:57PM +0100, Peter Zijlstra wrote:
> > Now there are few issues I see here (Sorry if they are all imaginary):
> > - In case a timer re-arms itself from its handler and is migrated from CPU
> > A to B, what
> > happens if the re-armed timer fires before the first
On 28 March 2015 at 19:14, Peter Zijlstra pet...@infradead.org wrote:
Yeah, something like the below (at the very end) should ensure the thing
is cacheline aligned, that should give us a fair few bits.
---
kernel/time/timer.c | 36
1 file changed, 8
On Sun, Mar 29, 2015 at 05:31:32PM +0530, Viresh Kumar wrote:
Warning:
config: blackfin-allyesconfig (attached as .config)
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
git
On Sat, Mar 28, 2015 at 02:44:57PM +0100, Peter Zijlstra wrote:
Now there are few issues I see here (Sorry if they are all imaginary):
- In case a timer re-arms itself from its handler and is migrated from CPU
A to B, what
happens if the re-armed timer fires before the first handler
On Sat, Mar 28, 2015 at 05:27:23PM +0530, viresh kumar wrote:
> So probably we need to make 'base' aligned to 8 bytes ?
Yeah, something like the below (at the very end) should ensure the thing
is cacheline aligned, that should give us a fair few bits.
> So, what you are suggesting is something
On 28 March 2015 at 17:27, viresh kumar wrote:
> On 28 March 2015 at 15:23, Peter Zijlstra wrote:
>
>> Well, for one your patch is indeed disgusting.
>
> Yeah, I agree :)
Sigh..
Sorry for the series of *nonsense* mails before the last one.
Its some thunderbird *BUG* which did that, I was
On 28 March 2015 at 15:23, Peter Zijlstra wrote:
> Well, for one your patch is indeed disgusting.
Yeah, I agree :)
> But yes I'm aware Thomas
> wants to rewrite the timer thing. But Thomas is away for a little while
> and if this really needs to happen then it does.
Sometime back I was trying
On Sat, Mar 28, 2015 at 09:58:38AM +0530, Viresh Kumar wrote:
> On 27 March 2015 at 17:32, Peter Zijlstra wrote:
> > What's not clear to me is why that thing is allocated at all, AFAICT
> > something like:
> >
> > static DEFINE_PER_CPU(struct tvec_base, tvec_bases);
> >
> > Should do the right
On Sat, Mar 28, 2015 at 09:48:28AM +0530, Viresh Kumar wrote:
> On Fri, Mar 27, 2015 at 3:00 PM, Peter Zijlstra wrote:
> > On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
>
> >> So the issue seems to be that we need base->running_timer in order to
> >> tell if a callback is
On 28 March 2015 at 17:27, viresh kumar viresh.ku...@linaro.org wrote:
On 28 March 2015 at 15:23, Peter Zijlstra pet...@infradead.org wrote:
Well, for one your patch is indeed disgusting.
Yeah, I agree :)
Sigh..
Sorry for the series of *nonsense* mails before the last one.
Its some
On Sat, Mar 28, 2015 at 05:27:23PM +0530, viresh kumar wrote:
So probably we need to make 'base' aligned to 8 bytes ?
Yeah, something like the below (at the very end) should ensure the thing
is cacheline aligned, that should give us a fair few bits.
So, what you are suggesting is something
On Sat, Mar 28, 2015 at 09:58:38AM +0530, Viresh Kumar wrote:
On 27 March 2015 at 17:32, Peter Zijlstra pet...@infradead.org wrote:
What's not clear to me is why that thing is allocated at all, AFAICT
something like:
static DEFINE_PER_CPU(struct tvec_base, tvec_bases);
Should do the
On 28 March 2015 at 15:23, Peter Zijlstra pet...@infradead.org wrote:
Well, for one your patch is indeed disgusting.
Yeah, I agree :)
But yes I'm aware Thomas
wants to rewrite the timer thing. But Thomas is away for a little while
and if this really needs to happen then it does.
Sometime
On Sat, Mar 28, 2015 at 09:48:28AM +0530, Viresh Kumar wrote:
On Fri, Mar 27, 2015 at 3:00 PM, Peter Zijlstra pet...@infradead.org wrote:
On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
So the issue seems to be that we need base-running_timer in order to
tell if a callback
On 27 March 2015 at 19:49, Michal Hocko wrote:
> Wouldn't something like I was suggesting few months back
> (http://article.gmane.org/gmane.linux.kernel.mm/127569) solve this
> problem as well? Scheduler should be idle aware, no? I mean it shouldn't
> wake up an idle CPU if the task might run on
On 27 March 2015 at 17:32, Peter Zijlstra wrote:
> What's not clear to me is why that thing is allocated at all, AFAICT
> something like:
>
> static DEFINE_PER_CPU(struct tvec_base, tvec_bases);
>
> Should do the right thing and be much simpler.
Does this comment from timers.c answers your query
On Fri, Mar 27, 2015 at 3:00 PM, Peter Zijlstra wrote:
> On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
>> So the issue seems to be that we need base->running_timer in order to
>> tell if a callback is running, right?
>>
>> We could align the base on 8 bytes to gain an extra bit
On Fri, 27 Mar 2015, Peter Zijlstra wrote:
> On Fri, Mar 27, 2015 at 06:11:44AM -0500, Christoph Lameter wrote:
> >
> > Create a new slab cache for this purpose that does the proper aligning?
>
> That is certainly a possibility, but we'll only ever allocate nr_cpus-1
> entries from it, a whole
On Thu 26-03-15 11:09:01, Viresh Kumar wrote:
> A delayed work to schedule vmstat_shepherd() is queued at periodic intervals
> for
> internal working of vmstat core. This work and its timer end up waking an idle
> cpu sometimes, as this always stays on CPU0.
>
> Because we re-queue the work from
On Fri, Mar 27, 2015 at 06:11:44AM -0500, Christoph Lameter wrote:
> On Fri, 27 Mar 2015, Peter Zijlstra wrote:
>
> > > We could align the base on 8 bytes to gain an extra bit in the pointer
> > > and use that bit to indicate the running state. Then these sites can
> > > spin on that bit while we
On Fri, 27 Mar 2015, Peter Zijlstra wrote:
> > We could align the base on 8 bytes to gain an extra bit in the pointer
> > and use that bit to indicate the running state. Then these sites can
> > spin on that bit while we can change the actual base pointer.
>
> Even though tvec_base has
On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
> On Fri, Mar 27, 2015 at 10:19:54AM +0530, Viresh Kumar wrote:
> > On 27 March 2015 at 01:48, Andrew Morton wrote:
> > > Shouldn't this be viewed as a shortcoming of the core timer code?
> >
> > Yeah, it is. Some (not so pretty)
On Fri, Mar 27, 2015 at 10:19:54AM +0530, Viresh Kumar wrote:
> On 27 March 2015 at 01:48, Andrew Morton wrote:
> > Shouldn't this be viewed as a shortcoming of the core timer code?
>
> Yeah, it is. Some (not so pretty) solutions were tried earlier to fix that,
> but
> they are rejected for
On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
On Fri, Mar 27, 2015 at 10:19:54AM +0530, Viresh Kumar wrote:
On 27 March 2015 at 01:48, Andrew Morton a...@linux-foundation.org wrote:
Shouldn't this be viewed as a shortcoming of the core timer code?
Yeah, it is. Some
On Fri, 27 Mar 2015, Peter Zijlstra wrote:
We could align the base on 8 bytes to gain an extra bit in the pointer
and use that bit to indicate the running state. Then these sites can
spin on that bit while we can change the actual base pointer.
Even though tvec_base has
On Fri, Mar 27, 2015 at 10:19:54AM +0530, Viresh Kumar wrote:
On 27 March 2015 at 01:48, Andrew Morton a...@linux-foundation.org wrote:
Shouldn't this be viewed as a shortcoming of the core timer code?
Yeah, it is. Some (not so pretty) solutions were tried earlier to fix that,
but
they
On Fri, Mar 27, 2015 at 06:11:44AM -0500, Christoph Lameter wrote:
On Fri, 27 Mar 2015, Peter Zijlstra wrote:
We could align the base on 8 bytes to gain an extra bit in the pointer
and use that bit to indicate the running state. Then these sites can
spin on that bit while we can change
On Thu 26-03-15 11:09:01, Viresh Kumar wrote:
A delayed work to schedule vmstat_shepherd() is queued at periodic intervals
for
internal working of vmstat core. This work and its timer end up waking an idle
cpu sometimes, as this always stays on CPU0.
Because we re-queue the work from its
On Fri, 27 Mar 2015, Peter Zijlstra wrote:
On Fri, Mar 27, 2015 at 06:11:44AM -0500, Christoph Lameter wrote:
Create a new slab cache for this purpose that does the proper aligning?
That is certainly a possibility, but we'll only ever allocate nr_cpus-1
entries from it, a whole new slab
On Fri, Mar 27, 2015 at 3:00 PM, Peter Zijlstra pet...@infradead.org wrote:
On Fri, Mar 27, 2015 at 10:16:13AM +0100, Peter Zijlstra wrote:
So the issue seems to be that we need base-running_timer in order to
tell if a callback is running, right?
We could align the base on 8 bytes to gain an
On 27 March 2015 at 19:49, Michal Hocko mho...@suse.cz wrote:
Wouldn't something like I was suggesting few months back
(http://article.gmane.org/gmane.linux.kernel.mm/127569) solve this
problem as well? Scheduler should be idle aware, no? I mean it shouldn't
wake up an idle CPU if the task
On 27 March 2015 at 17:32, Peter Zijlstra pet...@infradead.org wrote:
What's not clear to me is why that thing is allocated at all, AFAICT
something like:
static DEFINE_PER_CPU(struct tvec_base, tvec_bases);
Should do the right thing and be much simpler.
Does this comment from timers.c
On 27 March 2015 at 01:48, Andrew Morton wrote:
> Shouldn't this be viewed as a shortcoming of the core timer code?
Yeah, it is. Some (not so pretty) solutions were tried earlier to fix that, but
they are rejected for obviously reasons [1].
> vmstat_shepherd() is merely rescheduling itself with
On Thu, 26 Mar 2015 11:09:01 +0530 Viresh Kumar wrote:
> A delayed work to schedule vmstat_shepherd() is queued at periodic intervals
> for
> internal working of vmstat core. This work and its timer end up waking an idle
> cpu sometimes, as this always stays on CPU0.
>
> Because we re-queue
On 27 March 2015 at 01:48, Andrew Morton a...@linux-foundation.org wrote:
Shouldn't this be viewed as a shortcoming of the core timer code?
Yeah, it is. Some (not so pretty) solutions were tried earlier to fix that, but
they are rejected for obviously reasons [1].
vmstat_shepherd() is merely
On Thu, 26 Mar 2015 11:09:01 +0530 Viresh Kumar viresh.ku...@linaro.org wrote:
A delayed work to schedule vmstat_shepherd() is queued at periodic intervals
for
internal working of vmstat core. This work and its timer end up waking an idle
cpu sometimes, as this always stays on CPU0.
A delayed work to schedule vmstat_shepherd() is queued at periodic intervals for
internal working of vmstat core. This work and its timer end up waking an idle
cpu sometimes, as this always stays on CPU0.
Because we re-queue the work from its handler, idle_cpu() returns false and so
the timer
A delayed work to schedule vmstat_shepherd() is queued at periodic intervals for
internal working of vmstat core. This work and its timer end up waking an idle
cpu sometimes, as this always stays on CPU0.
Because we re-queue the work from its handler, idle_cpu() returns false and so
the timer
58 matches
Mail list logo
