Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-22  1:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50FDE2AE.8030608@linaro.org>

On Mon, Jan 21, 2013 at 6:51 PM, John Stultz <john.stultz@linaro.org> wrote:
> On 01/21/2013 02:54 PM, Matt Sealey wrote:
>>
>> On Mon, Jan 21, 2013 at 4:36 PM, John Stultz <john.stultz@linaro.org>
>> wrote:
>>>
>>> On 01/21/2013 01:14 PM, Matt Sealey wrote:
>
> As far as jiffies rating, from jiffies.c:
>     .rating        = 1, /* lowest valid rating*/
>
> So I'm not sure what you mean by "the debug on the kernel log is telling me
> it has a higher resolution".

Oh, it is just if I actually don't run setup_sched_clock on my
platform, it gives a little message (with #define DEBUG 1 in
sched_clock.c) about who setup the last sched_clock. Since you only
get one chance, and I was fiddling with setup_sched_clock being probed
from multiple possible timers from device tree (i.MX3 has a crapload
of valid timers, which one you use right now is basically forced by
the not-quite-fully-DT-only code and some funky iomap tricks).

And what I got was, if I use the real hardware timer, it runs at 66MHz
and says it has 15ns resolution and wraps every 500 seconds or so. The
jiffies timer says it's 750MHz, with a 2ns resoluton.. you get the
drift. The generic reporting of how "good" the sched_clock source is
kind of glosses over the quality rating of the clock source and at
first glance (if you're not paying that much attention), it is a
little bit misleading..

> Yes, in the case I was remembering, the 60HZ was driven by the electrical
> line.

While I have your attention, what would be the minimum "good" speed to
run the sched_clock or delay timer implementation from? My rudimentary
scribblings in my notebook give me a value of "don't bother" with less
than 10KHz based on HZ=100, so I'm wondering if a direct 32.768KHz
clock would do (i.MX osc clock input if I can supply it to one of the
above myriad timers) since this would be low-power compared to a 66MHz
one (by a couple mA anyway). I also have a bunch of questions about
the delay timer requirements.. I might mail you personally.. or would
you prefer on-list?

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* [PATCH] ARM: plat-samsung: using vsnprintf instead of vsprintf for the limit buffer length 256
From: Chen Gang @ 2013-01-22  1:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121160008.GO23505@n2100.arm.linux.org.uk>

? 2013?01?22? 00:00, Russell King - ARM Linux ??:
> sizeof(buff) would be better here so that it depends on the actual buffer
> size.

  thank you, I will send patch v2.

  :-)

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* [PATCH] ARM: plat-samsung: using vsnprintf instead of vsprintf for the limit buffer length 256
From: Chen Gang @ 2013-01-22  1:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121135626.GP1095@trinity.fluff.org>

? 2013?01?21? 21:56, Ben Dooks ??:
> How about: vsnprintf(buff, sizeof(buff), fmt, va);
> 
> It means we do not end up assuming the size of 'buff' and will be correct
> if the code is changed to declare 'buff' to be a different size.

  thank you, I will send patch v2.

  :-)

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* [PATCH] crypto: fix FTBFS with ARM SHA1-asm and THUMB2_KERNEL
From: Matt Sealey @ 2013-01-22  0:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301211739470.6300@xanadu.home>

On Mon, Jan 21, 2013 at 4:46 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 21 Jan 2013, Matt Sealey wrote:
>
>> The optimized assembler SHA1 code for ARM does not conform to Thumb2
>> register usage requirements, so it cannot be built when the kernel is
>> configured with THUMB2_KERNEL.
>>
>> Fix the FTBFS for now by preventing misconfigurations of the kernel.
>>
>> Signed-off-by: Matt Sealey <matt@genesi-usa.com>
>
> A .arm directive at the top of the assembly code would be a better
> "fix", as that wouldn't reduce functionality.

If I recall, doing that last time for ssi-fiq.S was the wrong solution
and it was suggesed proper configuration (on top of possibly rewriting
the assembly) was better than hacking around in the assembly..

> Yet, I'd invite you to have a look at commit 638591cd7b in linux-next.

I took a peek, and invite you to ignore my patch. I only tracked the
top of Linus' tree..

That said, it seems nobody benchmarked this on something different
than IXP425 or KS8695 to see if it's markedly faster than the
(moderately recently updated) C-code implementation outside of the
mentioned in the logs for initial commit? It seems like rather a
specific optimization for a rather specific use case for rather
specific processors (and therefore a small test base) probably meant
for a very specific product line somewhere. Whether you get any
benefit in enabling this config item or not for any other ARM platform
is up for debate, isn't it?

If it *is* in fact much faster everywhere, and it works in any ARM or
THUMB2 configuration, there's a case to be built for it being the
default ARM implementation for AES and SHA1..

This question is to the implementor/committer (Dave McCullough), how
exactly did you measure the benchmark and can we reproduce it on some
other ARM box?

If it's long and laborious and not so important to test the IPsec
tunnel use-case, what would be the simplest possible benchmark to see
if the C vs. assembly version is faster for a particular ARM device? I
can get hold of pretty much any Cortex-A8 or Cortex-A9 that matters, I
have access to a Chromebook for A15, and maybe an i.MX27 or i.MX35 and
a couple Marvell boards (ARMv6) if I set my mind to it... that much
testing implies we find a pretty concise benchmark though with a
fairly common kernel version we can spread around (i.MX, OMAP and the
Chromebook, I can handle, the rest I'm a little wary of bothering to
spend too much time on). I think that could cover a good swath of
not-ARMv5 use cases from lower speeds to quad core monsters.. but I
might stick to i.MX to start with..

--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: John Stultz @ 2013-01-22  0:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1bmtTH1N8nx79MGiMighdQROpNtpvAZXhHKT+mT3kcazBQ@mail.gmail.com>

On 01/21/2013 02:54 PM, Matt Sealey wrote:
> On Mon, Jan 21, 2013 at 4:36 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On 01/21/2013 01:14 PM, Matt Sealey wrote:
>>> My question really has to be is CONFIG_SCHED_HRTICK useful, what
>>> exactly is it going to do on ARM here since nobody can ever have
>>> enabled it? Is it going to keel over and explode if nobody registers a
>>> non-jiffies sched_clock (since the jiffies clock is technically
>>> reporting itself as a ridiculously high resolution clocksource..)?
>> ??? Not following this at all.  jiffies is the *MOST* coarse resolution
>> clocksource there is (at least that I'm aware of.. I recall someone wanting
>> to do a 60Hz clocksource, but I don't think that ever happened).
> Is that based on it's clocksource rating (probably worse than a real
> hrtimer) or it's reported resolution? Because on i.MX51 if I force it
> to use the jiffies clock the debug on the kernel log is telling me it
> has a higher resolution (it TELLS me that it ticks "as fast" as the
> CPU frequency and wraps less than my real timer).
So the clocksource rating is supposed to be defined by the clocksource 
driver writer, and just provides a way for the clocksource core to 
select the best clocksource given a set of clocksources. It is not 
defined as any sort of calculated mapping to any property of the 
clocksource itself (although some driver writers might compute a ratings 
value in that way, but I feel the static ranking is much simpler). The 
comment above struct clocksource in clocksource.h tries to explain this.

As far as jiffies rating, from jiffies.c:
     .rating        = 1, /* lowest valid rating*/

