All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.