nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Dave Jiang <dave.jiang@intel.com>,
	Dan Williams <dan.j.williams@intel.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [GIT PULL]: libnvdimm updates for v4.19-rc1
Date: Sat, 18 Aug 2018 16:15:05 -0700	[thread overview]
Message-ID: <CA+55aFy18-r8FM614FWo8o0T2yOSy9rvv1hX50EOij84v1n1-g@mail.gmail.com> (raw)
In-Reply-To: <1b332ae402bfc3806b834ee2c050dbc52b7eeb01.camel@intel.com>

On Fri, Aug 17, 2018 at 9:17 AM Jiang, Dave <dave.jiang@intel.com> wrote:
> Please pull to receive libnvdimm contributions for v4.19-rc1

So I don't care about the libnvdimm code itself, but when you guys add
code to the core mm/ code, I start looking.

And when I then see shit like this:

        if (is_zone_device_page(p))
                tk->size_shift = ilog2(dev_pagemap_mapping_size(p, vma));

I go "No".

There's two issues with this:

 - the damn thing can return 0, which would be an error for ilog2, and
the result is undefined

   You never check for errors. There's a check for tk->size_shift ==
0, but is that actually the guaranteed return value of ilog2(0)? No.

 - there is exactly one user of dev_pagemap_mapping_size(), and the above is it.

   Why the hell didn't that function just return the number of bits to
begin with?

I do not care if you screw up your own particular driver that much.

But when I see a pull request with complete and utter garbage in the
core mm part, I will not pull.

This is not acceptable.

Pulled, merge conflict fixed, and then immediately unpulled again.

I do not want to *EVER* see these kinds of patches to core MM code.
And I'm not gfoing to pull these patches or anythinig that looks like
it has any trace of this shit.

I get upset, because dammit, I expect better. I don't want to go "oh,
this changes core code, let's just skim over the patches" and
immediately find something fundamentally broken like this.

Linux-nvdimm mailing list

  reply	other threads:[~2018-08-18 23:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-17 16:17 [GIT PULL]: libnvdimm updates for v4.19-rc1 Jiang, Dave
2018-08-18 23:15 ` Linus Torvalds [this message]
2018-08-19  5:08   ` Dan Williams
2018-08-20 17:21   ` Dave Jiang

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:

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

  git send-email \
    --in-reply-to=CA+55aFy18-r8FM614FWo8o0T2yOSy9rvv1hX50EOij84v1n1-g@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \


* 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;
as well as URLs for NNTP newsgroup(s).