X86 platform drivers
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Jean Delvare <jdelvare@suse.de>
Cc: Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>,
	platform-driver-x86@vger.kernel.org
Subject: Re: intel_turbo_max_3 non-modularity
Date: Fri, 28 Apr 2017 13:44:37 -0400	[thread overview]
Message-ID: <20170428174437.GO23334@windriver.com> (raw)
In-Reply-To: <20170428094930.6ba0ba1f@endymion>

[Re: intel_turbo_max_3 non-modularity] On 28/04/2017 (Fri 09:49) Jean Delvare wrote:

> Hi Paul,
> 
> On Mon, 24 Apr 2017 10:02:18 -0400, Paul Gortmaker wrote:
> > [intel_turbo_max_3 non-modularity] On 24/04/2017 (Mon 11:31) Jean Delvare wrote:
> > 
> > > I see that the intel_turbo_max_3 driver was originally supposed to be
> > > modular, and then support for that possibility was removed. Is there
> > > any fundamental reason why this driver can't be built as a module?
> > 
> > Re-reading the thread, it was stated that the driver needed exports of
> > some scheduler functions that weren't currently exported.  To me, that
> > means one of two things -- the driver is poking at scheduler internals
> > that it shouldn't be and needs to be modified accordingly, or that
> > someone needs to convince the sched folks that there is valid use cases
> > for exports of these functions, and _then_ enable modularity.
> > 
> > I didn't try to build it as a module, so I can't say what the sched
> > dependencies were, or if they looked valid.  But I have seen the sched
> > folks being not impressed at how certain arch specific PM/freq stuff has
> > been implemented in the past, which is why I mention it as a possibility.
> 
> Thanks for the explanations. I understand that the purpose of your
> patch (commit af050abb5c2e, "platform/x86: intel_turbo_max_3: make it
> explicitly non-modular") was to get things square, and I have seen many
> similar patches from you in the past. However please realize that going
> for the easiest solution (dropping modular support altogether) isn't
> always the best thing to do.

I don't think saying that "always using the easiest solution" is at all
an accurate characterization.  I've nearly always indicated that
conversion to tristate was an option _if_ there was a use case and/or if
maintainers wanted that; and indeed I've converted several drivers to
tristate when requested.  I've also indicated many times why converting
to tristate can't be the default choice.  I won't repeat them all again
here.

Also, as you've now seen, the scheduler exports that I warned about are
indeed contentious, so the non-modular path taken here was valid, given
that background knowledge.

> 
> It's the same story as fixing compiler warnings. One should either
> look for the right way to fix them, or leave things as they are, to
> give others a chance to step in. Similar to compiler warnings,
> half-modular code is also an opportunity to make things better. In my
> experience, once the code is cleanly non-modular, the chances that
> someone looks into why it is so and whether it could be modularized
> get significantly lower.

I'd counter that the dead code would remain there untouched.  At least
this way we are aware of what is dead code, and by shining a light on
that, we can decide whether to remove it or properly modularize it; and
now because of doing this we do indeed have more modular drivers than we
had before.

> 
> Non-modular drivers do not scale. This is a serious issue considering
> the growth rate of the kernel.

Again, this is an oversimplification that doesn't reflect reality.
There are many valid reasons why some drivers be built-in only.  The
whole world isn't a distro looking to say "=m" to every config option
simply because they don't have the time or resources to actually triage
them all and see what is relevant for their use case(es).

Paul.

  reply	other threads:[~2017-04-28 17:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-24  9:31 intel_turbo_max_3 non-modularity Jean Delvare
2017-04-24 14:02 ` Paul Gortmaker
2017-04-27 21:48   ` Darren Hart
2017-04-27 23:18     ` Srinivas Pandruvada
2017-04-28  7:55       ` Peter Zijlstra
2017-04-28 13:20         ` Jean Delvare
2017-04-28 13:46           ` Peter Zijlstra
2017-04-28 15:49             ` Darren Hart
2017-04-28 16:34               ` Peter Zijlstra
2017-04-28 20:58                 ` Darren Hart
2017-04-28 21:39                   ` Peter Zijlstra
2017-04-28  8:40       ` Jean Delvare
2017-04-28 20:35         ` Srinivas Pandruvada
2017-04-28  7:49   ` Jean Delvare
2017-04-28 17:44     ` Paul Gortmaker [this message]
2017-04-28 20:49       ` Darren Hart
2017-04-28 21:34         ` Peter Zijlstra
2017-04-28 22:51           ` Darren Hart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170428174437.GO23334@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=andy@infradead.org \
    --cc=dvhart@infradead.org \
    --cc=jdelvare@suse.de \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox