Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 27 Apr 2020 15:17:52 -0700
From:   Sandeep Patil <sspatil@android.com>
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     Saravana Kannan <saravanak@google.com>,
        Baolin Wang <baolin.wang7@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Orson Zhai <orsonzhai@gmail.com>,
        Chunyan Zhang <zhang.lyra@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH 1/2] drivers/clocksource/timer-of: Remove __init markings
Message-ID: <20200427221752.GA166576@google.com>
References: <d2934f74d77c637c41d7cb98710cb5363d09e83b.1585021186.git.baolin.wang7@gmail.com>
 <CADBw62pmiguVmuq334eskLAZ5CkrjxohW_fXNjNev+O1U15=OQ@mail.gmail.com>
 <bd800f7b-05fd-b393-fba6-1965ba89e1b3@linaro.org>
 <CAGETcx9da-U+dXA6sreTKJQiS3brYh3Hfh_qVYt06=3UOSmT_g@mail.gmail.com>
 <429da47b-ad51-30eb-ff36-b05780c941ba@linaro.org>
 <CAGETcx9sc6oWSPK=aWJ+39i9eHAyg6iOf+Xf2oVECO+g1pFjtQ@mail.gmail.com>
 <573b7d9e-fb5d-5c81-a981-6d965db933f3@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <573b7d9e-fb5d-5c81-a981-6d965db933f3@linaro.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Daniel,

On Mon, Apr 27, 2020 at 10:50:24PM +0200, Daniel Lezcano wrote:
> On 27/04/2020 22:12, Saravana Kannan wrote:
> > On Mon, Apr 27, 2020 at 1:09 PM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 27/04/2020 21:04, Saravana Kannan wrote:
> >>> On Mon, Apr 27, 2020 at 10:13 AM Daniel Lezcano
> >>> <daniel.lezcano@linaro.org> wrote:
> >>>>
> >>>> On 13/04/2020 04:55, Baolin Wang wrote:
> >>>>> Hi Daniel,
> >>>>>
> >>>>> On Tue, Mar 24, 2020 at 1:59 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >>>>>>
> >>>>>> From: Saravana Kannan <saravanak@google.com>
> >>>>>>
> >>>>>> This allows timer drivers to be compiled as modules.
> >>>>>>
> >>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> >>>>>> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> >>>>>
> >>>>> Do you have any comments for this patch set? Thanks.
> >>>>
> >>>> If my understanding is correct, this patch is part of the GKI picture
> >>>> where hardware drivers are converted to modules.
> >>>>
> >>>> But do we really want to convert timer drivers to modules ?
> >>>>
> >>>> Is the core time framework able to support that (eg. load + unload )
> >>>
> >>> So this will mainly be used for secondary timers that the system
> >>> supports. Not for the main one that's set up during early boot for
> >>> sched timer to work. For the primary timer during boot up, we still
> >>> expect that to be the default ARM timer and don't want/expect that to
> >>> be a module (it can't be).
> >>
> >> My question is about clockevents_config_and_register() for instance, is
> >> there a function to unregister in the core framework ?
> > 
> > We can just have these modules be "permanent" modules that can't be
> > unloaded. They just need to not implement module_exit().
> 
> You are right.
> 
> I can understand the goal of making everything as much modular as possible.
> 
> But TBH, I have a bad feeling about this. Sounds like GKI will give the
> opportunity to companies to stop upstreaming their drivers and favoring
> fragmentation like what we had several years ago. Not sure it is a good
> thing, especially for upstream SoC support.

... and that is a very valid concern too IMO.

However, the way we see it, as things stand today, we don't even know what
goes into Linux on all android phones out there. We know what we add, as part
of the AOSP kernel, however, what actually runs on the device is normally
about a million lines of code changes on top of what we do.

So, for the GKI parts, we are doing the following

1. Making the peripheral drivers modules also means the GKI must have all the
core framework changes built-in. This gets us the list of core kernel changes
that ship on Android devices so we can work on upstreaming them OR find the
appropriate alternative. For Android, that answers the canonical
- "Where is the use case?" question coming from anyone.

You can see the list of these out-of-tree changes is growing by the day in
AOSP right now[1]. Its there for everyone to find in exactly *one place*.
Note that, almost all of those patches have been posted on the list already.
That's the first pre-requisite for any change that goes into AOSP kernel[2].

2. Once we have a core kernel that *truly* works on all Android devices, we
will have built up list of changes we want to upstream (or anyone can pick
them from our public tree). Android will still continue to move to newer
kernel versions easily (may be at a difference cadence ..)

3. About the incentive for upstream SoC support: As part of GKI, we are not
promising a forever stable kernel<->module interface. We still change it each
year. The *best way* for anyone to have their SoC / peripheral supported is
_still_ "going upstream". In fact, we advertise it as such[2]. The modularity
aspect just brings a much needed flexibility for execution.  The flexibility
is needed given the number of stakeholders involved just in the kernel as of
today. (Its a mix of Upstream, Google, SoC manufacturer, device maker and
many other small parts).


4. We also haven't been so keen on the "unloading" of a module. We know there
were subsystems where unloading may not work as expected. Then again, to my
knowledge, nobody has been stress testing with 500+ different modules that
register to all core frameworks being loaded and unloaded at random times.
Even if someone did, we just didn't think its worth the hassle or time at
this moment. Unloading the module didn't buy us anything. (Although, I do get
the point about "correctness"  -- so it shouldn't also be obviously broken)

- ssp

1. https://android.googlesource.com/kernel/common-patches/
2. https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4/README.md