Re: [PATCH 1/2] drivers/clocksource/timer-of: Remove __init markings

[Saravana Kannan]

On Tue, Apr 28, 2020 at 12:02 AM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:
>
>
> Hi Saravana,

You were replying to Sandeep :)

> On 28/04/2020 00:17, Sandeep Patil wrote:
> > 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@xxxxxxxxxx> wrote:
> >>>>
> >>>> On 27/04/2020 21:04, Saravana Kannan wrote:
> >>>>> On Mon, Apr 27, 2020 at 10:13 AM Daniel Lezcano
> >>>>> <daniel.lezcano@xxxxxxxxxx> 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@xxxxxxxxx> wrote:
> >>>>>>>>
> >>>>>>>> From: Saravana Kannan <saravanak@xxxxxxxxxx>
> >>>>>>>>
> >>>>>>>> This allows timer drivers to be compiled as modules.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
> >>>>>>>> Signed-off-by: Baolin Wang <baolin.wang7@xxxxxxxxx>
> >>>>>>>
> >>>>>>> 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)
>
> That was my understanding of the GKI, thanks for confirming.
>
> Putting apart the non-technical aspect of these changes, the benefit I
> see is the memory usage optimization regarding the single kernel image.
>
> With the ARM64 defconfig, multiple platforms and their corresponding
> drivers are compiled-in. It results in a big kernel image which fails to
> load because of overlapping on DT load address (or something else). When
> that is detected, it is fine to adjust the load addresses, otherwise it
> is painful to narrow down the root cause.
>
> In order to prevent this, we have to customize the defconfig each
> version release.

Sorry, I'm not sure I understand where you are going with this. Are
you agreeing to pick up this change?

-Saravana