All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Yaohui Wang <yaohuiwang@linux.alibaba.com>, dave.hansen@linux.intel.com
Cc: luto@kernel.org, peterz@infradead.org, mingo@redhat.com,
	bp@alien8.de, x86@kernel.org, linux-kernel@vger.kernel.org,
	Ben Luo <luoben@linux.alibaba.com>,
	Yahui Wang <yaohuiwang@linux.alibaba.com>
Subject: Re: [PATCH v2 2/2] mm: fix boundary judgment issues in kernel/resource.c
Date: Sun, 20 Jun 2021 00:16:45 +0200	[thread overview]
Message-ID: <87k0mph54i.ffs@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20210611042147.80744-3-yaohuiwang@linux.alibaba.com>

Yaohui!

On Fri, Jun 11 2021 at 12:21, Yaohui Wang wrote:

The same formal issues as with patch #1

> The original boundary judgment may ignore @end if @end equals @start. For

May means it can but it must not. But this is not the case here. end
equals start is always ignored.

Also 'original' is meaningless here. Before the patch is applied the
code is that way.

 find_next_iomem_res() and __walk_iomem_res_desc() require that the
 provided end address is larger than the start address, which ...


> example, if we call ioremap(phys, 1), then @end == @start, and the memory
> check will not be applied on the page where @end lives, which is
> unexpected.

Please avoid 'we' and 'I':

 is incorrect when ioremap() is invoked with length=1.

> In kernel/resource.c:find_next_iomem_res, the mem region is a closed

See the reply to #1 vs. function names. Also please write out 'memory',
there is no shortage of space in change logs.

> interval (i.e. [@start..@end]). So @start == @end should be allowed.

closed interval reads strange. The usual terminology is: The end address
is inclusive.

  Resources are described with the start address and the inclusive end
  address, which means for a resource with 1 byte length the start
  address is the same as the end address.

  find_next_iomem_res() and __walk_iomem_res_desc() ignore resources
  with 1 byte length, which prevents that ioremap(phys, 1) is checked
  whether it touches non ioremappable resources.

  ...

Thanks,

        tglx

  reply	other threads:[~2021-06-19 22:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11  4:21 [PATCH v2 0/2] mm: unexpected behavior of __ioremap_caller Yaohui Wang
2021-06-11  4:21 ` [PATCH v2 1/2] mm: fix the pfn calculation mistake in __ioremap_check_ram Yaohui Wang
2021-06-19 21:22   ` Thomas Gleixner
2021-06-21  8:06     ` Thomas Gleixner
2021-06-11  4:21 ` [PATCH v2 2/2] mm: fix boundary judgment issues in kernel/resource.c Yaohui Wang
2021-06-19 22:16   ` Thomas Gleixner [this message]
2021-06-21  6:12     ` Yaohui Wang

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=87k0mph54i.ffs@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luoben@linux.alibaba.com \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    --cc=yaohuiwang@linux.alibaba.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.