All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Christian Brauner <brauner@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Pankaj Raghav <p.raghav@samsung.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	djwong@kernel.org, ritesh.list@gmail.com,
	linuxppc-dev@lists.ozlabs.org, christophe.leroy@csgroup.eu
Subject: Re: linux-next: boot warning after merge of the vfs-brauner tree
Date: Mon, 26 Aug 2024 15:48:18 +0000	[thread overview]
Message-ID: <20240826154818.hzqnvofdmaxvuwrh@quentin> (raw)
In-Reply-To: <20240826175931.1989f99e@canb.auug.org.au>

On Mon, Aug 26, 2024 at 05:59:31PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the vfs-brauner tree, today's linux-next boot test (powerpc
> pseries_le_defconfig) produced this warning:

iomap dio calls set_memory_ro() on the page that is used for sub block
zeroing.

But looking at powerpc code, they don't support set_memory_ro() for
memory region that belongs to the kernel(LINEAR_MAP_REGION_ID).

/*
 * On hash, the linear mapping is not in the Linux page table so
 * apply_to_existing_page_range() will have no effect. If in the future
 * the set_memory_* functions are used on the linear map this will need
 * to be updated.
 */
if (!radix_enabled()) {
        int region = get_region_id(addr);

        if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
                return -EINVAL;
}

We call set_memory_ro() on the zero page as a extra security measure.
I don't know much about powerpc, but looking at the comment, is it just
adding the following to support it in powerpc:

diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index ac22bf28086fa..e6e0b40ba6db4 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -94,7 +94,9 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
        if (!radix_enabled()) {
                int region = get_region_id(addr);
 
-               if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
+               if (WARN_ON_ONCE(region != VMALLOC_REGION_ID &&
+                                region != IO_REGION_ID &&
+                                region != LINEAR_MAP_REGION_ID))
                        return -EINVAL;
        }
 #endif

 If it involves changing more things and this feature will be added to
 powerpc in the future, we could drop the set_memory_ro() call from
 iomap.

 CC: Darrick(as he suggested set_memory_ro() on zero page), Leroy,
 Ritesh, ppc list

> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pageattr.c:97 change_memory_attr+0xbc/0x150
> Modules linked in:
> CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.11.0-rc5-06731-g66e0882fba22 #1
> Hardware name: IBM pSeries (emulated by qemu) POWER8 (architected) 0x4d0200 0xf000004 of:SLOF,HEAD pSeries
> NIP:  c00000000008a1ac LR: c00000000008a14c CTR: 0000000000000000
> REGS: c0000000049b7930 TRAP: 0700   Not tainted  (6.11.0-rc5-06731-g66e0882fba22)
> MSR:  8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 84000482  XER: 00000000
> CFAR: c00000000008a218 IRQMASK: 0 
> GPR00: c00000000008a14c c0000000049b7bd0 c00000000167b100 0000000000000000 
> GPR04: 0000000000000001 0000000000000000 0000000000000200 c000000002b10878 
> GPR08: 000000007da60000 c007ffffffffffff ffffffffffffffff 0000000084000482 
> GPR12: 0000000000000180 c000000002b90000 c00000000001110c 0000000000000000 
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000001562288 
> GPR24: c000000002003e6c c000000001632418 000000000000018c c0000000020c1058 
> GPR28: 0000000000000000 0000000000000000 c000000006330000 0000000000000001 
> NIP [c00000000008a1ac] change_memory_attr+0xbc/0x150
> LR [c00000000008a14c] change_memory_attr+0x5c/0x150
> Call Trace:
> [c0000000049b7bd0] [000000000000018c] 0x18c (unreliable)
> [c0000000049b7c10] [c00000000206bf70] iomap_dio_init+0x64/0x88
> [c0000000049b7c30] [c000000000010d98] do_one_initcall+0x80/0x2f8
> [c0000000049b7d00] [c000000002005c9c] kernel_init_freeable+0x32c/0x520
> [c0000000049b7de0] [c000000000011138] kernel_init+0x34/0x26c
> [c0000000049b7e50] [c00000000000debc] ret_from_kernel_user_thread+0x14/0x1c
> --- interrupt: 0 at 0x0
> Code: 60000000 e8010050 eba10028 7c6307b4 ebc10030 38210040 ebe1fff8 7c0803a6 4e800020 7bc92720 2c29000c 41820058 <0fe00000> 4800002c 60000000 60000000 
> ---[ end trace 0000000000000000 ]---
> 
> Bisected to commit
> 
>   d940b3b7b76b ("iomap: fix iomap_dio_zero() for fs bs > system page size")
> 
> I have reverted commit
> 
>   9b0ebbc72358 ("Merge patch series "enable bs > ps in XFS"")
> 
> for today.
> 
> -- 
> Cheers,
> Stephen Rothwell



-- 
Pankaj Raghav

  reply	other threads:[~2024-08-26 15:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-26  7:59 linux-next: boot warning after merge of the vfs-brauner tree Stephen Rothwell
2024-08-26 15:48 ` Pankaj Raghav (Samsung) [this message]
2024-08-26 17:43   ` Christophe Leroy
2024-08-26 20:52     ` Luis Chamberlain
2024-08-26 21:10       ` Darrick J. Wong
2024-08-26 21:41         ` Luis Chamberlain
2024-08-27  5:26           ` Ritesh Harjani
2024-08-27 15:38       ` Mike Rapoport
2024-08-27  6:28   ` Michael Ellerman

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=20240826154818.hzqnvofdmaxvuwrh@quentin \
    --to=kernel@pankajraghav.com \
    --cc=brauner@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=djwong@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mcgrof@kernel.org \
    --cc=p.raghav@samsung.com \
    --cc=ritesh.list@gmail.com \
    --cc=sfr@canb.auug.org.au \
    /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.