From: Josh Boyer <jwboyer@redhat.com>
To: Sasha Levin <sasha.levin@oracle.com>
Cc: John Hubbard <jhubbard@nvidia.com>,
"john.hubbard@gmail.com" <john.hubbard@gmail.com>,
"Kirill A. Shutemov" <kirill@shutemov.name>,
"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH] A long explanation for a short patch
Date: Fri, 14 Mar 2014 12:53:32 -0400 [thread overview]
Message-ID: <20140314165332.GH16145@hansolo.jdub.homelinux.org> (raw)
In-Reply-To: <53231BB3.20205@oracle.com>
On Fri, Mar 14, 2014 at 11:09:39AM -0400, Sasha Levin wrote:
> On 03/14/2014 09:42 AM, Josh Boyer wrote:
> >On Thu, Mar 13, 2014 at 09:47:50PM -0700, John Hubbard wrote:
> >>>To keep it short, my opinion is that since it doesn't break any existing
> >>>code it should be kept as _GPL(), same way it was done for various other
> >>>subsystems.
> >
> >I'm not entirely sure that's an apples to apples comparison, or that
> >some of those statements are even accurate. I really don't want to get
> >into that debate though.
>
> Fair enough. I'm not trying to say that it's exactly the same issue, but
> only that such issues occurred in the past and were dealt differently by
> Fedora. From the text of the Fedora bug report on the subject:
>
> """
> A similar conflict exists between the nvidia.ko module and kernels with
> CONFIG_DEBUG_LOCK_ALLOC enabled: Fedora alpha kernels have this option
> enabled by default, and Fedora beta and release kernels have it disabled.
> """
Right. We can do that with DEBUG_VM as well if needs be. I suppose the
thing that gave me pause here is that the highlighted example was an
issue with a proprietary module whereas this one is permissively
licensed (more permissively than GPL even). Unfortunately, there is no
EXPORT_SYMBOL_OPENSOURCE or equivalent today, which means things break
somewhat unnecessarily.
This entire issue could be avoided if the UVM-lite module had:
MODULE_LICENSE("Dual MIT/GPL")
instead of just "MIT". John, is that something that is possible?
> >>>Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
> >>>thing to do. I agree you'll find more bugs, but you'll also hit one of the many
> >>>false-positives hidden there as well. I've reported a few of those but
> >>>in some cases it's hard to determine whether it's an actual false-positive
> >>>or a bug somewhere else. Since the assumption is that end-users won't
> >>>have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
> >>>end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .
> >
> >The last time this came up not that long ago, various MM people were
> >surprised we had it enabled but overall supportive. I think Hugh and
> >Dave were involved in those discussions. I'm certainly willing to
> >revisit having it enabled if the state of the code has changed since
> >then.
> >
> >However, if the things wrapped in DEBUG_VM aren't getting the attention
> >they deserve, are causing false-positives, and generally aren't
> >maintained, then perhaps having a config option to enable them isn't a
> >great idea. Users will toggle any and all config options that are out
> >there and making assumptions that they aren't going to be enabled by
> >users is pretty silly. Perhaps just make them depend on modifying a
> >#debug define at the top of each file in question? That way "end users"
> >won't easily be able to turn on broken debug code.
>
> I didn't want to imply that DEBUG_VM issues are getting pushed aside and
> ignored because "no one will see it". DEBUG_VM issues are being treated
> like any other mm/ issue, but they do get placed lower on a lower priority
> than issues that happen without DEBUG_VM due to lack of "manpower".
>
> There's also no assumption that they won't get enabled, the only assumption
> is that if you enable it then you know what you're doing.
Hm. It's enabled by default in defconfigs for tegra (ARM), blackfin,
s390, sh, and tile machines. I suppose the "end user" of those
architectures knows what they're doing. It still might be worthwhile to
add some cautionary wording to the Kconfig help text. Today that says:
"Enable this to turn on extended checks in the virtual-memory system
that may impact performance."
Extended checks sound good. Sounds like it would make things safer.
Why wouldn't I want them?
Perhaps this would be more accurate:
"Enable this to turn on debugging infrastructure used by kernel
virtual-memory developers. This may impact performance. If you are not
debugging the Linux kernel virtual-memory subsystem, say N."
josh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-03-14 16:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-14 2:30 john.hubbard
2014-03-14 2:30 ` [PATCH] Change mm debug routines back to EXPORT_SYMBOL john.hubbard
2014-03-14 4:36 ` [PATCH] A long explanation for a short patch Sasha Levin
2014-03-14 4:47 ` John Hubbard
2014-03-14 13:42 ` Josh Boyer
2014-03-14 15:09 ` Sasha Levin
2014-03-14 16:53 ` Josh Boyer [this message]
2014-04-01 23:08 ` 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=20140314165332.GH16145@hansolo.jdub.homelinux.org \
--to=jwboyer@redhat.com \
--cc=jhubbard@nvidia.com \
--cc=john.hubbard@gmail.com \
--cc=kirill@shutemov.name \
--cc=linux-mm@kvack.org \
--cc=sasha.levin@oracle.com \
/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