So I'm not sure what you mean by "the debug on the kernel log is telling 
me it has a higher resolution".



> I know where the 60Hz clocksource might come from, the old Amiga
> platforms have one based on the PSU frequency (50Hz in Europe, 60Hz
> US/Japan). Even a 60Hz clocksource is useful though (on the Amiga, at
> least, it is precisely the vsync clock for synchronizing your display
> output on TV-out, which makes it completely useful for the framebuffer
> driver), but.. you just won't expect to assign it as sched_clock or
> your delay timer. And if anyone does I'd expect they'd know full well
> it'd not run so well.

Yes, in the case I was remembering, the 60HZ was driven by the 
electrical line.

thanks
-john

^ permalink raw reply

* [PATCH] ARM: tegra114: cpuidle: add ARM_CPUIDLE_WFI_STATE support
From: Joseph Lo @ 2013-01-22  0:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50FD7876.4010306@wwwdotorg.org>

On Tue, Jan 22, 2013 at 01:18:46, Stephen Warren wrote:
> On 01/21/2013 02:49 AM, Joseph Lo wrote:
> > Adding the generic ARM_CPUIDLE_WFI_STATE support for Tegra114.
> 
> This of course depends on Hiroshi's Tegra114 patches, right; you 
> should explicitly point out any dependencies when posting patches.
> 
> The code changes look reasonable, but of course I'll hold off applying 
> it until the Tegra114 support is checked in.

Yes, thanks for taking care.

Joseph

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: John Stultz @ 2013-01-22  0:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1bmtTH1N8nx79MGiMighdQROpNtpvAZXhHKT+mT3kcazBQ@mail.gmail.com>

On 01/21/2013 02:54 PM, Matt Sealey wrote:
> On Mon, Jan 21, 2013 at 4:36 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On 01/21/2013 01:14 PM, Matt Sealey wrote:
>>> Or is this one of those things that if your platform doesn't have a
>>> real high resolution timer, you shouldn't enable HRTIMERS and
>>> therefore not enable SCHED_HRTICK as a result? That affects
>>> ARCH_MULTIPLATFORM here. Is the solution as simple as
>>> ARCH_MULTIPLATFORM compliant platforms kind of have to have a high
>>> resolution timer? Documentation to that effect?
>> SO HRITMERS was designed to be be build time enabled, while still giving you
>> a functioning system if it was booted on a system that didn't support
>> clockevents.  We boot with standard HZ, and only switch over to HRT mode if
>> we have a proper clocksource and clockevent driver.
> Okay. I'm still a little confused as to what SCHED_HRTICK actually
> makes a difference to, though.
>
>  From that description, we are booting with standard HZ on ARM, and the
> core sched_clock (as in we can call setup_sched_clock)
> and/or/both/optionally using a real delay_timer switch to HRT mode if
> we have the right equipment available in the kernel and at runtime on
> the SoC.. but the process scheduler isn't compiled with the means to
> actually take advantage of us being in HRT mode?

So I'm actually not super familiar with SCHED_HRTICK details, but from 
my brief skim of it it looks like its useful for turning off the 
periodic timer tick, and allowing the scheduler tick to be triggered by 
an hrtimer itself (There's a number of these interesting inversions that 
go on in switching to HRT mode - for instance, standard timer ticks are 
switched to being hrtimer events themselves).

This likely has the benefit of time-accurate preemption (well, long 
term, as if the timer granularity isn't matching you could be delayed up 
to a tick - but it wouldn't drift).

I'm guessing Thomas would probably know best what the potential issues 
would be from running ((CONFIG_HRTIMER  || CONFIG_NO_HZ) && 
!CONFIG_SCHED_HRTICK).

thanks
-john

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-22  0:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1bk-K=D6EDBfJ7LggpTroj1sFC8VStctqGi=NacLEu95cA@mail.gmail.com>

On Mon, Jan 21, 2013 at 6:09 PM, Matt Sealey <matt@genesi-usa.com> wrote:

[LAKML: about lack of SCHED_HRTICK because we don't use
kernel/Kconfig.hz on ARM)]

> kind of leaves ARM in the doghouse.. who knows what weirdo scheduler
> reactions are related to it not being enabled. Maybe when it is, HZ
> *would* need to be allowed to be bumped when using this code path?

Or conversely maybe this is exactly why the Samsung maintainers
decided they need HZ=200, because SCHED_HRTICK isn't being enabled and
they're experiencing some multimedia lag because of it?

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* [PATCH v2 1/2] ARM: shmobile: sh73a0: Use generic irqchip_init()
From: Simon Horman @ 2013-01-22  0:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121070301.GB15508@avionic-0098.adnet.avionic-design.de>

On Mon, Jan 21, 2013 at 08:03:01AM +0100, Thierry Reding wrote:
> On Mon, Jan 21, 2013 at 09:54:39AM +0900, Simon Horman wrote:
> > On Fri, Jan 18, 2013 at 08:16:12AM +0100, Thierry Reding wrote:
> > > The asm/hardware/gic.h header does no longer exist and the corresponding
> > > functionality was moved to linux/irqchip.h and linux/irqchip/arm-gic.h
> > > respectively. gic_handle_irq() and of_irq_init() are no longer available
> > > either and have been replaced by irqchip_init().
> > 
> > asm/hardware/gic.h Seems to still exist in Linus's tree.
> > Could you let me know which tree of which branch I should depend on
> > in order to apply this change?
> 
> I found this when doing an automated build over all ARM defconfigs on
> linux-next.
> 
> Commit 520f7bd73354f003a9a59937b28e4903d985c420 "irqchip: Move ARM gic.h
> to include/linux/irqchip/arm-gic.h" moved the file and was merged
> through Olof Johansson's next/cleanup and for-next branches.
> 
> Adding Olof on Cc since I'm not quite sure myself about how this is
> handled.

Thanks, I'm not quite sure either.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-22  0:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121234908.GD23505@n2100.arm.linux.org.uk>

Okay so the final resolution of this is;

1) That the arch/arm/Kconfig HZ block is suffering from some cruft

I think we could all be fairly confident that Exynos4 or S5P does not
require HZ=200 - in theory, it has no such timer restrictions like
EBSA110 (the docs I have show a perfectly capable 32-bit timer with a
double-digits MHz input clock, since these are multimedia-class SoCs
it'd be seriously f**ked up if they didn't).

But while some of the entries on this line may be cargo-cult
programming, the original addition on top of EBSA110 *may* be one of
your "unreported" responsiveness issues.

We could just let some Samsung employees complain when Android 6.x
starts to get laggy with a 3.8 kernel because we forced their HZ=100.

What I would do is predicate a fixed, obvious default on
ARCH_MULTIPLATFORM so that it would get the benefit of a default HZ
that you agree with. It wouldn't CHANGE anything, but it makes it look
less funky, since the non-multiplatform settings would be somewhere
else (it either needs more comments or an if - either way - otherwise
it's potentially confusing);

if ARCH_MULTIPLATFORM
config HZ
   int
   default 100
else
   # old config HZ block here
endif

2) We need to add config SCHED_HRTICK as a copy and paste from
kernel/Kconfig.hz since.. well, I still don't understand exactly what
the true effect would be, but I assume since Arnd is concerned and
John's explanation rings true that it really should be enabled on ARM
systems with the exact same dependencies as kernel/Kconfig.hz.

Or not.. I see it as an oddity until I understand if we really care
about it, but the code seems to be fairly important to the scheduler
and also enabled by default almost everywhere else, which means only
people with really freakish SMP architectures with no ability to use
GENERIC_SMP_HELPERS have ever run these code paths besides ARM. That
kind of leaves ARM in the doghouse.. who knows what weirdo scheduler
reactions are related to it not being enabled. Maybe when it is, HZ
*would* need to be allowed to be bumped when using this code path?
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.


On Mon, Jan 21, 2013 at 5:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 21, 2013 at 05:23:33PM -0600, Matt Sealey wrote:
>> On Mon, Jan 21, 2013 at 4:42 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Jan 21, 2013 at 04:20:14PM -0600, Matt Sealey wrote:
>> >> I am sorry it sounded if I was being high and mighty about not being
>> >> able to select my own HZ (or being forced by Exynos to be 200 or by
>> >> not being able to test an Exynos board, forced to default to 100). My
>> >> real "grievance" here is we got a configuration item for the scheduler
>> >> which is being left out of ARM configurations which *can* use high
>> >> resolution timers, but I don't know if this is a real problem or not,
>> >> hence asking about it, and that HZ=100 is the ARM default whether we
>> >> might be able to select that or not.. which seems low.
>> >
>> > Well, I have a versatile platform here.  It's the inteligence behind
>> > the power control system for booting the boards on the nightly tests
>> > (currently disabled because I'm waiting for my main server to lock up
>> > again, and I need to use one of the serial ports for that.)
>> >
>> > The point is, it talks via I2C to a load of power monitors to read
>> > samples out.  It does this at sub-100Hz intervals.  Yet the kernel is
>> > built with HZ=100.  NO_HZ=y and highres timers are enabled... works
>> > fine.
>> >
>> > So, no, HZ=100 is not a limit in that scenario.  With NO_HZ=y and
>> > highres timers, it all works with epoll() - you get the interval that
>> > you're after.  I've verified this with calls to gettimeofday() and
>> > the POSIX clocks.
>>
>> Okay.
>>
>> So, can you read this (it's short):
>>
>> http://ck.kolivas.org/patches/bfs/bfs-configuration-faq.txt
>>
>> And please tell me if he's batshit crazy and I should completely
>> ignore any scheduler discussion that isn't ARM-specific, or maybe..
>> and I can almost guarantee this, he doesn't have an ARM platform so
>> he's just delightfully ill-informed about anything but his quad-core
>> x86?
>
> Well... my x86 laptop is... HZ=1000, NO_HZ, HIGH_RES enabled, ondemand...
> doesn't really fit into any of those categories given there.  I'd suggest
> that what's given there is a suggestion/opinion based on behaviours
> observed on x86 platforms.
>
> Whether it's appropriate for other architectures is not really a proven
> point - is it worth running ARM at 1000Hz when the load from running at
> 100Hz is measurable as a definite error in loops_per_jiffy calibration?
> Remember - the load from the interrupt handler at 1000Hz is 10x the load
> at 100Hz.
>
> Do you want to spend more cycles per second on the possibly multi-layer
> IRQ servicing and timer servicing?
>
> And what about the interrupt latency issue that we've hit several times
> already with devices taking longer than 10ms to service their peripherals
> because the driver doesn't make use of delayed works/tasklets/etc.
>
> The lack of reasonable device DMA too has an impact for many drivers - the
> CPU has to spend more time in interrupt handlers (which are now run to the
> exclusion of any other interrupt in the system) performing PIO - or in the
> case of those systems which _do_ have DMA, they may end up having to do
> cache maintanence over large cache ranges from IRQ context which x86
> doesn't have to do.
>
> There's many factors here, and the choice of what the right HZ is for a
> platform is not as clear cut as one may think.  Given all the additional
> overheads we have on ARM because of the lack of memory coherency, the
> generally bad DMA support, etc, I think what we currently have is still
> right as an architecture default - 100Hz.
>
>> I did test it.. whatever you define last, sticks, and it's down to the
>> order they're parsed in the tree - luckily, arch/arm/Kconfig is
>> sourced first, which sources the mach/plat stuff way down at  the
>> bottom. As long as you have your "default" set somewhere, any further
>> default just has to be sourced or added later in *one* of the
>> Kconfigs, same as building any C file with "gcc -E" and spitting it
>> out.
>>
>> Someone, at the end of it all, has to set some default, and as long as
>> the one you want is the last one, everything is shiny.
>
> Actually, we're both wrong.  There seems to be two things which
> inflence it, and it basically comes down to this:
>
> - the value a particular symbol has comes from the _first_ declaration
>   which a value is assigned to a symbol.
>
> So:
>
> config HZ
>         int
>         default 300
>
> config HZ
>         int
>         default 100 if OPT1
>         default 200 if OPT2
>         default 400
>
> takes on the value of 300 no matter what combination of OPT1 and OPT2
> are enabled.
>
> config HZ
>         int
>         default 100 if OPT1
>         default 200 if OPT2
>         default 400
>
> config HZ
>         int
>         default 300
>
> never takes the value 300, but 100, 200 or 400.
>
> config HZ
>         int
>         default 100 if OPT1
>         default 200 if OPT2
>
> config HZ
>         int
>         default 300
>
> Will now take 100, 200, or 300 depending on which of OPT1/OPT2 are enabled.
>
> So, we _can_ use kernel/Kconfig.hz, but it's not very nice at all: we will
> be presenting users with configutation options for the HZ value which will
> be _silently_ ignored by Kconfig if we have a platform which overrides this.
>
> Probably fine if you think that Kconfig is a developers tool and you edit
> the configuration files (and therefore you expect them to know what they're
> doing, and how this stuff works), but not if you think that Kconfig users
> should be presented with meaningful options when configuring their kernel.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Russell King - ARM Linux @ 2013-01-22  0:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1b=wDE9ujby_gcoHJudsG=JbWDdNRo+thCe8BhoJLi+B5Q@mail.gmail.com>

On Mon, Jan 21, 2013 at 05:30:31PM -0600, Matt Sealey wrote:
> But it would effectively stop users drinking kool-aid.. if you set
> your HZ to something stupid, you don't even get a kernel to build, and
> certainly don't get to boot past the first 40 lines of boot messages..
> I think most people would rather a build error, or a runtime
> unmistakable, unmissable warning than a subtle and almost
> imperceptible skew in NTP synchronization, to use your example.

1. a kernel which doesn't build.  What do you think both Arnd and myself
   have been doing for the last few years, building such things as
   random configurations and such like, finding stuff that doesn't work
   and fixing the kernel so that we end up with _NO_ configuration which
   fails to build.

   Are you seriously about to tell us that we're wasting our time and we
   should just let the kernel build fail in all horrid sorts of ways?

2. As for NTP behaviour... well, have you ever experienced a system where
   NTP has to keep doing step corrections on the time of day, where some
   steps (eg, backwards) cause services to quit because time of day must
   be monotonic...

What you're proposing is that we litter the ARM arch with all sorts of
tests for CONFIG_HZ and #error out on ones that don't make sense.  I
think you're smoking crack.

What I think is that we should _not_ allow CONFIG_HZ to be set to
anything which isn't appropriate for the platforms - or indeed the
reverse.  That's going to be extremely difficult to do with multi-arch
because it's effectively a two-way dependency.

I don't think we can do that with kernel/Kconfig.hz unless we introduce
another layer of permissive configurations for the HZ_1000... etc, but
I'm not sure that anyone outside ARM would like even that.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Russell King - ARM Linux @ 2013-01-21 23:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1b=AG_a=KCi6qoeoRoG+5bcYkn3gAVb4S2dbaFzu4ioJdQ@mail.gmail.com>

On Mon, Jan 21, 2013 at 05:23:33PM -0600, Matt Sealey wrote:
> On Mon, Jan 21, 2013 at 4:42 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jan 21, 2013 at 04:20:14PM -0600, Matt Sealey wrote:
> >> I am sorry it sounded if I was being high and mighty about not being
> >> able to select my own HZ (or being forced by Exynos to be 200 or by
> >> not being able to test an Exynos board, forced to default to 100). My
> >> real "grievance" here is we got a configuration item for the scheduler
> >> which is being left out of ARM configurations which *can* use high
> >> resolution timers, but I don't know if this is a real problem or not,
> >> hence asking about it, and that HZ=100 is the ARM default whether we
> >> might be able to select that or not.. which seems low.
> >
> > Well, I have a versatile platform here.  It's the inteligence behind
> > the power control system for booting the boards on the nightly tests
> > (currently disabled because I'm waiting for my main server to lock up
> > again, and I need to use one of the serial ports for that.)
> >
> > The point is, it talks via I2C to a load of power monitors to read
> > samples out.  It does this at sub-100Hz intervals.  Yet the kernel is
> > built with HZ=100.  NO_HZ=y and highres timers are enabled... works
> > fine.
> >
> > So, no, HZ=100 is not a limit in that scenario.  With NO_HZ=y and
> > highres timers, it all works with epoll() - you get the interval that
> > you're after.  I've verified this with calls to gettimeofday() and
> > the POSIX clocks.
> 
> Okay.
> 
> So, can you read this (it's short):
> 
> http://ck.kolivas.org/patches/bfs/bfs-configuration-faq.txt
> 
> And please tell me if he's batshit crazy and I should completely
> ignore any scheduler discussion that isn't ARM-specific, or maybe..
> and I can almost guarantee this, he doesn't have an ARM platform so
> he's just delightfully ill-informed about anything but his quad-core
> x86?

Well... my x86 laptop is... HZ=1000, NO_HZ, HIGH_RES enabled, ondemand...
doesn't really fit into any of those categories given there.  I'd suggest
that what's given there is a suggestion/opinion based on behaviours
observed on x86 platforms.

Whether it's appropriate for other architectures is not really a proven
point - is it worth running ARM at 1000Hz when the load from running at
100Hz is measurable as a definite error in loops_per_jiffy calibration?
Remember - the load from the interrupt handler at 1000Hz is 10x the load
at 100Hz.

Do you want to spend more cycles per second on the possibly multi-layer
IRQ servicing and timer servicing?

And what about the interrupt latency issue that we've hit several times
already with devices taking longer than 10ms to service their peripherals
because the driver doesn't make use of delayed works/tasklets/etc.

The lack of reasonable device DMA too has an impact for many drivers - the
CPU has to spend more time in interrupt handlers (which are now run to the
exclusion of any other interrupt in the system) performing PIO - or in the
case of those systems which _do_ have DMA, they may end up having to do
cache maintanence over large cache ranges from IRQ context which x86
doesn't have to do.

There's many factors here, and the choice of what the right HZ is for a
platform is not as clear cut as one may think.  Given all the additional
overheads we have on ARM because of the lack of memory coherency, the
generally bad DMA support, etc, I think what we currently have is still
right as an architecture default - 100Hz.

> I did test it.. whatever you define last, sticks, and it's down to the
> order they're parsed in the tree - luckily, arch/arm/Kconfig is
> sourced first, which sources the mach/plat stuff way down at  the
> bottom. As long as you have your "default" set somewhere, any further
> default just has to be sourced or added later in *one* of the
> Kconfigs, same as building any C file with "gcc -E" and spitting it
> out.
> 
> Someone, at the end of it all, has to set some default, and as long as
> the one you want is the last one, everything is shiny.

Actually, we're both wrong.  There seems to be two things which
inflence it, and it basically comes down to this:

- the value a particular symbol has comes from the _first_ declaration
  which a value is assigned to a symbol.

So:

config HZ
        int
        default 300

config HZ
        int
        default 100 if OPT1
        default 200 if OPT2
        default 400

takes on the value of 300 no matter what combination of OPT1 and OPT2
are enabled.

config HZ
        int
        default 100 if OPT1
        default 200 if OPT2
        default 400
           
config HZ
        int
        default 300

never takes the value 300, but 100, 200 or 400.

config HZ
        int
        default 100 if OPT1
        default 200 if OPT2
           
config HZ
        int
        default 300

Will now take 100, 200, or 300 depending on which of OPT1/OPT2 are enabled.

So, we _can_ use kernel/Kconfig.hz, but it's not very nice at all: we will
be presenting users with configutation options for the HZ value which will
be _silently_ ignored by Kconfig if we have a platform which overrides this.

Probably fine if you think that Kconfig is a developers tool and you edit
the configuration files (and therefore you expect them to know what they're
doing, and how this stuff works), but not if you think that Kconfig users
should be presented with meaningful options when configuring their kernel.

^ permalink raw reply

* [PATCH 6/7] tty: of_serial: Add pinctrl support
From: Greg Kroah-Hartman @ 2013-01-21 23:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdYNOTcTVNMqD-q8sJ50nuwoXZLUJZpMXWa=mrZGnGY9+g@mail.gmail.com>

On Mon, Jan 21, 2013 at 11:13:55PM +0100, Linus Walleij wrote:
> On Fri, Jan 18, 2013 at 10:30 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> 
> > Use pinctrl to configure the SoCs pins directly from the driver.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> You can poke Greg about this which is fun because it does become
> superfluous with the patch titled
> "[PATCH v4] drivers/pinctrl: grab default handles from device core"
> http://marc.info/?l=linux-kernel&m=135879594515932&w=2
> 
> Which will auto-grab the default state on any device.
> 
> Greg, can I have your ACK on that device core grab patch
> so you don't have to merge this patch but can point out that
> it solves the boilerplate problem?

Yes you now do.

greg k-h

^ permalink raw reply

* [PATCH v4] drivers/pinctrl: grab default handles from device core
From: Greg Kroah-Hartman @ 2013-01-21 23:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1358795855-21064-1-git-send-email-linus.walleij@stericsson.com>

On Mon, Jan 21, 2013 at 08:17:35PM +0100, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This makes the device core auto-grab the pinctrl handle and set
> the "default" (PINCTRL_STATE_DEFAULT) state for every device
> that is present in the device model right before probe. This will
> account for the lion's share of embedded silicon devcies.
> 
> A modification of the semantics for pinctrl_get() is also done:
> previously if the pinctrl handle for a certain device was already
> taken, the pinctrl core would return an error. Now, since the
> core may have already default-grabbed the handle and set its
> state to "default", if the handle was already taken, this will
> be disregarded and the located, previously instanitated handle
> will be returned to the caller.
> 
> This way all code in drivers explicitly requesting their pinctrl
> handlers will still be functional, and drivers that want to
> explicitly retrieve and switch their handles can still do that.
> But if the desired functionality is just boilerplate of this
> type in the probe() function:
> 
> struct pinctrl  *p;
> 
> p = devm_pinctrl_get_select_default(&dev);
> if (IS_ERR(p)) {
>    if (PTR_ERR(p) == -EPROBE_DEFER)
>         return -EPROBE_DEFER;
>         dev_warn(&dev, "no pinctrl handle\n");
> }
> 
> The discussion began with the addition of such boilerplate
> to the omap4 keypad driver:
> http://marc.info/?l=linux-input&m=135091157719300&w=2
> 
> A previous approach using notifiers was discussed:
> http://marc.info/?l=linux-kernel&m=135263661110528&w=2
> This failed because it could not handle deferred probes.
> 
> This patch alone does not solve the entire dilemma faced:
> whether code should be distributed into the drivers or
> if it should be centralized to e.g. a PM domain. But it
> solves the immediate issue of the addition of boilerplate
> to a lot of drivers that just want to grab the default
> state. As mentioned, they can later explicitly retrieve
> the handle and set different states, and this could as
> well be done by e.g. PM domains as it is only related
> to a certain struct device * pointer.
> 
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Mitch Bradley <wmb@firmworks.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Rickard Andersson <rickard.andersson@stericsson.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Feel free to take this through your tree.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v7 09/15] gpio: pl061: set initcall level to module init
From: Haojian Zhuang @ 2013-01-21 23:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1358785488.6590.33.camel@hornet>

On 22 January 2013 00:24, Pawel Moll <pawel.moll@arm.com> wrote:
> On Mon, 2013-01-21 at 14:41 +0000, Linus Walleij wrote:
>> On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang
>> <haojian.zhuang@linaro.org> wrote:
>>
>> > Replace subsys initcall by module initcall level. Since pinctrl
>> > driver is already launched before gpio driver. It's unnecessary
>> > to set gpio driver in subsys init call level.
>> >
>> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>
>> On you platform maybe it works, but have you made sure that nobody
>> else will be affected?
>>
>> SPEAr of course, then these:
>>
>> arch/arm/mach-realview/core.c:           * GPIO on PL061 is active,
>> which is the proper
>> arch/arm/mach-socfpga/Kconfig:  select GPIO_PL061 if GPIOLIB
>>
>> Pawel, Dinh: are you OK with this change?
>
> Hm. Doesn't this make the MMCI probing depending on the module_init
> order? As in: wouldn't it make the mmci probe completely fail (not even
> defer it) if it was called before the pl061? In that case even your,
> Linus, hack with inverting the CD status wouldn't work...
>
> Pawel
>
>
>
The sequence of module probe is pinctrl --> gpio --> mmc. So the dependance
of mmc on gpio isn't broken.

