[PATCH] RISC-V: defconfig: Enable printk timestamps

[PATCH] RISC-V: defconfig: Enable printk timestamps

[Anup Patel]

The printk timestamps are very useful information to visually see
where kernel is spending time during boot. It also helps us see
the timing of hotplug events at runtime.

This patch enables printk timestamps in RISC-V defconfig so that
we have it enabled by default (similar to other architectures
such as x86_64, arm64, etc).

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/configs/defconfig | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 36473d7dbaac..ef4f15df9adf 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -1,6 +1,3 @@
-CONFIG_SMP=y
-CONFIG_PCI=y
-CONFIG_PCIE_XILINX=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
 CONFIG_IKCONFIG=y
@@ -11,10 +8,15 @@ CONFIG_CFS_BANDWIDTH=y
 CONFIG_CGROUP_BPF=y
 CONFIG_NAMESPACES=y
 CONFIG_USER_NS=y
+CONFIG_CHECKPOINT_RESTORE=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_EXPERT=y
-CONFIG_CHECKPOINT_RESTORE=y
 CONFIG_BPF_SYSCALL=y
+CONFIG_SMP=y
+CONFIG_PCI=y
+CONFIG_PCIE_XILINX=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_UNIX=y
@@ -59,6 +61,7 @@ CONFIG_USB_OHCI_HCD_PLATFORM=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_UAS=y
 CONFIG_VIRTIO_MMIO=y
+CONFIG_SIFIVE_PLIC=y
 CONFIG_RAS=y
 CONFIG_EXT4_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
@@ -72,8 +75,6 @@ CONFIG_NFS_V4=y
 CONFIG_NFS_V4_1=y
 CONFIG_NFS_V4_2=y
 CONFIG_ROOT_NFS=y
-# CONFIG_RCU_TRACE is not set
 CONFIG_CRYPTO_USER_API_HASH=y
-CONFIG_MODULES=y
-CONFIG_MODULE_UNLOAD=y
-CONFIG_SIFIVE_PLIC=y
+CONFIG_PRINTK_TIME=y
+# CONFIG_RCU_TRACE is not set
-- 
2.17.1

Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

[Olof Johansson]

On Tue, Oct 30, 2018 at 5:37 AM Anup Patel <anup@brainfault.org> wrote:
>
> The printk timestamps are very useful information to visually see
> where kernel is spending time during boot. It also helps us see
> the timing of hotplug events at runtime.
>
> This patch enables printk timestamps in RISC-V defconfig so that
> we have it enabled by default (similar to other architectures
> such as x86_64, arm64, etc).
>
> Signed-off-by: Anup Patel <anup@brainfault.org>

Acked-by: Olof Johansson <olof@lixom.net>

For next time: doing a re-format of the defconfig (to shuffle config
order), plus changes in the same patch, tends to be a bit fragile. For
cases like these, I'd recommend hand-pruning to just flip the one
option if needed, and then have Palmer or Andrew refresh the defconfig
during a merge window if needed (usually quite rare).


-Olof

Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

[Palmer Dabbelt]

On Wed, 31 Oct 2018 12:20:40 PDT (-0700), Olof Johansson wrote:
> On Tue, Oct 30, 2018 at 5:37 AM Anup Patel <anup@brainfault.org> wrote:
>>
>> The printk timestamps are very useful information to visually see
>> where kernel is spending time during boot. It also helps us see
>> the timing of hotplug events at runtime.
>>
>> This patch enables printk timestamps in RISC-V defconfig so that
>> we have it enabled by default (similar to other architectures
>> such as x86_64, arm64, etc).
>>
>> Signed-off-by: Anup Patel <anup@brainfault.org>
>
> Acked-by: Olof Johansson <olof@lixom.net>
>
> For next time: doing a re-format of the defconfig (to shuffle config
> order), plus changes in the same patch, tends to be a bit fragile. For
> cases like these, I'd recommend hand-pruning to just flip the one
> option if needed, and then have Palmer or Andrew refresh the defconfig
> during a merge window if needed (usually quite rare).

I poked around and it looks like most architectures have this enabled for at 
least one defconfig, with the big architectures having it enabled for all of 
them.

