Re: m68k using deprecated internal APIs?
On 17/11/18 5:44 am, Geert Uytterhoeven wrote: Hi Linus, On Fri, Nov 16, 2018 at 12:13 PM Linus Walleij wrote: On Fri, Nov 16, 2018 at 1:31 AM Finn Thain wrote: On Wed, 14 Nov 2018, Linus Walleij wrote: Apart from this (which is the most important step!) I think the custom LED heartbeat code in kernel/time.c needs to be replaced with a standard drivers/leds driver for each LED using the "heartbeat" trigger as is custom these days. That should clean out another chunk of legacy time-related code. Are you referring to LED heartbeat code in arch/m68k/kernel/time.c? I suppose you are currently keeping the call to timer_interrupt() for exactly this reason (i.e. keep the heartbeat LED blinking)? It would be great to have that call inlined, which the compiler can't do at the moment, because timer_interrupt() is in a different compilation unit (arch/m68k/kernel/time.c). Is there some other benefit to eliminating the call to timer_interrupt() that I've overlooked? I mean that whole thing should go away by abstracting those LEDs (for the systems that have them) using the struct led_classdev, populating a proper platform device for it and instantiate using a driver in drivers/leds/*, and the function to provide the heartbeat be replaced with the existing heartbeat trigger in drivers/leds/trigger/ledtrig-heartbeat.c assigned as default trigger for that LED. I think that is WAY out of the focus for your current work (which, by the way, is a piece of art) but more something for the m68k maintainers to look into. Just going with struct led_classdev is probably doable. Going for the full monty, using leds-gpio, probably requires moving m68k to DT. Which would not be that ... uninteresting ;-) I have been thinking about a move to DT for a while for ColdFire. There are lots of common hardware device blocks that would fit really neatly into a DT framework. Regards Greg
Re: m68k using deprecated internal APIs?
Hi Linus,
On Fri, Nov 16, 2018 at 10:33 PM Linus Walleij wrote:
> On Fri, Nov 16, 2018 at 8:44 PM Geert Uytterhoeven
> wrote:
> > On Fri, Nov 16, 2018 at 12:13 PM Linus Walleij
> > wrote:
> > > I mean that whole thing should go away by abstracting those LEDs
> > > (for the systems that have them) using the struct led_classdev,
> > > populating a proper platform device for it and instantiate using
> > > a driver in drivers/leds/*, and the function to provide the heartbeat
> > > be replaced with the existing heartbeat trigger in
> > > drivers/leds/trigger/ledtrig-heartbeat.c assigned as default
> > > trigger for that LED.
> > >
> > > I think that is WAY out of the focus for your current work (which,
> > > by the way, is a piece of art) but more something for the m68k
> > > maintainers to look into.
> >
> > Just going with struct led_classdev is probably doable.
>
> Would be nice.
>
> > Going for the full monty, using leds-gpio, probably requires moving m68k
> > to DT. Which would not be that ... uninteresting ;-)
>
> If the line with the LED is not general purpose but an actual register
> bit for the LED it should not be using legs-gpio anyway.
On Amiga, the line is connected to a CIA.
In modern parlor, that would be a system-controller, providing a GPIO
controller, SPI (microwire) controller, clock controller, clock source,
two clock events, and an interrupt controller.
> But something similar to Russells neat drivers/gpio/gpio-reg.c
> but for LED classdevs in drivers/leds/leds-reg.c would be
> a really tempting option I think.
Thanks, I'll have a look...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: m68k using deprecated internal APIs?
On Fri, Nov 16, 2018 at 8:44 PM Geert Uytterhoeven wrote: > On Fri, Nov 16, 2018 at 12:13 PM Linus Walleij > wrote: > > I mean that whole thing should go away by abstracting those LEDs > > (for the systems that have them) using the struct led_classdev, > > populating a proper platform device for it and instantiate using > > a driver in drivers/leds/*, and the function to provide the heartbeat > > be replaced with the existing heartbeat trigger in > > drivers/leds/trigger/ledtrig-heartbeat.c assigned as default > > trigger for that LED. > > > > I think that is WAY out of the focus for your current work (which, > > by the way, is a piece of art) but more something for the m68k > > maintainers to look into. > > Just going with struct led_classdev is probably doable. Would be nice. > Going for the full monty, using leds-gpio, probably requires moving m68k > to DT. Which would not be that ... uninteresting ;-) If the line with the LED is not general purpose but an actual register bit for the LED it should not be using legs-gpio anyway. But something similar to Russells neat drivers/gpio/gpio-reg.c but for LED classdevs in drivers/leds/leds-reg.c would be a really tempting option I think. Yours, Linus Walleij
Re: m68k using deprecated internal APIs?
Hi Linus,
On Fri, Nov 16, 2018 at 12:13 PM Linus Walleij wrote:
> On Fri, Nov 16, 2018 at 1:31 AM Finn Thain wrote:
> > On Wed, 14 Nov 2018, Linus Walleij wrote:
> > > Apart from this (which is the most important step!) I think the custom
> > > LED heartbeat code in kernel/time.c needs to be replaced with a standard
> > > drivers/leds driver for each LED using the "heartbeat" trigger as is
> > > custom these days.
> > >
> > > That should clean out another chunk of legacy time-related code.
> >
> > Are you referring to LED heartbeat code in arch/m68k/kernel/time.c?
> >
> > > I suppose you are currently keeping the call to timer_interrupt() for
> > > exactly this reason (i.e. keep the heartbeat LED blinking)?
> >
> > It would be great to have that call inlined, which the compiler can't do
> > at the moment, because timer_interrupt() is in a different compilation
> > unit (arch/m68k/kernel/time.c).
> >
> > Is there some other benefit to eliminating the call to timer_interrupt()
> > that I've overlooked?
>
> I mean that whole thing should go away by abstracting those LEDs
> (for the systems that have them) using the struct led_classdev,
> populating a proper platform device for it and instantiate using
> a driver in drivers/leds/*, and the function to provide the heartbeat
> be replaced with the existing heartbeat trigger in
> drivers/leds/trigger/ledtrig-heartbeat.c assigned as default
> trigger for that LED.
>
> I think that is WAY out of the focus for your current work (which,
> by the way, is a piece of art) but more something for the m68k
> maintainers to look into.
Just going with struct led_classdev is probably doable.
Going for the full monty, using leds-gpio, probably requires moving m68k
to DT. Which would not be that ... uninteresting ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: m68k using deprecated internal APIs?
On Fri, Nov 16, 2018 at 1:31 AM Finn Thain wrote: > On Wed, 14 Nov 2018, Linus Walleij wrote: > > Apart from this (which is the most important step!) I think the custom > > LED heartbeat code in kernel/time.c needs to be replaced with a standard > > drivers/leds driver for each LED using the "heartbeat" trigger as is > > custom these days. > > > > That should clean out another chunk of legacy time-related code. > > > > Are you referring to LED heartbeat code in arch/m68k/kernel/time.c? > > > I suppose you are currently keeping the call to timer_interrupt() for > > exactly this reason (i.e. keep the heartbeat LED blinking)? > > It would be great to have that call inlined, which the compiler can't do > at the moment, because timer_interrupt() is in a different compilation > unit (arch/m68k/kernel/time.c). > > Is there some other benefit to eliminating the call to timer_interrupt() > that I've overlooked? I mean that whole thing should go away by abstracting those LEDs (for the systems that have them) using the struct led_classdev, populating a proper platform device for it and instantiate using a driver in drivers/leds/*, and the function to provide the heartbeat be replaced with the existing heartbeat trigger in drivers/leds/trigger/ledtrig-heartbeat.c assigned as default trigger for that LED. I think that is WAY out of the focus for your current work (which, by the way, is a piece of art) but more something for the m68k maintainers to look into. Yours, Linus Walleij
Re: m68k using deprecated internal APIs?
On Wed, 14 Nov 2018, Linus Walleij wrote:
> > >
> > > You can see those patches here,
> > > https://github.com/fthain/linux/commits/mac68k-queue/
>
> This is looking very good. FWIW:
> Acked-by: Linus Walleij
>
Thanks for your review.
> Apart from this (which is the most important step!) I think the custom
> LED heartbeat code in kernel/time.c needs to be replaced with a standard
> drivers/leds driver for each LED using the "heartbeat" trigger as is
> custom these days.
>
> That should clean out another chunk of legacy time-related code.
>
Are you referring to LED heartbeat code in arch/m68k/kernel/time.c?
> I suppose you are currently keeping the call to timer_interrupt() for
> exactly this reason (i.e. keep the heartbeat LED blinking)?
>
It would be great to have that call inlined, which the compiler can't do
at the moment, because timer_interrupt() is in a different compilation
unit (arch/m68k/kernel/time.c).
Is there some other benefit to eliminating the call to timer_interrupt()
that I've overlooked?
> ...
> > Adding the timekeeping maintainers and Linus Walleij to Cc. Linus
> > worked on removing ARCH_USES_GETTIMEOFFSET on ARM several platforms in
> > the past.
> >
> > I noticed that the last timer rework involving
> > CONFIG_ARCH_USES_GETTIMEOFFSET was back in 2012 with commit
> > 7b1f62076bba ("time: convert arch_gettimeoffset to a pointer"). At the
> > time, we had cris, m32r, blackfin, m68k and lots of ARM platforms, now
> > it's only two StrongARM platforms (RPC and ARCH_EBSA110) and classic
> > m68k.
>
Adding the maintainer for those platforms to Cc...
> Believe it or not, I managed to procure both machines (RPC and EBSA110).
> Whether I can get them to boot Linux is still an open question.
>
> I suppose it would be possible to use a conversion strategy similar to
> Finn's and get rid of gettimeoffset altogether, if I could only test it
> on these boards.
>
I've dropped the ARM patch from my clocksource API patch series. As
Russell pointed out, it didn't actually fix the regression caused by
5cfc8ee0bb51.
I've also dropped the equivalent m68k patch, which was intended to address
4ad4c76b7afb. The best way to address that problem for -stable for m68k is
probably by backporting the clocksource API conversion.
Then there's the question of -stable fixes for the other architectures...
4f543fa41e78 alpha: convert to use arch_gettimeoffset()
95ad759c6b0f m32r: convert to use arch_gettimeoffset()
10f03f1a249d Blackfin: convert to use arch_gettimeoffset()
0299b1371d8f sparc: convert to arch_gettimeoffset()
ba875ba6b7cd cris: convert to use arch_gettimeoffset()
No doubt many of these have had a clocksource conversions already, and in
some cases even the oldest -stable branch may be unaffected because of
that. E.g.
9ce34c8f4466 Convert alpha to use clocksources instead of arch_gettimeoffset
--
> Yours.
> Linus Walleij
>
Re: m68k using deprecated internal APIs?
On Sat, Nov 10, 2018 at 10:47 PM Arnd Bergmann wrote:
> On Fri, Nov 9, 2018 at 12:42 AM Finn Thain wrote:
> > On Sun, 28 Oct 2018, Geert Uytterhoeven wrote:
> > > > > The example I gave was GENERIC_CLOCKEVENTS on m68, which is
> > > > > supported on most but not all machines there.
> > > >
> > > > That suggests that the removal of just those machines would suffice
> > > > (as opposed to the removal of the entire arch).
> > > >
> > > > Also, Documentation/features/time/clockevents/arch-support.txt says,
> > > > |m68k: | ok |
> > > >
> > > > These two observations make me wonder whether the clockevents feature
> > > > is related to the discussion quoted above (?)
> > >
> > > GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types.
> > >
> >
> > !GENERIC_CLOCKEVENTS implies PREEMPT_NONE, and disables the "Timers
> > subsystem" (i.e. the NO_HZ config options), but it works fine -- I was
> > able to convert the Mac port to !ARCH_USES_GETTIMEOFFSET &&
> > !GENERIC_CLOCKEVENTS. (Like many of the Coldfire CPU types.)
> >
> > You can see those patches here,
> > https://github.com/fthain/linux/commits/mac68k-queue/
This is looking very good. FWIW:
Acked-by: Linus Walleij
Apart from this (which is the most important step!) I think the custom
LED heartbeat code in kernel/time.c needs to be replaced with a standard
drivers/leds driver for each LED using the "heartbeat" trigger as is custom
these days.
That should clean out another chunk of legacy time-related code.
I suppose you are currently keeping the call to timer_interrupt() for
exactly this reason (i.e. keep the heartbeat LED blinking)?
> > Note that !ARCH_USES_GETTIMEOFFSET is a build-time choice, meaning that
> > all platforms need to be converted together.
> >
> > Also, some platforms will need to adopt the clocksource API, otherwise the
> > built-in "jiffies" clocksource will get used, causing a regression in
> > timer accuracy.
> >
> > Conversion to the clocksource API is straight-forward where the platform
> > already implements arch_gettimeoffset. The conversion is discussed in
> > include/linux/time.h:
> >
> > /* Some architectures do not supply their own clocksource.
> > * This is mainly the case in architectures that get their
> > * inter-tick times by reading the counter on their interval
> > * timer. Since these timers wrap every tick, they're not really
> > * useful as clocksources. Wrapping them to act like one is possible
> > * but not very efficient. So we provide a callout these arches
> > * can implement for use with the jiffies clocksource to provide
> > * finer then tick granular time.
> > */
> >
> > (Not sure what is meant by "not very efficient" here.)
> >
> > Certain 680x0 platforms do not implement arch_gettimeoffset: apollo, q40,
> > sun3, sun3x. These platforms can fall back on the "jiffies" clocksource
> > with no loss of timer accuracy, so conversion for these platforms is
> > trivial.
> >
> > Should I continue with the clocksource API conversion for the other
> > platforms: amiga, atari, bvme6000, hp300, mvme147, mvme16x? This would
> > allow for removal of "select ARCH_USES_GETTIMEOFFSET" (and satisfy
> > Documentation/features/time/modern-timekeeping) without loss of timer
> > accuracy.
If you ask me: forge ahead.
> > Alternatively, we could defer the clocksource API conversion, leaving it
> > up to the platform maintainers (who can actually test the driver changes).
> > But that would mean that many platforms would suffer a regression in timer
> > accuracy upon removal of arch_gettimeoffset.
>
> Adding the timekeeping maintainers and Linus Walleij to Cc. Linus
> worked on removing ARCH_USES_GETTIMEOFFSET on ARM
> several platforms in the past.
>
> I noticed that the last timer rework involving
> CONFIG_ARCH_USES_GETTIMEOFFSET was back in 2012 with
> commit 7b1f62076bba ("time: convert arch_gettimeoffset to a pointer").
> At the time, we had cris, m32r, blackfin, m68k and lots of ARM
> platforms, now it's only two StrongARM platforms (RPC and
> ARCH_EBSA110) and classic m68k.
Believe it or not, I managed to procure both machines (RPC and
EBSA110). Whether I can get them to boot Linux is still an open
question.
I suppose it would be possible to use a conversion strategy
similar to Finn's and get rid of gettimeoffset altogether, if I could
only test it on these boards.
Yours.
Linus Walleij
Re: m68k using deprecated internal APIs?
On Mon, 12 Nov 2018, Michael Schmitz wrote: > I'm wondering if disabling interrupts really is required when updating > the ticks counter in mfp_timer_handler, or whether READ_ONCE() and > WRITE_ONCE() would work as well. > I get the impression from Documentation/core-api/atomic_ops.rst that the author is opposed to that kind of usage of READ_ONCE() and WRITE_ONCE(). Apparently these operations are to be used solely for avoiding certain compiler optimizations. In this patch series I've used the same synchronization pattern for each platform for expediency. It is probably overkill in some places. But looking at the atari patch agin, I think there is a bug. In atari_read_clk(), clk_total is accessed outside the irq lock. But it is supposed to be synchronized with the timer interrupt handler and timer interrupt flag. I'll have to change this. > Note that there's a mfptimer_handler() in arch/m68k/atari/ataints.c > already (timer D, for polling interrupt-less hardware - I used to have > patches to adjust the rate of that timer at runtime...). Might be best > to rename the two (mfp_timerc_handler(), mfp_timerd_handler()) for > clarity. Or hook that timer function into the generic clocksource timer. > OK, I'll rename them. > > If I understand correctly, removing arch_gettimeoffset without adding > > a clocksource would reduce timer accuracy to 10 ms regardless of > > platform (because that's what the default jiffies clocksource offers). > That's my understanding. I now see that we could quite easily change the > timer C divisor to 10 while retaining the timer C data (246) and obtain > a clocksource rate of 2500. Setting that in the clocksource 'rating' > field will keep the scheduling timer at 100 Hz, is that correct? > The rating is arbitrary AFAICT. I gather that it just allows the clocksource core to select the best clocksource. The more accurate the clocksource, the higher its rating. You can see the available clocksources in dmesg. > With only the timer C interrupt running at increased rate, I don't think > the impact on the system would be all that severe. > If you need more timer accuracy, you'll need a faster oscillator driving the counter. But then you get counter overflow sooner, which may or may not be a problem. This patch series avoids changing any device configuration (clock rates, dividers, interrupt rates etc.) The exception is a VIA driver bug fix patch that is part of this series even though it is not related to the clocksource conversion. That patch got included just because of the mechanics of testing and merging patches. If you were to write a timer patch relevant to -stable, it could go before my series, and I could rebase. Otherwise I suggest that it go afterwards, either at the end of this series or submitted separately with a note about the dependency. -- > Cheers, > > ??? Michael > >
Re: m68k using deprecated internal APIs?
Hi Finn, Am 12.11.18 um 17:34 schrieb Finn Thain: > On Mon, 12 Nov 2018, Michael Schmitz wrote: > >> That's ultimately for Geert to decide - I'll yet have to look at your >> mac patches to even get a vague idea what this conversion involves, but >> I can certainly test whatever you came up with for Mac on Atari. >> > Thanks. I'll send out the latest patches for people to test. Thanks - I just had a look at the Atari clocksource implementation and it looks fine to me. I'm wondering if disabling interrupts really is required when updating the ticks counter in mfp_timer_handler, or whether READ_ONCE() and WRITE_ONCE() would work as well. Note that there's a mfptimer_handler() in arch/m68k/atari/ataints.c already (timer D, for polling interrupt-less hardware - I used to have patches to adjust the rate of that timer at runtime...). Might be best to rename the two (mfp_timerc_handler(), mfp_timerd_handler()) for clarity. Or hook that timer function into the generic clocksource timer. > >> Tick granularity is 40 us at best on Atari with the current timer >> configuration, and can't be increased without increasing HZ. I suspect >> the limitations on Mac are similar? >> > If I understand correctly, removing arch_gettimeoffset without adding a > clocksource would reduce timer accuracy to 10 ms regardless of platform > (because that's what the default jiffies clocksource offers). That's my understanding. I now see that we could quite easily change the timer C divisor to 10 while retaining the timer C data (246) and obtain a clocksource rate of 2500. Setting that in the clocksource 'rating' field will keep the scheduling timer at 100 Hz, is that correct? With only the timer C interrupt running at increased rate, I don't think the impact on the system would be all that severe. Cheers, Michael
Re: m68k using deprecated internal APIs?
On Mon, 12 Nov 2018, Michael Schmitz wrote: > > That's ultimately for Geert to decide - I'll yet have to look at your > mac patches to even get a vague idea what this conversion involves, but > I can certainly test whatever you came up with for Mac on Atari. > Thanks. I'll send out the latest patches for people to test. > Tick granularity is 40 us at best on Atari with the current timer > configuration, and can't be increased without increasing HZ. I suspect > the limitations on Mac are similar? > If I understand correctly, removing arch_gettimeoffset without adding a clocksource would reduce timer accuracy to 10 ms regardless of platform (because that's what the default jiffies clocksource offers). -- > Cheers, > > Michael >
Re: m68k using deprecated internal APIs?
Thanks Finn, Am 09.11.2018 um 12:42 schrieb Finn Thain: On Sun, 28 Oct 2018, Geert Uytterhoeven wrote: The example I gave was GENERIC_CLOCKEVENTS on m68, which is supported on most but not all machines there. That suggests that the removal of just those machines would suffice (as opposed to the removal of the entire arch). Also, Documentation/features/time/clockevents/arch-support.txt says, |m68k: | ok | These two observations make me wonder whether the clockevents feature is related to the discussion quoted above (?) GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types. !GENERIC_CLOCKEVENTS implies PREEMPT_NONE, and disables the "Timers subsystem" (i.e. the NO_HZ config options), but it works fine -- I was able to convert the Mac port to !ARCH_USES_GETTIMEOFFSET && !GENERIC_CLOCKEVENTS. (Like many of the Coldfire CPU types.) You can see those patches here, https://github.com/fthain/linux/commits/mac68k-queue/ Note that !ARCH_USES_GETTIMEOFFSET is a build-time choice, meaning that all platforms need to be converted together. Also, some platforms will need to adopt the clocksource API, otherwise the built-in "jiffies" clocksource will get used, causing a regression in timer accuracy. Conversion to the clocksource API is straight-forward where the platform already implements arch_gettimeoffset. The conversion is discussed in include/linux/time.h: /* Some architectures do not supply their own clocksource. * This is mainly the case in architectures that get their * inter-tick times by reading the counter on their interval * timer. Since these timers wrap every tick, they're not really * useful as clocksources. Wrapping them to act like one is possible * but not very efficient. So we provide a callout these arches * can implement for use with the jiffies clocksource to provide * finer then tick granular time. */ (Not sure what is meant by "not very efficient" here.) Certain 680x0 platforms do not implement arch_gettimeoffset: apollo, q40, sun3, sun3x. These platforms can fall back on the "jiffies" clocksource with no loss of timer accuracy, so conversion for these platforms is trivial. Should I continue with the clocksource API conversion for the other platforms: amiga, atari, bvme6000, hp300, mvme147, mvme16x? This would allow for removal of "select ARCH_USES_GETTIMEOFFSET" (and satisfy Documentation/features/time/modern-timekeeping) without loss of timer accuracy. Alternatively, we could defer the clocksource API conversion, leaving it up to the platform maintainers (who can actually test the driver changes). But that would mean that many platforms would suffer a regression in timer accuracy upon removal of arch_gettimeoffset. That's ultimately for Geert to decide - I'll yet have to look at your mac patches to even get a vague idea what this conversion involves, but I can certainly test whatever you came up with for Mac on Atari. Tick granularity is 40 us at best on Atari with the current timer configuration, and can't be increased without increasing HZ. I suspect the limitations on Mac are similar? Cheers, Michael
Re: m68k using deprecated internal APIs?
On Fri, Nov 9, 2018 at 12:42 AM Finn Thain wrote:
> On Sun, 28 Oct 2018, Geert Uytterhoeven wrote:
> > > > The example I gave was GENERIC_CLOCKEVENTS on m68, which is
> > > > supported on most but not all machines there.
> > >
> > > That suggests that the removal of just those machines would suffice
> > > (as opposed to the removal of the entire arch).
> > >
> > > Also, Documentation/features/time/clockevents/arch-support.txt says,
> > > |m68k: | ok |
> > >
> > > These two observations make me wonder whether the clockevents feature
> > > is related to the discussion quoted above (?)
> >
> > GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types.
> >
>
> !GENERIC_CLOCKEVENTS implies PREEMPT_NONE, and disables the "Timers
> subsystem" (i.e. the NO_HZ config options), but it works fine -- I was
> able to convert the Mac port to !ARCH_USES_GETTIMEOFFSET &&
> !GENERIC_CLOCKEVENTS. (Like many of the Coldfire CPU types.)
>
> You can see those patches here,
> https://github.com/fthain/linux/commits/mac68k-queue/
>
> Note that !ARCH_USES_GETTIMEOFFSET is a build-time choice, meaning that
> all platforms need to be converted together.
>
> Also, some platforms will need to adopt the clocksource API, otherwise the
> built-in "jiffies" clocksource will get used, causing a regression in
> timer accuracy.
>
> Conversion to the clocksource API is straight-forward where the platform
> already implements arch_gettimeoffset. The conversion is discussed in
> include/linux/time.h:
>
> /* Some architectures do not supply their own clocksource.
> * This is mainly the case in architectures that get their
> * inter-tick times by reading the counter on their interval
> * timer. Since these timers wrap every tick, they're not really
> * useful as clocksources. Wrapping them to act like one is possible
> * but not very efficient. So we provide a callout these arches
> * can implement for use with the jiffies clocksource to provide
> * finer then tick granular time.
> */
>
> (Not sure what is meant by "not very efficient" here.)
>
> Certain 680x0 platforms do not implement arch_gettimeoffset: apollo, q40,
> sun3, sun3x. These platforms can fall back on the "jiffies" clocksource
> with no loss of timer accuracy, so conversion for these platforms is
> trivial.
>
> Should I continue with the clocksource API conversion for the other
> platforms: amiga, atari, bvme6000, hp300, mvme147, mvme16x? This would
> allow for removal of "select ARCH_USES_GETTIMEOFFSET" (and satisfy
> Documentation/features/time/modern-timekeeping) without loss of timer
> accuracy.
>
> Alternatively, we could defer the clocksource API conversion, leaving it
> up to the platform maintainers (who can actually test the driver changes).
> But that would mean that many platforms would suffer a regression in timer
> accuracy upon removal of arch_gettimeoffset.
Adding the timekeeping maintainers and Linus Walleij to Cc. Linus
worked on removing ARCH_USES_GETTIMEOFFSET on ARM
several platforms in the past.
I noticed that the last timer rework involving
CONFIG_ARCH_USES_GETTIMEOFFSET was back in 2012 with
commit 7b1f62076bba ("time: convert arch_gettimeoffset to a pointer").
At the time, we had cris, m32r, blackfin, m68k and lots of ARM
platforms, now it's only two StrongARM platforms (RPC and
ARCH_EBSA110) and classic m68k.
Arnd
Re: m68k using deprecated internal APIs?
On Sun, 28 Oct 2018, Geert Uytterhoeven wrote: > > > > > > The example I gave was GENERIC_CLOCKEVENTS on m68, which is > > > supported on most but not all machines there. > > > > That suggests that the removal of just those machines would suffice > > (as opposed to the removal of the entire arch). > > > > Also, Documentation/features/time/clockevents/arch-support.txt says, > > |m68k: | ok | > > > > These two observations make me wonder whether the clockevents feature > > is related to the discussion quoted above (?) > > GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types. > !GENERIC_CLOCKEVENTS implies PREEMPT_NONE, and disables the "Timers subsystem" (i.e. the NO_HZ config options), but it works fine -- I was able to convert the Mac port to !ARCH_USES_GETTIMEOFFSET && !GENERIC_CLOCKEVENTS. (Like many of the Coldfire CPU types.) You can see those patches here, https://github.com/fthain/linux/commits/mac68k-queue/ Note that !ARCH_USES_GETTIMEOFFSET is a build-time choice, meaning that all platforms need to be converted together. Also, some platforms will need to adopt the clocksource API, otherwise the built-in "jiffies" clocksource will get used, causing a regression in timer accuracy. Conversion to the clocksource API is straight-forward where the platform already implements arch_gettimeoffset. The conversion is discussed in include/linux/time.h: /* Some architectures do not supply their own clocksource. * This is mainly the case in architectures that get their * inter-tick times by reading the counter on their interval * timer. Since these timers wrap every tick, they're not really * useful as clocksources. Wrapping them to act like one is possible * but not very efficient. So we provide a callout these arches * can implement for use with the jiffies clocksource to provide * finer then tick granular time. */ (Not sure what is meant by "not very efficient" here.) Certain 680x0 platforms do not implement arch_gettimeoffset: apollo, q40, sun3, sun3x. These platforms can fall back on the "jiffies" clocksource with no loss of timer accuracy, so conversion for these platforms is trivial. Should I continue with the clocksource API conversion for the other platforms: amiga, atari, bvme6000, hp300, mvme147, mvme16x? This would allow for removal of "select ARCH_USES_GETTIMEOFFSET" (and satisfy Documentation/features/time/modern-timekeeping) without loss of timer accuracy. Alternatively, we could defer the clocksource API conversion, leaving it up to the platform maintainers (who can actually test the driver changes). But that would mean that many platforms would suffer a regression in timer accuracy upon removal of arch_gettimeoffset. --
Re: m68k using deprecated internal APIs?
On Mon, 29 Oct 2018, Arnd Bergmann wrote: > [...] The real question that we tried to resolve is how we can more > generally find out whether a driver is still being used or not when it > gets in the way of some API conversion. [...] I think you have to show that the driver or platform or arch has been broken for some mandated minimum length of time. More importantly, you also need to prevent developers from carelessly breaking legacy drivers. And there is a clear tendency among maintainers to abandon the usual standards of care and diligence when refactoring legacy code or reviewing other developers' patches to that code. (I've seen this happen again and again.) This leads to regressions that go undetected, or when detected, may go unreported by users*. Either way, the driver can get removed as being "unused". That result serves the interests of certain parties who happen to employ maintainers. You can avoid this moral hazard by use of automated refactoring and other tools (to prevent behavioural changes or prove equivalence) and by impartial review. > > I brought up m68k in particular as it was used on the oldest machines I > could find, sun3. It's worth noting that the oldest drivers and ports are often the smallest. If your criterion for deleteion is age, you can expect rapidly diminishing returns. I think you need to quantify this problem. How much would your past API modernization effort have been reduced if there was no arch/m68k, or no CONFIG_SUN3? You can measure that in lines of code. You should also look at the portion of past modernization effort spent inside arch/m68k or #ifdef CONFIG_SUN3, and figure out how much of that effort could have been automated (given better tools). * Reporting regressions is not free. I suspect it is more efficient for a developer to avoid introducing the bug in the first place, than for a developer and a user to collaborate on fixing it. --
Re: m68k using deprecated internal APIs?
On Mon, Oct 29, 2018 at 2:58 AM Greg Ungerer wrote: > I have been working on and maintaining parts of m68k for a long time, > and I was not aware that there was a number of problem areas that are > causing real pain. Maybe a friendly email from subsystem maintainers that > see issues would go a long way. At least if the m68k (and other arch) > communities know they can work toward solutions sooner rather than later. > > Talk of ultimatums seems a bit heavy handed when only one side seems > to be aware there is a problem. Adding Ted to Cc, as he was the one that brought up the ultimatum, and James who raised the broader question of the long-term prospects of 32-bit hardware. I have to admit that I failed to direct the maintainer summit discussion the way I had originally intended. I started off with the eight architectures that got removed, as an example for stuff that was already known to be basically completely unused (for mainline Linux), and how it took me a long time to be really sure that we are not hurting any remaining users. I did not think (and still don't) that we are going to be removing further CPU architectures any time soon, with the possible exception of those that no longer have any compiler that can build them (unicore32 only has a gcc-4.5 binary that no longer works, hexagon has 4.6 sources and binary that barely work). The real question that we tried to resolve is how we can more generally find out whether a driver is still being used or not when it gets in the way of some API conversion. This most often involves drivers for PC add-on cards (network, scsi, ide, isdn, ...) and old bus interconnects (ISA, EISA, previously MCA), but can also affect architectures from those days (alpha, parisc, m68k). The conflict here is that unmaintained and unused drivers are a burden for any treewide API changes in particular because there is nobody who can test or Ack the changes, but removing those drivers too aggressively is certain to hit some users that still require the drivers and simply update their kernels too infrequently to test changes in time. I brought up m68k in particular as it was used on the oldest machines I could find, sun3: To my best knowledge, the last working version of Linux on this hardware was 2.6.26 with the addition of a simple patch that was (and presumably still is) needed to get access to the serial console. My guess is that aside from the popular m68k platforms (amiga, atari, coldfire and mac and a one or two I'm not familiar with), there is a lot of code that has not been used on many years. The same thing is certainly true of mips, ppc, sh, and arm, all of which could benefit from having someone take a critical look at which platforms were actually ever properly supported at any point. When we did that on ARM a few years ago, we ended up removing several that seemed promising when they initially got added but that were not completed before the people working on them moved on, or that had worked at some point but stopped compiling several years earlier without anyone noticing. Arnd
Re: m68k using deprecated internal APIs?
Hi Arnd, On 28/10/18 1:54 am, Arnd Bergmann wrote: On Sat, Oct 27, 2018 at 5:02 PM Geert Uytterhoeven wrote: Hi Arnd, https://lwn.net/Articles/769468/ wrote: For example, the m68k architecture uses a number of internal APIs that no other architecture needs at this point; removing that architecture would enable removing the APIs as well and Ted Ts'o suggested that an ultimatum could be made: either the m68k architecture stops using the old, deprecated timer API (for example) within one year or it is removed from the kernel. Which APIs are these exactly? The example I gave was GENERIC_CLOCKEVENTS on m68, which is supported on most but not all machines there. This is also missing on a couple of others (ia64 at least, not sure what else). Another one would be having CLKDEV_LOOKUP without COMMON_CLK. This one is not a problem for m68k but is for a couple of ARM and MIPS platforms that have not yet been converted to COMMON_CLK. There are probably a couple more like this. I don't actually see any that are /only/ used by m68k, but there are some interfaces that would be good to stop using overall to keep things simpler. I have been working on and maintaining parts of m68k for a long time, and I was not aware that there was a number of problem areas that are causing real pain. Maybe a friendly email from subsystem maintainers that see issues would go a long way. At least if the m68k (and other arch) communities know they can work toward solutions sooner rather than later. Talk of ultimatums seems a bit heavy handed when only one side seems to be aware there is a problem. Regards Greg
Re: m68k using deprecated internal APIs?
On Sun, 28 Oct 2018, Geert Uytterhoeven wrote: > > > > > > The example I gave was GENERIC_CLOCKEVENTS on m68, which is > > > supported on most but not all machines there. > > > > That suggests that the removal of just those machines would suffice > > (as opposed to the removal of the entire arch). > > > > Also, Documentation/features/time/clockevents/arch-support.txt says, > > |m68k: | ok | > > > > These two observations make me wonder whether the clockevents feature > > is related to the discussion quoted above (?) > > GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types. > Certain arm and m68k platforms select ARCH_USES_GETTIMEOFFSET (hence these architectures have "TODO" in features/time/modern-timekeeping). Moreover, it seems that we need both features (!ARCH_USES_GETTIMEOFFSET && GENERIC_CLOCKEVENTS) in order to get the benefit of simplification... > > Perhaps the most overdue features are the following. At least 50% of > > architectures have already implemented these features (or else cannot > > implement them). > > > > $ grep -cw TODO Documentation/features/*/*/arch-support.txt |sort -t: -k2n > > |head -n 9 > > Documentation/features/time/clockevents/arch-support.txt:1 > > Documentation/features/time/modern-timekeeping/arch-support.txt:2 > > Documentation/features/vm/numa-memblock/arch-support.txt:2 > > Documentation/features/vm/THP/arch-support.txt:5 > > Documentation/features/sched/numa-balancing/arch-support.txt:6 > > Documentation/features/core/tracehook/arch-support.txt:7 > > Documentation/features/core/generic-idle-thread/arch-support.txt:8 > > Documentation/features/locking/lockdep/arch-support.txt:8 > > Documentation/features/debug/kgdb/arch-support.txt:12 > > > > Of those, m68k has a "TODO" against the following 5 features. > > > > time/modern-timekeeping > > core/tracehook > > core/generic-idle-thread > > locking/lockdep > > debug/kgdb > > > > The question is, which of those features, if implemented, would contribute > > the most towards the goal (that is, to keep things simpler)? > > That's a very good question! > I think it would be helpful if Documentation/features/... would state what kind of benefit warranted the TODO. E.g. a win-win simplification arises from replacing arch code with generic code. Another kind of simplification arises from removing special cases from core code. The !ARCH_USES_GETTIMEOFFSET && GENERIC_CLOCKEVENTS example seems to offer both kinds of benefit. > E.g. kgdb is not a must-have, and lockdep matters much less if !SMP (and > I don't have RAM for storing all data it needs anyway). > Perhaps we should ask, "which features, if implemented, would actually complicate things, taking us further from the goal?" In those cases, "TODO" seems to be bad advice. Perhaps "optional" would be better. --
Re: m68k using deprecated internal APIs?
[ Wow, I hadn't expected such a heated discussion, I just wanted to
know which code
needed fixes... ]
On Sun, Oct 28, 2018 at 8:00 AM Finn Thain wrote:
> On Sat, 27 Oct 2018, Arnd Bergmann wrote:
> > On Sat, Oct 27, 2018 at 5:02 PM Geert Uytterhoeven
> > wrote:
> > > https://lwn.net/Articles/769468/ wrote:
> > > > For example, the m68k architecture uses a number of internal APIs that
> > > > no other
> > > > architecture needs at this point; removing that architecture would
> > > > enable removing
> > > > the APIs as well
> > >
> > > and
> > >
> > > > Ted Ts'o suggested that an ultimatum could be made: either the m68k
> > > > architecture
> > > > stops using the old, deprecated timer API (for example) within one year
> > > > or it is
> > > > removed from the kernel.
> > >
> > > Which APIs are these exactly?
> >
> > The example I gave was GENERIC_CLOCKEVENTS on m68, which is
> > supported on most but not all machines there.
>
> That suggests that the removal of just those machines would suffice (as
> opposed to the removal of the entire arch).
>
> Also, Documentation/features/time/clockevents/arch-support.txt says,
> |m68k: | ok |
>
> These two observations make me wonder whether the clockevents feature is
> related to the discussion quoted above (?)
GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types.
> Perhaps the most overdue features are the following. At least 50% of
> architectures have already implemented these features (or else cannot
> implement them).
>
> $ grep -cw TODO Documentation/features/*/*/arch-support.txt |sort -t: -k2n
> |head -n 9
> Documentation/features/time/clockevents/arch-support.txt:1
> Documentation/features/time/modern-timekeeping/arch-support.txt:2
> Documentation/features/vm/numa-memblock/arch-support.txt:2
> Documentation/features/vm/THP/arch-support.txt:5
> Documentation/features/sched/numa-balancing/arch-support.txt:6
> Documentation/features/core/tracehook/arch-support.txt:7
> Documentation/features/core/generic-idle-thread/arch-support.txt:8
> Documentation/features/locking/lockdep/arch-support.txt:8
> Documentation/features/debug/kgdb/arch-support.txt:12
>
> Of those, m68k has a "TODO" against the following 5 features.
>
> time/modern-timekeeping
> core/tracehook
> core/generic-idle-thread
> locking/lockdep
> debug/kgdb
>
> The question is, which of those features, if implemented, would contribute
> the most towards the goal (that is, to keep things simpler)?
That's a very good question!
E.g. kgdb is not a must-have, and lockdep matters much less if !SMP (and I don't
have RAM for storing all data it needs anyway).
> We might also ask about features that cannot be implemented on m68k; there
> are 4 of those.
>
> $ grep -cw "m68k.*[.][.]" Documentation/features/*/*/arch-support.txt |sort
> -t: -k2n
> ...
> Documentation/features/sched/numa-balancing/arch-support.txt:1
> Documentation/features/vm/THP/arch-support.txt:1
> Documentation/features/vm/TLB/arch-support.txt:1
> Documentation/features/vm/numa-memblock/arch-support.txt:1
>
> At least 8 out of 24 architectures would have to be deleted to make that
> list any shorter.
I seriously doubt "impossible" features matter ;-)
BTW, I believe KPTI can be implemented, without a performance penalty,
unlike on x86...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: m68k using deprecated internal APIs?
On Sat, 27 Oct 2018, Arnd Bergmann wrote: > On Sat, Oct 27, 2018 at 5:02 PM Geert Uytterhoeven > wrote: > > > > Hi Arnd, > > > > https://lwn.net/Articles/769468/ wrote: > > > For example, the m68k architecture uses a number of internal APIs that > > > no other > > > architecture needs at this point; removing that architecture would enable > > > removing > > > the APIs as well > > > > and > > > > > Ted Ts'o suggested that an ultimatum could be made: either the m68k > > > architecture > > > stops using the old, deprecated timer API (for example) within one year > > > or it is > > > removed from the kernel. > > > > Which APIs are these exactly? > > The example I gave was GENERIC_CLOCKEVENTS on m68, which is > supported on most but not all machines there. That suggests that the removal of just those machines would suffice (as opposed to the removal of the entire arch). Also, Documentation/features/time/clockevents/arch-support.txt says, |m68k: | ok | These two observations make me wonder whether the clockevents feature is related to the discussion quoted above (?) > This is also missing on a couple of others (ia64 at least, not sure what > else). Another one would be having CLKDEV_LOOKUP without COMMON_CLK. > This one is not a problem for m68k but is for a couple of ARM and MIPS > platforms that have not yet been converted to COMMON_CLK. > > There are probably a couple more like this. I don't actually see any > that are /only/ used by m68k, but there are some interfaces that would > be good to stop using overall to keep things simpler. > Perhaps the most overdue features are the following. At least 50% of architectures have already implemented these features (or else cannot implement them). $ grep -cw TODO Documentation/features/*/*/arch-support.txt |sort -t: -k2n |head -n 9 Documentation/features/time/clockevents/arch-support.txt:1 Documentation/features/time/modern-timekeeping/arch-support.txt:2 Documentation/features/vm/numa-memblock/arch-support.txt:2 Documentation/features/vm/THP/arch-support.txt:5 Documentation/features/sched/numa-balancing/arch-support.txt:6 Documentation/features/core/tracehook/arch-support.txt:7 Documentation/features/core/generic-idle-thread/arch-support.txt:8 Documentation/features/locking/lockdep/arch-support.txt:8 Documentation/features/debug/kgdb/arch-support.txt:12 Of those, m68k has a "TODO" against the following 5 features. time/modern-timekeeping core/tracehook core/generic-idle-thread locking/lockdep debug/kgdb The question is, which of those features, if implemented, would contribute the most towards the goal (that is, to keep things simpler)? We might also ask about features that cannot be implemented on m68k; there are 4 of those. $ grep -cw "m68k.*[.][.]" Documentation/features/*/*/arch-support.txt |sort -t: -k2n ... Documentation/features/sched/numa-balancing/arch-support.txt:1 Documentation/features/vm/THP/arch-support.txt:1 Documentation/features/vm/TLB/arch-support.txt:1 Documentation/features/vm/numa-memblock/arch-support.txt:1 At least 8 out of 24 architectures would have to be deleted to make that list any shorter. -- >Arnd >
Re: m68k using deprecated internal APIs?
On Sat, 27 Oct 2018, Geert Uytterhoeven wrote:
> Hi Arnd,
>
> https://lwn.net/Articles/769468/ wrote:
> > For example, the m68k architecture uses a number of internal APIs
> > that no other architecture needs at this point; removing that
> > architecture would enable removing the APIs as well
>
> and
>
> > Ted Ts'o suggested that an ultimatum could be made: either the m68k
> > architecture stops using the old, deprecated timer API (for example)
> > within one year or it is removed from the kernel.
>
> Which APIs are these exactly?
>
Tree-wide replacement of deprecated API's is one thing.
Gunning for one particular architecture is a different matter.
Of course, if that architecture is known to be fatally flawed (hi spectre)
that's quite okay.
--
> > This kind of approach has worked well in the Debian community
>
> Right, Debian stopped supporting m68k a long time ago ;-)
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
>
Re: m68k using deprecated internal APIs?
On Sat, 27 Oct 2018, John Paul Adrian Glaubitz wrote: > This stuff is so getting annoying. I don't understand why all of a > sudden there is such a big urge to kick everything out that is old. Is > the Linux kernel supposed to be an x86-only project? > Some maintainers don't like mature code. But it's hard to figure out why it is so. Defect density in old code is never used as a criterion. I guess there are perverse incentives at work in industry which don't exist in the wider community. E.g. workers being paid according to the number of lines of code added/removed/modified. Refactoring legacy code can be automated. But doing so would touch fewer lines of code than wholesale deletion, so the incentive seems to be backwards. > I don't get it. The port is actively maintained and working well, new > drivers are being added. And there is a vibrant community using it. Try > buying a used Amiga on eBay and you know what I mean. > > There is new hardware being developed all the time. Just recently, we > added support for the xsurf100 network card from Individual Computers. > And I expect more drivers to be added in the future. > > Is Linux really only a commercial product now so that everything that is > not maintained by a large company needs to go quick? > That has been my impression also. It's very short sighted, because standard practice in engineering is to re-use proven ideas. Old designs get shipped in new guises, albeit smaller and faster. The alternative to re-use is re-invention, which is not obviously profitable but does generate economic activity. Therefore, re-use may be undesirable according to Keynesianism. Also, bogus patents issued for designs that already have prior art may be compounding the problem but this is all just speculation. -- > Adrian > >
Re: m68k using deprecated internal APIs?
On Sat, Oct 27, 2018 at 5:02 PM Geert Uytterhoeven wrote: > > Hi Arnd, > > https://lwn.net/Articles/769468/ wrote: > > For example, the m68k architecture uses a number of internal APIs that no > > other > > architecture needs at this point; removing that architecture would enable > > removing > > the APIs as well > > and > > > Ted Ts'o suggested that an ultimatum could be made: either the m68k > > architecture > > stops using the old, deprecated timer API (for example) within one year or > > it is > > removed from the kernel. > > Which APIs are these exactly? The example I gave was GENERIC_CLOCKEVENTS on m68, which is supported on most but not all machines there. This is also missing on a couple of others (ia64 at least, not sure what else). Another one would be having CLKDEV_LOOKUP without COMMON_CLK. This one is not a problem for m68k but is for a couple of ARM and MIPS platforms that have not yet been converted to COMMON_CLK. There are probably a couple more like this. I don't actually see any that are /only/ used by m68k, but there are some interfaces that would be good to stop using overall to keep things simpler. Arnd
Re: m68k using deprecated internal APIs?
On 10/27/18 5:01 PM, Geert Uytterhoeven wrote: >> This kind of approach has worked well in the Debian community > > Right, Debian stopped supporting m68k a long time ago ;-) This stuff is so getting annoying. I don't understand why all of a sudden there is such a big urge to kick everything out that is old. Is the Linux kernel supposed to be an x86-only project? I don't get it. The port is actively maintained and working well, new drivers are being added. And there is a vibrant community using it. Try buying a used Amiga on eBay and you know what I mean. There is new hardware being developed all the time. Just recently, we added support for the xsurf100 network card from Individual Computers. And I expect more drivers to be added in the future. Is Linux really only a commercial product now so that everything that is not maintained by a large company needs to go quick? Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