Regards
Haojian

^ permalink raw reply

* [PATCH V2 1/3] gpio: pca953x: make the register access by GPIO bank
From: Gregory CLEMENT @ 2013-01-21 23:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50EBDFAD.2090903@free-electrons.com>

On 01/08/2013 09:58 AM, Gregory CLEMENT wrote:
> On 01/08/2013 09:32 AM, Maxime Ripard wrote:
>> Hi Gregory,
> 
> Hi Maxime,
> 
> thanks for testing
>>
>> On 07/01/2013 23:51, Gregory CLEMENT wrote:
>>> -static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
>>> +static int pca953x_read_single(struct pca953x_chip *chip, int reg, u32 *val,
>>> +				int off)
>>> +{
>>> +	int ret;
>>> +	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
>>> +	int offset = off / BANK_SZ;
>>> +
>>> +	ret = i2c_smbus_read_byte_data(chip->client,
>>> +				(reg << bank_shift) + offset);
>>> +	*val = ret;
>>> +
>>> +	if (ret < 0) {
>>> +		dev_err(&chip->client->dev, "failed reading register\n", reg);
>>
>> This triggers a warning, since you have an argument but nothing corresponding to it in your format string.
> 
> OK this one is easy, I didn't finished to clean up the message.
> 
>>
>>>  static irqreturn_t pca953x_irq_handler(int irq, void *devid)
>>>  {
>>>  	struct pca953x_chip *chip = devid;
>>> -	u32 pending;
>>> -	u32 level;
>>> +	u8 pending[MAX_BANK];
>>> +	u8 level;
>>> +	int i;
>>>  
>>> -	pending = pca953x_irq_pending(chip);
>>> -
>>> -	if (!pending)
>>> +	if (!pca953x_irq_pending(chip, pending))
>>>  		return IRQ_HANDLED;
>>>  
>>> -	do {
>>> -		level = __ffs(pending);
>>> -		handle_nested_irq(irq_find_mapping(chip->domain, level));
>>> -
>>> -		pending &= ~(1 << level);
>>> -	} while (pending);
>>> +	for (i = 0; i < NBANK(chip); i++) {
>>> +		do {
>>> +			level = __ffs(pending[i]);
>>> +			handle_nested_irq(irq_find_mapping(chip->domain,
>>> +							level + (BANK_SZ * i)));
>>> +			pending[i] &= ~(1 << level);
>>> +		} while (pending[i]);
>>> +	}
>>>  
>>>  	return IRQ_HANDLED;
>>>  }
>>
>> This triggers the following warning when an interrupt is raised:
>>
>> [   30.773500] ------------[ cut here ]------------
>> [   30.778843] WARNING: at /home/tmp/linux/kernel/irq/irqdomain.c:137 irq_domain_legacy_revmap+0x2c/0x48()
>> [   30.788375] Modules linked in:
>> [   30.791531] [<c0014100>] (unwind_backtrace+0x0/0xf0) from [<c001c878>] (warn_slowpath_common+0x4c/0x64)
>> [   30.801125] [<c001c878>] (warn_slowpath_common+0x4c/0x64) from [<c001c8ac>] (warn_slowpath_null+0x1c/0x24)
>> [   30.810968] [<c001c8ac>] (warn_slowpath_null+0x1c/0x24) from [<c006b5e0>] (irq_domain_legacy_revmap+0x2c/0x48)
>> [   30.821187] [<c006b5e0>] (irq_domain_legacy_revmap+0x2c/0x48) from [<c01c3734>] (pca953x_irq_handler+0x16c/0x1ac)
>> [   30.831656] [<c01c3734>] (pca953x_irq_handler+0x16c/0x1ac) from [<c0068688>] (irq_thread+0xd0/0x124)
>> [   30.841000] [<c0068688>] (irq_thread+0xd0/0x124) from [<c003c238>] (kthread+0xa4/0xb0)
>> [   30.849125] [<c003c238>] (kthread+0xa4/0xb0) from [<c000ed48>] (ret_from_fork+0x14/0x2c)
>> [   30.857375] ---[ end trace 09584b7a73100a49 ]---
>>
> 
> Humm it seems that this warning is caused by attempting to use an out of range hwirq.
> I need to investigate it more in details.

Unfortunately I didn't receive information on Mirabox to be able to use
interrupt. So I will need you help to debug this part.

Could you try the branch gpio-pca9505-debug  at
https://github.com/MISL-EBU-System-SW/mainline-public.git ?

I suspect that we use a GPIO above 8 to generate the interrupt, am I right?

Thanks,

Gregory

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-21 23:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121231318.GC23505@n2100.arm.linux.org.uk>

On Mon, Jan 21, 2013 at 5:13 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 21, 2013 at 04:54:31PM -0600, Matt Sealey wrote:
>> Hmm, I think it might be appreciated for people looking at this stuff
>> (same as I stumbled into it) for a little comment on WHY the default
>> is 200. That way you don't wonder even if you know why EBSA110 has a
>> HZ=200 default, why Exynos is lumped in there too (to reduce the
>> number of interrupts firing?
>
> Err, _reduce_ ?
>
> Can you please explain why changing HZ from 100 to 200 is a reduction?

We were talking about HZ=1000 at the time, sorry...

>> Maybe the Exynos timer interrupt is kind
>> of a horrid core NMI kind of thing and it's desirable for it not to be
>> every millisecond,
>
> Huh?  HZ=100 is centisecond intervals...

See above..

> I think you're understanding is just waaaayyyyy off.  That default is
> there because that is the _architecture_ _default_ and there _has_ to
> be a default.  No, including kernel/Kconfig.hz won't give us any kind
> of non-specified default because, as I've already said in one of my
> other mails, you can't supplement Kconfig symbol definitions by
> declaring it multiple times.

Okay, so the real



>> I know where the 60Hz clocksource might come from, the old Amiga
>> platforms have one based on the PSU frequency (50Hz in Europe, 60Hz
>> US/Japan). Even a 60Hz clocksource is useful though (on the Amiga, at
>> least, it is precisely the vsync clock for synchronizing your display
>> output on TV-out, which makes it completely useful for the framebuffer
>> driver), but.. you just won't expect to assign it as sched_clock or
>> your delay timer. And if anyone does I'd expect they'd know full well
>> it'd not run so well.
>
> Except in the UK where it'd be 50Hz for the TV out.  (Lengthy irrelevant
> explanation why this is so for UK cut.)

Read again: "50Hz in Europe".

Australia too. I'm British and I used to have more EU-manufactured
Amigas than I knew what to do with.. so.. just like your NTP story, I
definitely know this already.

>> >From that description, we are booting with standard HZ on ARM, and the
>> core sched_clock (as in we can call setup_sched_clock)
>> and/or/both/optionally using a real delay_timer switch to HRT mode if
>> we have the right equipment available in the kernel and at runtime on
>> the SoC.. but the process scheduler isn't compiled with the means to
>> actually take advantage of us being in HRT mode?
>
> Don't mix sched_clock() into this; it has nothing to do with HZ at all.
> You're confusing your apples with your oranges.

Okay..

>> A simple BUILD_BUG_ON and a BUG_ON right after each other in the
>> appropriate clocksource driver solves that.. if there's an insistence
>> on having at least some rope, we can put them in a field and tell them
>> they have to use the moon to actually hang themselves...
>
> No it doesn't - it introduces a whole load of new ways to make the
> kernel build or boot fail for pointless reasons - more failures, more
> regressions.
>
> No thank you.

But it would effectively stop users drinking kool-aid.. if you set
your HZ to something stupid, you don't even get a kernel to build, and
certainly don't get to boot past the first 40 lines of boot messages..
I think most people would rather a build error, or a runtime
unmistakable, unmissable warning than a subtle and almost
imperceptible skew in NTP synchronization, to use your example.

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* [PATCH 1/6] arm: mvebu: Add support for local interrupt
From: Ezequiel Garcia @ 2013-01-21 23:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50FDBC0E.1030706@free-electrons.com>

Hi Thomas and Gregory,

On Mon, Jan 21, 2013 at 11:07:10PM +0100, Gregory CLEMENT wrote:
> On 01/21/2013 07:17 PM, Thomas Petazzoni wrote:
> >> +		irq_set_percpu_devid(virq);
> >> +		irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
> >> +					handle_percpu_devid_irq);
> >> +
> >> +	} else {
> >> +		irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
> >> +					handle_level_irq);
> >> +	}
> > 
> > Braces useless since there is only one statement in the else.
> > 