I decided to do a bit of a case study here to try and figure out why some 
architectures have this enabled for some defconfigs but not for others, and as 
far as I can tell it's just an oversight.  Specifically, looking at sparc32 (# 
CONFIG_PRINTK_TIME is not set) vs sparc64 (CONFIG_PRINTK_TIME=y) I can't find 
any reason for the difference.  sparc32's setting can be traced back to

    commit 216da721b881838d639a3987bf8a825e6b4aacdd
    Author: David S. Miller <davem@sunset.davemloft.net>
    Date:   Sun Dec 17 14:21:34 2006 -0800
    
        [SPARC]: Update defconfig.
    
        Signed-off-by: David S. Miller <davem@davemloft.net>

which changes everything in the defconfig, while the sparc64 version dates back to

    commit 3ebc284d52757cf39788731f8fddd70a89f7fc23
    Author: David S. Miller <davem@sunset.davemloft.net>
    Date:   Mon Jan 9 14:36:49 2006 -0800
    
        [SPARC64]: Update defconfig.
    
        Signed-off-by: David S. Miller <davem@davemloft.net>

When we first submitted out port upstream we had an empty defconfig, with the 
theory being that we should just pick whatever the default in Kconfig is for 
everything.  That's obviously the wrong thing to do because most of those 
options are bogus.  At the time I didn't care enough to look because I just 
wanted something to work, but now I find myself asking the question "what goes 
in a defconfig?"  Is it:

* What I, as the maintainer of arch/riscv, want?  That's essentially what it is 
  now, as we have things like "CONFIG_R8169=y" in there because I happened to 
  have one sitting around when we needed to plug in an Ethernet card to test 
  out PCIe.
* What distributions expect?  There's a major element of this in there right 
  now, as half this stuff was just selected because the Debian and Fedora guys 
  suggested we do so because adding things to the RISC-V defconfig made it 
  easier to put together their build scripts.  For example, we ended up with 
  "CONFIG_EXPERT=y" because some setting necessary for the distros was hidden 
  behind it -- that seems like an odd default.
* What users expect?  I'm not even sure who users are in this case, as from my 
  understating most people use distro kernels and don't twiddle Kconfig 
  options.
* What is necessary to work on RISC-V hardware?  This seems like the most 
  reasonable use for an arch-specific defconfig, and subsumes things like 
  "CONFIG_SIFIVE_PLIC=y" because without the PLIC driver nothing will work (but 
  the PLIC driver shouldn't be enabled by default for all architectures, as 
  it's useless everywhere else).

Maybe I've opened up a big can of worms here...  It just seems silly to have 
most of our current defconfig be RISC-V specific.

Anyway, I'm happy with the change because it meets my "what I want" criteria 
:).  I'll split it into two parts, though, as that way when someone else has to 
go to some archaeology on our port they'll be less likely to get lost.

Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

[Olof Johansson]

On Wed, Oct 31, 2018 at 1:42 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Wed, 31 Oct 2018 12:20:40 PDT (-0700), Olof Johansson wrote:
> > On Tue, Oct 30, 2018 at 5:37 AM Anup Patel <anup@brainfault.org> wrote:
> >>
> >> The printk timestamps are very useful information to visually see
> >> where kernel is spending time during boot. It also helps us see
> >> the timing of hotplug events at runtime.
> >>
> >> This patch enables printk timestamps in RISC-V defconfig so that
> >> we have it enabled by default (similar to other architectures
> >> such as x86_64, arm64, etc).
> >>
> >> Signed-off-by: Anup Patel <anup@brainfault.org>
> >
> > Acked-by: Olof Johansson <olof@lixom.net>
> >
> > For next time: doing a re-format of the defconfig (to shuffle config
> > order), plus changes in the same patch, tends to be a bit fragile. For
> > cases like these, I'd recommend hand-pruning to just flip the one
> > option if needed, and then have Palmer or Andrew refresh the defconfig
> > during a merge window if needed (usually quite rare).
>
> I poked around and it looks like most architectures have this enabled for at
> least one defconfig, with the big architectures having it enabled for all of
> them.

It's pretty convenient to have it on, and I think those who haven't
probably don't use those defconfigs much.

> I decided to do a bit of a case study here to try and figure out why some
> architectures have this enabled for some defconfigs but not for others, and as
> far as I can tell it's just an oversight.  Specifically, looking at sparc32 (#
> CONFIG_PRINTK_TIME is not set) vs sparc64 (CONFIG_PRINTK_TIME=y) I can't find
> any reason for the difference.  sparc32's setting can be traced back to

I'm not sure sparc32 is overly active these days, which might help explain it?

