public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: Parag Warudkar <parag.lkml@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Andreas Gruenbacher <agruen@suse.de>,
	Jeff Mahoney <jeffm@suse.de>
Subject: Re: [patch 00/04] RFC: Staging tree (drivers/staging)
Date: Thu, 25 Sep 2008 15:04:54 -0700	[thread overview]
Message-ID: <20080925220454.GA12116@suse.de> (raw)
In-Reply-To: <f7848160809251440jfcd60f9lb962a75ff01798c8@mail.gmail.com>

On Thu, Sep 25, 2008 at 05:40:28PM -0400, Parag Warudkar wrote:
> On Thu, Sep 25, 2008 at 4:53 PM, Greg KH <gregkh@suse.de> wrote:
> >
> > Heh, ok, so the basic premise of getting code that is not currently in
> > mergable shape into the tree earlier to get wider usage and testing is
> > something that you agree with?
> 
> I don't think I got my point across - let me try again.
> 
> My disagreement was with getting crap code in mainline, re-classifying
> it as something other than experimental when it is not, expecting
> users to use it, printing warning when it is used, tainting the kernel
> for even more harassment and then having developers not support it  as
> a bonus - all cumulative. The naming part was also something that I
> did not like but that like I said was just my preference and does not
> matter because I disagree there is no need to TAINT  in first place.

I'm sorry we disagree here.  Also note that this goes against what a
number of us decided at the kernel summit, we want this code into the
tree as easily as possible.

> I can imagine that getting crap code in mainline is helpful for some
> class of users and developers - no problem we can have it in on that
> basis - I give up my objection there.

Great.

> But my objections to the other parts remain -  i.e. we should
> definitely avoid the useless re-classification and TAINT, we should
> change little or no other kernel code in the process of integrating
> this crap, give enough deterrent for people to really think before
> loading the crap code, and do not make it an official statement that
> you will get no support on LKML if you load this driver. Because
> certainly not all developers agree and we by definition of this
> staging effort have an interest in fixing the code.
> 
> The below approach I referred few times earlier will allow us to do
> exactly that - it would be good if you can tell me if this is
> workable.

So you now agree with the idea, just the implementation of the area
around the drivers, nice that's easy to work out.

> 1) Give a staging directory for all such low quality crap

Done in the patches I sent out.

> 2) Give a KConfig group "Staging Drivers (Low Quality/High Risk)" and
> if that is selected allow users to individually select the crappy
> drivers they want to actually use - default all entries to N

Done in the patches I sent out.

> 3) Name all modules under staging  with a _stg suffix or something unique

A name doesn't really matter here, the modinfo tag is just as
sufficient like I already did.

> 4) By default do NOT load anything with a _stg suffix - deal with this
> in insmod code, not the kernel

Um, no, I'm not going to force all userspace users to upgrade their
version of module-init-tools, JUST to prevent them from automatically
loading these drivers.  That's not going to happen, sorry.

> 5) Require that -f be specified to load _stg modules - which will
> auto-taint the kernel and developers can decide on their judgment /
> situation whether or not to pursue it

So you have to load the modules by "hand"?  Ick, no, that too is not
acceptable as it means no one will ever do it.  Don't break the
wonderful infrastructure we already have around the kernel of
auto-loading the proper driver for the proper hardware just because you
want to make the user jump through an extra step please.

>  6) OOPS reports with force loaded taint and _stg in module list will
> allow developers to decide whether or not to go after it

My patch set already does this, and it does so in a way that uniquely
distinguishes from the -f option, which some of us don't want to debug
as well.

> The above approach avoids re-classification/TAINTING, touches no
> kernel code and does not load the crap code automatically and still
> allows you to do what you intended.

What's the objection for the 9 extra lines in module.c, 2 lines in
panic.c, and 9 lines on scripts/mod/modpost.c?  Is this some huge
overhead that is unmaintainable?

Heck, the insmod changes would be more than that I imagine (and it would
be in modprobe, not insmod, it has to do with alias matching and that
chunk of code is _nasty_, I sure don't want to touch that.)

> Hopefully this doesn't sound like another "let us arm wrestle on what
> to name it" - because it is clearly more than that!

No, I really think it isn't more than that.  For some reason you are
afraid of another taint flag, why?  There is no extra overhead, and no
user needs to upgrade any userspace component at all (which, if you have
any experience with, you will know it just will not happen fast, if at
all for the large majority of users/developers.)

I don't want to make the kernel require a new version of
module-init-tools, and my current proposal of infrastructure changes
requires a massive core change of:

 Documentation/sysctl/kernel.txt |    1 +
 include/linux/kernel.h          |    1 +
 kernel/module.c                 |    9 +++++++++
 kernel/panic.c                  |    6 ++++--
 scripts/mod/modpost.c           |    9 +++++++++
5 files changed, 24 insertions(+), 2 deletions(-)

How much smaller do you expect this to get?

thanks,

greg k-h

  reply	other threads:[~2008-09-25 22:08 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080924224638.514504825@mini.kroah.org>
2008-09-24 23:00 ` [patch 00/04] RFC: Staging tree (drivers/staging) Greg KH
2008-09-24 23:01   ` [patch 01/04] Staging: add TAINT_CRAP for all drivers/staging code Greg KH
2008-09-24 23:01   ` [patch 02/04] Staging: add TAINT_CRAP flag to drivers/staging modules Greg KH
2008-09-24 23:01   ` [patch 04/04] USB: add princeton instruments usb camera driver Greg KH
2008-09-24 23:01   ` [patch 03/04] Staging: add Kconfig entries and Makefile infrastructure Greg KH
2008-09-24 23:39   ` [patch 00/04] RFC: Staging tree (drivers/staging) Parag Warudkar
2008-09-25  1:03     ` Randy Dunlap
2008-09-25  2:06       ` Greg KH
2008-09-25  2:06     ` Greg KH
2008-09-25  2:59       ` Parag Warudkar
2008-09-25  4:21         ` Greg KH
2008-09-25 11:02           ` Parag Warudkar
2008-09-25 20:53             ` Greg KH
2008-09-25 21:40               ` Parag Warudkar
2008-09-25 22:04                 ` Greg KH [this message]
2008-09-25 22:22                   ` Parag Warudkar
2008-09-26 18:36                     ` Stefan Richter
2008-09-26 20:11                       ` Parag Warudkar
2008-09-26 20:19                         ` Greg KH
2008-09-26 20:56                           ` Parag Warudkar
2008-09-26 22:03                             ` Greg KH
2008-09-26 21:00                           ` Leon Woestenberg
2008-09-26 22:04                             ` Greg KH
2008-09-26 20:39                         ` Stefan Richter
2008-09-26 20:47                           ` Parag Warudkar
2008-09-26 22:46                             ` Stefan Richter
2008-09-25  5:27         ` Paul Mundt
2008-09-25 14:49           ` Randy Dunlap
2008-09-25 17:53             ` Randy Dunlap
2008-09-25 20:48               ` Greg KH
2008-09-25 21:04                 ` Randy Dunlap
2008-09-25 21:51                   ` Stefan Richter
2008-10-06 15:11               ` config_experimental was " Pavel Machek
2008-10-09 21:01   ` Adrian Bunk
2008-10-09 21:08     ` Greg KH
2008-10-09 21:17     ` Andrew Morton

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=20080925220454.GA12116@suse.de \
    --to=gregkh@suse.de \
    --cc=agruen@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=jeffm@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=parag.lkml@gmail.com \
    --cc=torvalds@linux-foundation.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