IMHO, this is an exception to the rule.
Since the first block is more than one line,
we usually put braces on the single line block too.
(or at least that's what Documentation/CodingStyle says).

Regards,

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-21 23:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121224252.GY23505@n2100.arm.linux.org.uk>

On Mon, Jan 21, 2013 at 4:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 21, 2013 at 04:20:14PM -0600, Matt Sealey wrote:
>> I am sorry it sounded if I was being high and mighty about not being
>> able to select my own HZ (or being forced by Exynos to be 200 or by
>> not being able to test an Exynos board, forced to default to 100). My
>> real "grievance" here is we got a configuration item for the scheduler
>> which is being left out of ARM configurations which *can* use high
>> resolution timers, but I don't know if this is a real problem or not,
>> hence asking about it, and that HZ=100 is the ARM default whether we
>> might be able to select that or not.. which seems low.
>
> Well, I have a versatile platform here.  It's the inteligence behind
> the power control system for booting the boards on the nightly tests
> (currently disabled because I'm waiting for my main server to lock up
> again, and I need to use one of the serial ports for that.)
>
> The point is, it talks via I2C to a load of power monitors to read
> samples out.  It does this at sub-100Hz intervals.  Yet the kernel is
> built with HZ=100.  NO_HZ=y and highres timers are enabled... works
> fine.
>
> So, no, HZ=100 is not a limit in that scenario.  With NO_HZ=y and
> highres timers, it all works with epoll() - you get the interval that
> you're after.  I've verified this with calls to gettimeofday() and
> the POSIX clocks.

Okay.

So, can you read this (it's short):

http://ck.kolivas.org/patches/bfs/bfs-configuration-faq.txt

And please tell me if he's batshit crazy and I should completely
ignore any scheduler discussion that isn't ARM-specific, or maybe..
and I can almost guarantee this, he doesn't have an ARM platform so
he's just delightfully ill-informed about anything but his quad-core
x86?

>> HZ=250 is the "current" kernel default if you don't touch anything, it
>> seems, apologies for thinking it was HZ=100.
>
> Actually, it always used to be 100Hz on everything, including x86.
> It got upped when there were interactivity issues... which haven't
> been reported on ARM - so why change something that we know works and
> everyone is happy with?

I don't know. I guess this is why I included Ingo and Peter as they
seem to be responsible for core HZ-related things; why have HZ=250 on
x86 when CONFIG_NO_HZ and HZ=100 would work just as effectively? Isn't
CONFIG_NO_HZ the default on x86 and PPC and.. pretty much everything
else?

I know Con K. has been accused many times of peddling snake-oil... but
he has pretty graphs and benchmarks that kind of bear him out on most
things even if the results do not get his work upstream. I can't fault
the statistical significance of his results.. but even a placebo
effect can be graphed, correlation is not causation, etc, etc. - I
don't know if anything real filters down into the documentation
though.

>> And that is too high for
>> EBSA110 and a couple of other boards, especially where HZ must equal
>> some exact divisor being pumped right into some timer unit.
>
> EBSA110 can do 250Hz, but it'll mean manually recalculating the timer
> arithmetic - because it's not a "reloading" counter - software has to
> manually reload it, and you have to take account of how far it's
> rolled over to get anything close to a regular interrupt rate which
> NTP is happy with.  And believe me, it used to be one of two main NTP
> broadcasting servers on my network, so I know it works.

A-ha...

>> Anyway, a patch for ARM could perhaps end up like this:
>>
>> ~~
>> if ARCH_MULTIPLATFORM
>> source kernel/Kconfig.hz
>> else
>> HZ
>>     default 100
>> endif
>>
>> HZ
>>     default 200 if ARCH_EBSA110 || ARCH_ETC_ETC || ARCH_UND_SO_WEITER
>>     # any previous platform definitions where *really* required here.
>>     # but not default 100 since it would override kernel/Kconfig.hz every time
>
> That doesn't work - if you define the same symbol twice, one definition
> takes priority over the other (I don't remember which way it works).
> They don't accumulate.

Well I did some testing.. a couple days of poking around, and they
don't need to accumulate.

> Because... it simply doesn't work like that.  Try it and check to see
> what Kconfig produces.

I did test it.. whatever you define last, sticks, and it's down to the
order they're parsed in the tree - luckily, arch/arm/Kconfig is
sourced first, which sources the mach/plat stuff way down at  the
bottom. As long as you have your "default" set somewhere, any further
default just has to be sourced or added later in *one* of the
Kconfigs, same as building any C file with "gcc -E" and spitting it
out.

Someone, at the end of it all, has to set some default, and as long as
the one you want is the last one, everything is shiny.

> We know this, because our FRAME_POINTER config overrides the generic
> one - not partially, but totally and utterly in every way.

But for something as simple as CONFIG_HZ getting a value.. it works
okay. If Kconfig.hz sets CONFIG_HZ=250 because CONFIG_HZ_250 is
default yes, and it CONFIG_HZ defaults to 250 if it's set, and then
you put

HZ
   default 100

Right after it, or right after it's source in arch/x86/Kconfig, or
whatever, that "default" is what sticks and what ends up in CONFIG_HZ
in the local .config.

> I just don't see how that's remotely possible.

Maybe I tested it wrong, you'd know better than I exactly how (and I
would appreciate knowing how so I can go back and test it again :)

--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Tony Lindgren @ 2013-01-21 23:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121210341.GW23505@n2100.arm.linux.org.uk>

* Russell King - ARM Linux <linux@arm.linux.org.uk> [130121 13:07]:
> 
> As for Samsung and the rest I can't comment.  The original reason OMAP
> used this though was because the 32768Hz counter can't produce 100Hz
> without a .1% error - too much error under pre-clocksource
> implementations for timekeeping.  Whether that's changed with the
> clocksource/clockevent support needs to be checked.

Yes that's why HZ was originally set to 128. That value (or some multiple)
still makes sense when the 32 KiHZ clock source is being used. Of course
we should rely on the local timer when running for the SoCs that have
them.

Regards,

Tony

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Russell King - ARM Linux @ 2013-01-21 23:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKGA1bmtTH1N8nx79MGiMighdQROpNtpvAZXhHKT+mT3kcazBQ@mail.gmail.com>

On Mon, Jan 21, 2013 at 04:54:31PM -0600, Matt Sealey wrote:
> Hmm, I think it might be appreciated for people looking at this stuff
> (same as I stumbled into it) for a little comment on WHY the default
> is 200. That way you don't wonder even if you know why EBSA110 has a
> HZ=200 default, why Exynos is lumped in there too (to reduce the
> number of interrupts firing?

Err, _reduce_ ?

Can you please explain why changing HZ from 100 to 200 is a reduction?

> Maybe the Exynos timer interrupt is kind
> of a horrid core NMI kind of thing and it's desirable for it not to be
> every millisecond,

Huh?  HZ=100 is centisecond intervals...

> or maybe it has the same restrictions as EBSA110,
> but where would anyone go to find out this information?)

Maybe the mailing list archives.  No, not these ones.  The full ones
on lists.arm.linux.org.uk.  The lurker archives contain every email
that has been on these mailing lists stretching back into the late
1990s.  They are the only _full_ archives (give or take a few problems
with connectivity between lists.arm.linux.org.uk and lists.infradead.org
throwing the archiver subscription off.)

> I think then the default 100 at the end of the arch/arm/Kconfig is
> saying "you are not allowed to know that such a thing as rope even
> exists," when in fact what we should be doing is just making sure they
> can't swing it over the rafters.. am I taking the analogy too far? :)

I think you're understanding is just waaaayyyyy off.  That default is
there because that is the _architecture_ _default_ and there _has_ to
be a default.  No, including kernel/Kconfig.hz won't give us any kind
of non-specified default because, as I've already said in one of my
other mails, you can't supplement Kconfig symbol definitions by
declaring it multiple times.

> I know where the 60Hz clocksource might come from, the old Amiga
> platforms have one based on the PSU frequency (50Hz in Europe, 60Hz
> US/Japan). Even a 60Hz clocksource is useful though (on the Amiga, at
> least, it is precisely the vsync clock for synchronizing your display
> output on TV-out, which makes it completely useful for the framebuffer
> driver), but.. you just won't expect to assign it as sched_clock or
> your delay timer. And if anyone does I'd expect they'd know full well
> it'd not run so well.

Except in the UK where it'd be 50Hz for the TV out.  (Lengthy irrelevant
explanation why this is so for UK cut.)

> >From that description, we are booting with standard HZ on ARM, and the
> core sched_clock (as in we can call setup_sched_clock)
> and/or/both/optionally using a real delay_timer switch to HRT mode if
> we have the right equipment available in the kernel and at runtime on
> the SoC.. but the process scheduler isn't compiled with the means to
> actually take advantage of us being in HRT mode?

Don't mix sched_clock() into this; it has nothing to do with HZ at all.
You're confusing your apples with your oranges.

> A simple BUILD_BUG_ON and a BUG_ON right after each other in the
> appropriate clocksource driver solves that.. if there's an insistence
> on having at least some rope, we can put them in a field and tell them
> they have to use the moon to actually hang themselves...

No it doesn't - it introduces a whole load of new ways to make the
kernel build or boot fail for pointless reasons - more failures, more
regressions.

No thank you.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-21 23:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130121224526.GA23505@n2100.arm.linux.org.uk>

On Mon, Jan 21, 2013 at 4:45 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 21, 2013 at 10:30:07PM +0000, Arnd Bergmann wrote:
>> On Monday 21 January 2013, Matt Sealey wrote:
>> > So is that a bug in that it is not available to ARM right now, a bug
>> > in that it would be impossible for anyone on ARM to have ever tested
>> > this code, or a bug in that it should NEVER be enabled for ARM for
>> > some reason? John? Ingo? :)
>> >
>>
>> I think it's a bug that it's not available. That does not look intentional.
>
> What's a bug?  kernel/Kconfig.hz not being available?  No, it's
> intentional.  (See my replies).