>     commit 216da721b881838d639a3987bf8a825e6b4aacdd
>     Author: David S. Miller <davem@sunset.davemloft.net>
>     Date:   Sun Dec 17 14:21:34 2006 -0800
>
>         [SPARC]: Update defconfig.
>
>         Signed-off-by: David S. Miller <davem@davemloft.net>
>
> which changes everything in the defconfig, while the sparc64 version dates back to
>
>     commit 3ebc284d52757cf39788731f8fddd70a89f7fc23
>     Author: David S. Miller <davem@sunset.davemloft.net>
>     Date:   Mon Jan 9 14:36:49 2006 -0800
>
>         [SPARC64]: Update defconfig.
>
>         Signed-off-by: David S. Miller <davem@davemloft.net>
>
> When we first submitted out port upstream we had an empty defconfig, with the
> theory being that we should just pick whatever the default in Kconfig is for
> everything.  That's obviously the wrong thing to do because most of those
> options are bogus.  At the time I didn't care enough to look because I just
> wanted something to work, but now I find myself asking the question "what goes
> in a defconfig?"  Is it:
>
> * What I, as the maintainer of arch/riscv, want?  That's essentially what it is
>   now, as we have things like "CONFIG_R8169=y" in there because I happened to
>   have one sitting around when we needed to plug in an Ethernet card to test
>   out PCIe.
> * What distributions expect?  There's a major element of this in there right
>   now, as half this stuff was just selected because the Debian and Fedora guys
>   suggested we do so because adding things to the RISC-V defconfig made it
>   easier to put together their build scripts.  For example, we ended up with
>   "CONFIG_EXPERT=y" because some setting necessary for the distros was hidden
>   behind it -- that seems like an odd default.
> * What users expect?  I'm not even sure who users are in this case, as from my
>   understating most people use distro kernels and don't twiddle Kconfig
>   options.
> * What is necessary to work on RISC-V hardware?  This seems like the most
>   reasonable use for an arch-specific defconfig, and subsumes things like
>   "CONFIG_SIFIVE_PLIC=y" because without the PLIC driver nothing will work (but
>   the PLIC driver shouldn't be enabled by default for all architectures, as
>   it's useless everywhere else).
>
> Maybe I've opened up a big can of worms here...  It just seems silly to have
> most of our current defconfig be RISC-V specific.

So, on 32-bit ARM we have heavy defconfig proliferation, and for arm64
there was a push to only have one defconfig. I'm heavily in favor of
the latter.

On ARM64, the options turned on, are more or less "what makes the
kernel useful on systems". Anything that would be needed to boot to a
rootfs tends to be =y, while some other things that might be optional
and not used everywhere tends to go in as modules.

Beyond that, for the most part it is maintainer preference. I've
normally turned on platforms and drivers I need for my boot farm on
the multi*defconfig configs on 32-bit ARM, and the same has been going
for 64-bit.

Distros will always want something different, for policy reasons or
otherwise. Main purpose of mainline defconfigs is to make mainline
kernels useful and bootable, so having more of a superset approach
makes a lot of sense in my mind.

> Anyway, I'm happy with the change because it meets my "what I want" criteria
> :).  I'll split it into two parts, though, as that way when someone else has to
> go to some archaeology on our port they'll be less likely to get lost.

Sounds good to me. On ARM, we sometimes do the "refresh defconfig"
runs across the board, but not for every release since it tends to get
churny. Make sure you do it before -rc1 to avoid conflicts if
contributors base branches on -rc1 and send them to you (probably not
that common yet).



-Olof

Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

[Anup Patel]

