Date:   Thu, 1 Nov 2018 09:51:28 +0530
From:   Anup Patel <anup@...infault.org>
To:     Olof Johansson <olof@...om.net>
Cc:     Palmer Dabbelt <palmer@...ive.com>, davem@...set.davemloft.net,
        Albert Ou <aou@...s.berkeley.edu>,
        Atish Patra <atish.patra@....com>,
        Christoph Hellwig <hch@...radead.org>,
        linux-riscv@...ts.infradead.org,
        "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] RISC-V: defconfig: Enable printk timestamps

On Thu, Nov 1, 2018 at 4:20 AM Olof Johansson <olof@...om.net> wrote:
>
> On Wed, Oct 31, 2018 at 1:42 PM Palmer Dabbelt <palmer@...ive.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@...infault.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@...infault.org>
> > >
> > > Acked-by: Olof Johansson <olof@...om.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@...set.davemloft.net>
> >     Date:   Sun Dec 17 14:21:34 2006 -0800
> >
> >         [SPARC]: Update defconfig.
> >
> >         Signed-off-by: David S. Miller <davem@...emloft.net>
> >
> > which changes everything in the defconfig, while the sparc64 version dates back to
> >
> >     commit 3ebc284d52757cf39788731f8fddd70a89f7fc23
> >     Author: David S. Miller <davem@...set.davemloft.net>
> >     Date:   Mon Jan 9 14:36:49 2006 -0800
> >
> >         [SPARC64]: Update defconfig.
> >
> >         Signed-off-by: David S. Miller <davem@...emloft.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

Powered by blists - more mailing lists