All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Yang <richardw.yang@linux.intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Wei Yang <richardw.yang@linux.intel.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	dave.hansen@linux.intel.com, luto@kernel.org,
	peterz@infradead.or
Subject: Re: [PATCH 4/6] x86, mm: make split_mem_range() more easy to read
Date: Thu, 28 Mar 2019 11:35:48 +0800	[thread overview]
Message-ID: <20190328033548.GA19364@richard> (raw)
In-Reply-To: <alpine.DEB.2.21.1903232345500.1798@nanos.tec.linutronix.de>

On Sun, Mar 24, 2019 at 03:29:04PM +0100, Thomas Gleixner wrote:
>Wei,
>
>On Tue, 12 Feb 2019, Wei Yang wrote:
>> 
>> This patch changes the implementation from the first perception to the
>> second to reduce one different handling on end_pfn. After doing so, the
>> code is easier to read.
>
>It's maybe slightly easier to read, but it's still completely unreadable
>garbage.
>
>  Not your fault, it was garbage before.
>
>But refining garbage still results in garbage. Just the smell is slightly
>different.
>
>Why?
>
> 1) Doing all the calculations PFN based is just a pointless
>    indirection. Just look at all the rounding magic and back and forth
>    conversions.
>    
>    All of that can be done purely address/size based which makes the code
>    truly readable.
>
> 2) The 5(3) sections are more or less copied code with a few changes of
>    constants, except for the first section (A) which has an extra quirk
>    for 32bit. Plus all the 64bit vs. 32bit #ifdeffery which is not making
>    it any better.
>
>    This copied mess can be avoided by using helper functions and proper
>    loops.
>
> 3) During the bootmem phase the code tries to preserve large mappings
>    _AFTER_ splitting them up and then it tries to merge the resulting
>    overlaps.
>
>    This is completely backwards because the expansion of the range can be
>    tried right away when then mapping of a large page is attempted. Surely
>    not with the current mess, but with a proper loop based approach it can
>    be done nicely.
>
>    Btw, there is a bug in that expansion code which could result in
>    undoing the enforced 4K mapping of the lower 2M/4M range on 32bit. It's
>    probably not an issue in practice because the low range is usually not
>    contiguous. But it works by chance not by design.

Hi, Thomas

I want to confirm with you, this bug is in adjust_range_page_size_mask(),
right?

-- 
Wei Yang
Help you, Help me

  parent reply	other threads:[~2019-03-28  3:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-12  2:12 [PATCH 0/6] x86, mm: refine split_mem_range a little Wei Yang
2019-02-12  2:12 ` [PATCH 1/6] x86, mm: remove second argument of split_mem_range() Wei Yang
2019-03-24 14:38   ` Thomas Gleixner
2019-03-25  1:16     ` Wei Yang
2019-02-12  2:12 ` [PATCH 2/6] x86, mm: remove check in save_mr Wei Yang
2019-02-12  2:12 ` [PATCH 3/6] x86, mm: add comment for split_mem_range to help understanding Wei Yang
2019-02-12  2:12 ` [PATCH 4/6] x86, mm: make split_mem_range() more easy to read Wei Yang
2019-03-24 14:29   ` Thomas Gleixner
2019-03-27 22:05     ` Wei Yang
2019-03-28  0:16       ` Thomas Gleixner
2019-03-28  0:25         ` Wei Yang
2019-03-28  3:35     ` Wei Yang [this message]
2019-03-28  7:20     ` Wei Yang
2019-03-28  8:08       ` Thomas Gleixner
2019-03-29  3:38         ` Wei Yang
     [not found]     ` <20190328065117.GA6202@richard>
2019-03-28  8:02       ` Thomas Gleixner
2019-04-12  3:08     ` Wei Yang
2019-02-12  2:12 ` [PATCH 5/6] x86, mm: skip 1G range if the range doesn't span PUD Wei Yang
2019-02-12  2:12 ` [PATCH 6/6] x86, mm: x86, mm: jump to split only 4K range when range doesn't span PMD Wei Yang

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=20190328033548.GA19364@richard \
    --to=richardw.yang@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=peterz@infradead.or \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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.