On Thu, Nov 1, 2018 at 4:20 AM Olof Johansson <olof@lixom.net> wrote:
>
> On Wed, Oct 31, 2018 at 1:42 PM Palmer Dabbelt <palmer@sifive.com> wrote:
> >
> > On Wed, 31 Oct 2018 12:20:40 PDT (-0700), Olof Johansson wrote:
> > > On Tue, Oct 30, 2018 at 5:37 AM Anup Patel <anup@brainfault.org> wrote:
> > >>
> > >> The printk timestamps are very useful information to visually see
> > >> where kernel is spending time during boot. It also helps us see
> > >> the timing of hotplug events at runtime.
> > >>
> > >> This patch enables printk timestamps in RISC-V defconfig so that
> > >> we have it enabled by default (similar to other architectures
> > >> such as x86_64, arm64, etc).
> > >>
> > >> Signed-off-by: Anup Patel <anup@brainfault.org>
> > >
> > > Acked-by: Olof Johansson <olof@lixom.net>
> > >
> > > For next time: doing a re-format of the defconfig (to shuffle config
> > > order), plus changes in the same patch, tends to be a bit fragile. For
> > > cases like these, I'd recommend hand-pruning to just flip the one
> > > option if needed, and then have Palmer or Andrew refresh the defconfig
> > > during a merge window if needed (usually quite rare).
> >
> > I poked around and it looks like most architectures have this enabled for at
> > least one defconfig, with the big architectures having it enabled for all of
> > them.
>
> It's pretty convenient to have it on, and I think those who haven't
> probably don't use those defconfigs much.
>
> > I decided to do a bit of a case study here to try and figure out why some
> > architectures have this enabled for some defconfigs but not for others, and as
> > far as I can tell it's just an oversight.  Specifically, looking at sparc32 (#
> > CONFIG_PRINTK_TIME is not set) vs sparc64 (CONFIG_PRINTK_TIME=y) I can't find
> > any reason for the difference.  sparc32's setting can be traced back to
>
> I'm not sure sparc32 is overly active these days, which might help explain it?
>
> >     commit 216da721b881838d639a3987bf8a825e6b4aacdd
> >     Author: David S. Miller <davem@sunset.davemloft.net>
> >     Date:   Sun Dec 17 14:21:34 2006 -0800
> >
> >         [SPARC]: Update defconfig.
> >
> >         Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > which changes everything in the defconfig, while the sparc64 version dates back to
> >
> >     commit 3ebc284d52757cf39788731f8fddd70a89f7fc23
> >     Author: David S. Miller <davem@sunset.davemloft.net>
> >     Date:   Mon Jan 9 14:36:49 2006 -0800
> >
> >         [SPARC64]: Update defconfig.
> >
> >         Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > When we first submitted out port upstream we had an empty defconfig, with the
> > theory being that we should just pick whatever the default in Kconfig is for
> > everything.  That's obviously the wrong thing to do because most of those
> > options are bogus.  At the time I didn't care enough to look because I just
> > wanted something to work, but now I find myself asking the question "what goes
> > in a defconfig?"  Is it:
> >
> > * What I, as the maintainer of arch/riscv, want?  That's essentially what it is
> >   now, as we have things like "CONFIG_R8169=y" in there because I happened to
> >   have one sitting around when we needed to plug in an Ethernet card to test
> >   out PCIe.
> > * What distributions expect?  There's a major element of this in there right
> >   now, as half this stuff was just selected because the Debian and Fedora guys
> >   suggested we do so because adding things to the RISC-V defconfig made it
> >   easier to put together their build scripts.  For example, we ended up with
> >   "CONFIG_EXPERT=y" because some setting necessary for the distros was hidden
> >   behind it -- that seems like an odd default.
> > * What users expect?  I'm not even sure who users are in this case, as from my
> >   understating most people use distro kernels and don't twiddle Kconfig
> >   options.
> > * What is necessary to work on RISC-V hardware?  This seems like the most
> >   reasonable use for an arch-specific defconfig, and subsumes things like
> >   "CONFIG_SIFIVE_PLIC=y" because without the PLIC driver nothing will work (but
> >   the PLIC driver shouldn't be enabled by default for all architectures, as
> >   it's useless everywhere else).
> >
> > Maybe I've opened up a big can of worms here...  It just seems silly to have
> > most of our current defconfig be RISC-V specific.
>
> So, on 32-bit ARM we have heavy defconfig proliferation, and for arm64
> there was a push to only have one defconfig. I'm heavily in favor of
> the latter.
>
> On ARM64, the options turned on, are more or less "what makes the
> kernel useful on systems". Anything that would be needed to boot to a
> rootfs tends to be =y, while some other things that might be optional
> and not used everywhere tends to go in as modules.
>
> Beyond that, for the most part it is maintainer preference. I've
> normally turned on platforms and drivers I need for my boot farm on
> the multi*defconfig configs on 32-bit ARM, and the same has been going
> for 64-bit.
>
> Distros will always want something different, for policy reasons or
> otherwise. Main purpose of mainline defconfigs is to make mainline
> kernels useful and bootable, so having more of a superset approach
> makes a lot of sense in my mind.
>
> > Anyway, I'm happy with the change because it meets my "what I want" criteria
> > :).  I'll split it into two parts, though, as that way when someone else has to
> > go to some archaeology on our port they'll be less likely to get lost.
>
> Sounds good to me. On ARM, we sometimes do the "refresh defconfig"
> runs across the board, but not for every release since it tends to get
> churny. Make sure you do it before -rc1 to avoid conflicts if
> contributors base branches on -rc1 and send them to you (probably not
> that common yet).
>

