From: Ingo Molnar <mingo@kernel.org>
To: Matthew Wilcox <willy@linux.intel.com>
Cc: akpm@linux-foundation.org, hpa@zytor.com, mingo@elte.hu,
tglx@linutronix.de, linux-kernel@vger.kernel.org,
Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: + x86-add-support-for-pud-sized-transparent-hugepages-checkpatch-fixes.patch added to -mm tree
Date: Thu, 10 Mar 2016 10:37:50 +0100 [thread overview]
Message-ID: <20160310093750.GA2452@gmail.com> (raw)
In-Reply-To: <20160309165507.GA2464@linux.intel.com>
* Matthew Wilcox <willy@linux.intel.com> wrote:
> On Wed, Mar 09, 2016 at 01:08:08PM +0100, Ingo Molnar wrote:
> > * Matthew Wilcox <willy@linux.intel.com> wrote:
> > > I have no idea what it means. This is copy-and-change of the pmd version,
> > > which was originally commit db3eb96f4e6281b84dd33c8980dacc27f2efe177 by
> > > Andrea.
> >
> > It means that we don't want to copy-and-change a crappy comment that slipped
> > through 5 years ago, we want to copy-and-improve. I even suggested the comment
> > improvement (which needs to be checked though).
>
> The "it" in my sentence referred to the comment. I have no idea what
> the comment is supposed to mean. I am the worst person to figure out
> what the comment is supposed to mean as I have the least experience with
> the code here.
>
> The PUD and PMD code should be as similar as possible, down to the
> comments and the spacing. If you want the original fixed, that's fine,
> and I'm willing to include it as part of this patch set. But it's not
> my responsibility to fix up the comments that you don't like.
>
> > > It seems unfair to ask me to do better than what is there right now.
> >
> > It's absolutely fair for maintainers to require the improvement of existing code
> > you want to modify, especially when you are complicating existing code ...
>
> I'm not complicating it. I'm duplicating it.
I don't think your language lawyering is particularly constructive: you are adding
new functionality to existing x86 code, and as such you need to address review
feedback from x86 maintainers - even if it involves old code.
( There's an obvious maintainability threshold concern behind such requests from
maintainers: existing bad practices in old code accumulate, and the code can
bear only so much complexity, so there's a level over which we require cleanups
to existing code before we accept new changes. )
This is nothing new, this happens all the time, it's a routine review practice
when new patches are applied.
Anyway, until my concerns are addressed the x86 bits are NAK-ed:
NAKed-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
next prev parent reply other threads:[~2016-03-10 9:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-02 22:53 + x86-add-support-for-pud-sized-transparent-hugepages-checkpatch-fixes.patch added to -mm tree akpm
2016-02-03 7:48 ` Ingo Molnar
2016-03-04 20:30 ` Matthew Wilcox
2016-03-09 12:08 ` Ingo Molnar
2016-03-09 16:55 ` Matthew Wilcox
2016-03-10 9:37 ` Ingo Molnar [this message]
2016-03-10 14:39 ` Matthew Wilcox
2016-03-09 17:40 ` Andrea Arcangeli
2016-03-09 18:45 ` Matthew Wilcox
2016-03-09 20:03 ` Matthew Wilcox
2016-03-09 23:08 ` Andrea Arcangeli
-- strict thread matches above, loose matches on Subject: below --
2016-01-27 20:30 akpm
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=20160310093750.GA2452@gmail.com \
--to=mingo@kernel.org \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
--cc=willy@linux.intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.