The bug I saw as real is that CONFIG_SCHED_HRTICK is defined only in
kernel/Kconfig.hz (and used in kernel/sched only) - so if we want that
functionality enabled we will also have to opencode it in
arch/arm/Kconfig. Everyone else, by virtue of using kernel/Kconfig.hz,
gets this config item enabled for free if they have hrtimers or
generic smp helpers.. if I understood what John just said, this means
on ARM, since we don't use kernel/Kconfig.hz and we don't also define
an item for CONFIG_SCHED_HRTICK, the process scheduler is completely
oblivious that we're running in HRT mode?

The thing I don't know is real is if that really matters one bit..

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Matt Sealey @ 2013-01-21 22:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50FDC2DD.7090406@linaro.org>

On Mon, Jan 21, 2013 at 4:36 PM, John Stultz <john.stultz@linaro.org> wrote:
> On 01/21/2013 01:14 PM, Matt Sealey wrote:
>>
>> On Mon, Jan 21, 2013 at 3:00 PM, John Stultz <john.stultz@linaro.org>
>> wrote:
>>>
>>> On 01/21/2013 12:41 PM, Arnd Bergmann wrote:
>>>>
>>>> Right. It's pretty clear that the above logic does not work
>>>> with multiplatform.  Maybe we should just make ARCH_MULTIPLATFORM
>>>> select NO_HZ to make the question much less interesting.
>>>
>>> Although, even with NO_HZ, we still have some sense of HZ.
>>
>> In this case, no matter whether CONFIG_HZ=1000 or CONFIG_HZ=250 (for
>> example) combined with CONFIG_NO_HZ and less than e.g. 250 things
>> happening per second will wake up "exactly" the same number of times?
>
> Ideally, if both systems are completely idle, they may see similar number of
> actual interrupts.
>
> But when the cpus are running processes, the HZ=1000 system will see more
> frequent interrupts, since the timer/scheduler interrupt will jump in 4
> times more frequently.

