Messages in this thread | | | Subject | Re: [GIT PULL]: libnvdimm updates for v4.19-rc1 | From | Dave Jiang <> | Date | Mon, 20 Aug 2018 10:21:43 -0700 |
| |
On 08/18/2018 04:15 PM, Linus Torvalds wrote: > 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. > > Linus >
Linus, I have addressed the mistake by changing the function directly to return PXX_SHIFT instead of size and removed the offending ilog2() call. I have pushed the branch to kernel.org nvdimm tree for linux-next soaking. If it's acceptable to you I'll send the pull request again later this week for 4.19 inclusion. Thank you!
| |