Thanks for the comments Olof and Palmar.

I will break it into two patches and send it right away.

Regards,
Anup

Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

[Karsten Merker]

On Wed, Oct 31, 2018 at 03:50:38PM -0700, Olof Johansson wrote:
> On Wed, Oct 31, 2018 at 1:42 PM Palmer Dabbelt <palmer@sifive.com> wrote:
[...]
> > When we first submitted out port upstream we had an empty defconfig, with the
> > theory being that we should just pick whatever the default in Kconfig is for
> > everything.  That's obviously the wrong thing to do because most of those
> > options are bogus.  At the time I didn't care enough to look because I just
> > wanted something to work, but now I find myself asking the question "what goes
> > in a defconfig?"  Is it:
> >
> > * What I, as the maintainer of arch/riscv, want?  That's essentially what it is
> >   now, as we have things like "CONFIG_R8169=y" in there because I happened to
> >   have one sitting around when we needed to plug in an Ethernet card to test
> >   out PCIe.
> > * What distributions expect?  There's a major element of this in there right
> >   now, as half this stuff was just selected because the Debian and Fedora guys
> >   suggested we do so because adding things to the RISC-V defconfig made it
> >   easier to put together their build scripts.  For example, we ended up with
> >   "CONFIG_EXPERT=y" because some setting necessary for the distros was hidden
> >   behind it -- that seems like an odd default.
> > * What users expect?  I'm not even sure who users are in this case, as from my
> >   understating most people use distro kernels and don't twiddle Kconfig
> >   options.
> > * What is necessary to work on RISC-V hardware?  This seems like the most
> >   reasonable use for an arch-specific defconfig, and subsumes things like
> >   "CONFIG_SIFIVE_PLIC=y" because without the PLIC driver nothing will work (but
> >   the PLIC driver shouldn't be enabled by default for all architectures, as
> >   it's useless everywhere else).
> >
> > Maybe I've opened up a big can of worms here...  It just seems silly to have
> > most of our current defconfig be RISC-V specific.
> 
> So, on 32-bit ARM we have heavy defconfig proliferation, and for arm64
> there was a push to only have one defconfig. I'm heavily in favor of
> the latter.

ACK.

> On ARM64, the options turned on, are more or less "what makes the
> kernel useful on systems". Anything that would be needed to boot to a
> rootfs tends to be =y, while some other things that might be optional
> and not used everywhere tends to go in as modules.

ACK.

> Beyond that, for the most part it is maintainer preference. I've
> normally turned on platforms and drivers I need for my boot farm on
> the multi*defconfig configs on 32-bit ARM, and the same has been going
> for 64-bit.
> 
> Distros will always want something different, for policy reasons or
> otherwise. Main purpose of mainline defconfigs is to make mainline
> kernels useful and bootable, so having more of a superset approach
> makes a lot of sense in my mind.

My personal approach to defconfigs is that they should try to
provide the minimum hardware-related options required to boot on
existing hardware or virtual machines for the corresponding
architecture, and the minimum non-hardware related kernel options
that are required to run a typical disto userland, but not more. 
This "minimum" can still be a rather long list, though.  On the
hardware side that can include lots of low-level drivers (e.g. 
various PMICs, USB and AHCI PHYs, the various platform-specifc
frontends for (more or less) generic drivers such as
OHCI/EHCI/XHCI/AHCI/DWMAC/MUSB, etc.).  On the software side,
that minimum includes common filesystems (ext*, FAT, NFS),
everything required for systemd as the latter is nowadays used to
boot a significant proportion of all Linux distros, and basic
network support, but not much more.  I don't think that a
defconfig should follow the "everything and the kitchen sink"
approach that the distros use for their kernel builds (and which
of course makes sense for their userbase) - the defconfig is
usually used for development builds and should be small to keep
build times down.  The typical "I want it all" distro user
usually doesn't build a defconfig but either uses a pre-built
distro kernel or re-builds with an existing distro kernel config,
so the primary criterion for a defconfig should IMHO be the needs
of developers.

The "CONFIG_EXPERT=y" mentioned above was IIRC necessary to
enable some feature required by systemd, so unfortunately it is
quasi-essential to run common distro userlands nowadays.

Regards,
Karsten
-- 
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung
sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der
Werbung sowie der Markt- oder Meinungsforschung.