Understood..

>> CONFIG_HZ=1000 with CONFIG_NO_HZ would be an effective, all-round
>> solution here, then, and CONFIG_HZ=100 should be a reasonable default
>> (as it is anyway with an otherwise-unconfigured kernel on any other
>> platform) for !CONFIG_NO_HZ.
>
> Eeehhh... I'm not sure this is follows.

Okay, I'm happy to be wrong on this...

>> As above, or "not select anything at all" since HZ=100 if you don't
>> touch anything, right?
>
> Well, Russell brought up a case that doesn't handle this. If a system
> *can't* do HZ=100, but can do HZ=200.
>
> Though there are hacks, of course, that might get around this (skip every
> other interrupt at 200HZ).

Hmm, I think it might be appreciated for people looking at this stuff
(same as I stumbled into it) for a little comment on WHY the default
is 200. That way you don't wonder even if you know why EBSA110 has a
HZ=200 default, why Exynos is lumped in there too (to reduce the
number of interrupts firing? Maybe the Exynos timer interrupt is kind
of a horrid core NMI kind of thing and it's desirable for it not to be
every millisecond, or maybe it has the same restrictions as EBSA110,
but where would anyone go to find out this information?)

>> If someone picks HZ=1000 and their platform can't support it, then
>> that's their own damn problem (don't touch things you don't
>> understand, right? ;)
>
> Well, ideally with kconfig we try to add proper dependencies so impossible
> options aren't left to the user.
> HZ is a common enough knob to turn on most systems, I don't know if leaving
> the user rope to hang himself is a great idea.

