Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1740773 > unrolled thread

Re: [PATCH 18/25] hrtimer: Enable soft and hard hrtimer

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 18/25] hrtimer: Enable soft and hard hrtimer Anna-Maria Gleixner <anna-maria@linutronix.de> - 2017-09-27 16:20 +0200
    Re: [PATCH 18/25] hrtimer: Enable soft and hard hrtimer Thomas Gleixner <tglx@linutronix.de> - 2017-09-27 18:00 +0200
      Re: [PATCH 18/25] hrtimer: Enable soft and hard hrtimer Peter Zijlstra <peterz@infradead.org> - 2017-09-27 18:50 +0200

#1740773 — Re: [PATCH 18/25] hrtimer: Enable soft and hard hrtimer

On Tue, 26 Sep 2017, Peter Zijlstra wrote:

> On Thu, Aug 31, 2017 at 12:23:42PM -0000, Anna-Maria Gleixner wrote:
> > --- a/include/linux/hrtimer.h
> > +++ b/include/linux/hrtimer.h
> > @@ -23,6 +23,17 @@
> >  #include <linux/timer.h>
> >  #include <linux/timerqueue.h>
> >  
> > +/*
> > + * Clock ids for hrtimers which expire in softirq context. These clock ids
> > + * are kernel internal and never exported to user space.
> > + */
> > +#define HRTIMER_BASE_SOFT_MASK	MAX_CLOCKS
> > +
> > +#define CLOCK_REALTIME_SOFT	(CLOCK_REALTIME	 | HRTIMER_BASE_SOFT_MASK)
> > +#define CLOCK_MONOTONIC_SOFT	(CLOCK_MONOTONIC | HRTIMER_BASE_SOFT_MASK)
> > +#define CLOCK_BOOTTIME_SOFT	(CLOCK_BOOTTIME	 | HRTIMER_BASE_SOFT_MASK)
> > +#define CLOCK_TAI_SOFT		(CLOCK_TAI	 | HRTIMER_BASE_SOFT_MASK)
> > +
> >  struct hrtimer_clock_base;
> >  struct hrtimer_cpu_base;
> >  
> 
> > --- a/kernel/time/hrtimer.c
> > +++ b/kernel/time/hrtimer.c
> > @@ -1173,7 +1161,7 @@ u64 hrtimer_get_next_event(void)
> >  
> >  static inline int hrtimer_clockid_to_base(clockid_t clock_id)
> >  {
> > +	if (likely(clock_id < MAX_CLOCKS_HRT)) {
> >  		int base = hrtimer_clock_to_base_table[clock_id];
> >  
> >  		if (likely(base != HRTIMER_MAX_CLOCK_BASES))
> > @@ -1193,8 +1181,12 @@ static void __hrtimer_init(struct hrtime
> >  
> >  	cpu_base = raw_cpu_ptr(&hrtimer_bases);
> >  
> > +	if (mode != HRTIMER_MODE_ABS) {
> > +		if (clock_id == CLOCK_REALTIME)
> > +			clock_id = CLOCK_MONOTONIC;
> > +		else if (clock_id == CLOCK_REALTIME_SOFT)
> > +			clock_id = CLOCK_MONOTONIC_SOFT;
> > +	}
> >  
> >  	base = hrtimer_clockid_to_base(clock_id);
> >  	timer->base = &cpu_base->clock_base[base];
> 
> So why expose these extra bases at all, why not stick another flag in
> MODE? These extra bases is a pure implementation detail imo; you could
> equally implement the functionality without (albeit at extra cost).
> 

It was Thomas' request not to use a flag for this.

Anna-Maria

[toc] | [next] | [standalone]

#1740853

On Wed, 27 Sep 2017, Anna-Maria Gleixner wrote:
> On Tue, 26 Sep 2017, Peter Zijlstra wrote:
> 
> > On Thu, Aug 31, 2017 at 12:23:42PM -0000, Anna-Maria Gleixner wrote:
> > > --- a/include/linux/hrtimer.h
> > > +++ b/include/linux/hrtimer.h
> > > @@ -23,6 +23,17 @@
> > >  #include <linux/timer.h>
> > >  #include <linux/timerqueue.h>
> > >  
> > > +/*
> > > + * Clock ids for hrtimers which expire in softirq context. These clock ids
> > > + * are kernel internal and never exported to user space.
> > > + */
> > > +#define HRTIMER_BASE_SOFT_MASK	MAX_CLOCKS
> > > +
> > > +#define CLOCK_REALTIME_SOFT	(CLOCK_REALTIME	 | HRTIMER_BASE_SOFT_MASK)
> > > +#define CLOCK_MONOTONIC_SOFT	(CLOCK_MONOTONIC | HRTIMER_BASE_SOFT_MASK)
> > > +#define CLOCK_BOOTTIME_SOFT	(CLOCK_BOOTTIME	 | HRTIMER_BASE_SOFT_MASK)
> > > +#define CLOCK_TAI_SOFT		(CLOCK_TAI	 | HRTIMER_BASE_SOFT_MASK)
> > > +
> > >  struct hrtimer_clock_base;
> > >  struct hrtimer_cpu_base;
> > >  
> > 
> > > --- a/kernel/time/hrtimer.c
> > > +++ b/kernel/time/hrtimer.c
> > > @@ -1173,7 +1161,7 @@ u64 hrtimer_get_next_event(void)
> > >  
> > >  static inline int hrtimer_clockid_to_base(clockid_t clock_id)
> > >  {
> > > +	if (likely(clock_id < MAX_CLOCKS_HRT)) {
> > >  		int base = hrtimer_clock_to_base_table[clock_id];
> > >  
> > >  		if (likely(base != HRTIMER_MAX_CLOCK_BASES))
> > > @@ -1193,8 +1181,12 @@ static void __hrtimer_init(struct hrtime
> > >  
> > >  	cpu_base = raw_cpu_ptr(&hrtimer_bases);
> > >  
> > > +	if (mode != HRTIMER_MODE_ABS) {
> > > +		if (clock_id == CLOCK_REALTIME)
> > > +			clock_id = CLOCK_MONOTONIC;
> > > +		else if (clock_id == CLOCK_REALTIME_SOFT)
> > > +			clock_id = CLOCK_MONOTONIC_SOFT;
> > > +	}
> > >  
> > >  	base = hrtimer_clockid_to_base(clock_id);
> > >  	timer->base = &cpu_base->clock_base[base];
> > 
> > So why expose these extra bases at all, why not stick another flag in
> > MODE? These extra bases is a pure implementation detail imo; you could
> > equally implement the functionality without (albeit at extra cost).

Right and that cost too high.

> > 
> 
> It was Thomas' request not to use a flag for this.

We can make that a flag as well. There is no hard requirement for making it
a CLOCK. Peter is right that it's a pure internal implementation detail and
we can hide it from the outside world.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1740882

On Wed, Sep 27, 2017 at 05:54:02PM +0200, Thomas Gleixner wrote:

> > > So why expose these extra bases at all, why not stick another flag in
> > > MODE? These extra bases is a pure implementation detail imo; you could
> > > equally implement the functionality without (albeit at extra cost).
> 
> Right and that cost too high.

Yes, no question about that.

> > It was Thomas' request not to use a flag for this.
> 
> We can make that a flag as well. There is no hard requirement for making it
> a CLOCK. Peter is right that it's a pure internal implementation detail and
> we can hide it from the outside world.

Yeah, that was my point.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web