I think then the default 100 at the end of the arch/arm/Kconfig is
saying "you are not allowed to know that such a thing as rope even
exists," when in fact what we should be doing is just making sure they
can't swing it over the rafters.. am I taking the analogy too far? :)

>> My question really has to be is CONFIG_SCHED_HRTICK useful, what
>> exactly is it going to do on ARM here since nobody can ever have
>> enabled it? Is it going to keel over and explode if nobody registers a
>> non-jiffies sched_clock (since the jiffies clock is technically
>> reporting itself as a ridiculously high resolution clocksource..)?
>
> ??? Not following this at all.  jiffies is the *MOST* coarse resolution
> clocksource there is (at least that I'm aware of.. I recall someone wanting
> to do a 60Hz clocksource, but I don't think that ever happened).

Is that based on it's clocksource rating (probably worse than a real
hrtimer) or it's reported resolution? Because on i.MX51 if I force it
to use the jiffies clock the debug on the kernel log is telling me it
has a higher resolution (it TELLS me that it ticks "as fast" as the
CPU frequency and wraps less than my real timer).

I know where the 60Hz clocksource might come from, the old Amiga
platforms have one based on the PSU frequency (50Hz in Europe, 60Hz
US/Japan). Even a 60Hz clocksource is useful though (on the Amiga, at
least, it is precisely the vsync clock for synchronizing your display
output on TV-out, which makes it completely useful for the framebuffer
driver), but.. you just won't expect to assign it as sched_clock or
your delay timer. And if anyone does I'd expect they'd know full well
it'd not run so well.

>> Or is this one of those things that if your platform doesn't have a
>> real high resolution timer, you shouldn't enable HRTIMERS and
>> therefore not enable SCHED_HRTICK as a result? That affects
>> ARCH_MULTIPLATFORM here. Is the solution as simple as
>> ARCH_MULTIPLATFORM compliant platforms kind of have to have a high
>> resolution timer? Documentation to that effect?
>
> SO HRITMERS was designed to be be build time enabled, while still giving you
> a functioning system if it was booted on a system that didn't support
> clockevents.  We boot with standard HZ, and only switch over to HRT mode if
> we have a proper clocksource and clockevent driver.

Okay. I'm still a little confused as to what SCHED_HRTICK actually
makes a difference to, though.

>From that description, we are booting with standard HZ on ARM, and the
core sched_clock (as in we can call setup_sched_clock)
and/or/both/optionally using a real delay_timer switch to HRT mode if
we have the right equipment available in the kernel and at runtime on
the SoC.. but the process scheduler isn't compiled with the means to
actually take advantage of us being in HRT mode?

> However, HRTIMERS or NOHZ doesn't fix the case of having a system boot with
> HZ=1000 or HZ=100 if the system can *only* do HZ=200.

A simple BUILD_BUG_ON and a BUG_ON right after each other in the
appropriate clocksource driver solves that.. if there's an insistence
on having at least some rope, we can put them in a field and tell them
they have to use the moon to actually hang themselves...

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply

* One of these things (CONFIG_HZ) is not like the others..
From: Russell King - ARM Linux @ 2013-01-21 22:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50FDC2DD.7090406@linaro.org>

On Mon, Jan 21, 2013 at 02:36:13PM -0800, John Stultz wrote:
> Well, Russell brought up a case that doesn't handle this. If a system  
> *can't* do HZ=100, but can do HZ=200.
>
> Though there are hacks, of course, that might get around this (skip  
> every other interrupt at 200HZ).

Note: in the early days of EBSA110 support, yes, we did that, so that
we could have HZ=100 everywhere.  _However_ it sufficiently peturbed
NTP that it basically was unable to slew the clock in any sane manner.
I never got to the bottom of why that was, and when USER_HZ was
decoupled from the kernel HZ, it allowed the problem to be fixed, and
the kernel code to become a _lot_ cleaner.

^ permalink raw reply

* [PATCH] crypto: fix FTBFS with ARM SHA1-asm and THUMB2_KERNEL
From: Nicolas Pitre @ 2013-01-21 22:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1358787763-1226-1-git-send-email-matt@genesi-usa.com>

On Mon, 21 Jan 2013, Matt Sealey wrote:

> The optimized assembler SHA1 code for ARM does not conform to Thumb2
> register usage requirements, so it cannot be built when the kernel is
> configured with THUMB2_KERNEL.
> 
> Fix the FTBFS for now by preventing misconfigurations of the kernel.
> 
> Signed-off-by: Matt Sealey <matt@genesi-usa.com>

A .arm directive at the top of the assembly code would be a better 
"fix", as that wouldn't reduce functionality.

Yet, I'd invite you to have a look at commit 638591cd7b in linux-next.


Nicolas

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox