* Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()
From: Edgecombe, Rick P @ 2023-12-05 22:23 UTC (permalink / raw)
To: broonie@kernel.org
Cc: dietmar.eggemann@arm.com, keescook@chromium.org, shuah@kernel.org,
brauner@kernel.org, dave.hansen@linux.intel.com,
debug@rivosinc.com, mgorman@suse.de, Szabolcs.Nagy@arm.com,
fweimer@redhat.com, linux-kernel@vger.kernel.org,
mingo@redhat.com, hjl.tools@gmail.com, rostedt@goodmis.org,
vincent.guittot@linaro.org, tglx@linutronix.de,
vschneid@redhat.com, catalin.marinas@arm.com, bristot@redhat.com,
will@kernel.org, hpa@zytor.com, peterz@infradead.org,
jannh@google.com, bp@alien8.de, bsegall@google.com,
linux-kselftest@vger.kernel.org, linux-api@vger.kernel.org,
x86@kernel.org, juri.lelli@redhat.com
In-Reply-To: <ed665d6f-66b0-4eeb-8cf8-db852e017d6a@sirena.org.uk>
On Tue, 2023-12-05 at 15:51 +0000, Mark Brown wrote:
> On Tue, Dec 05, 2023 at 12:26:57AM +0000, Edgecombe, Rick P wrote:
> > On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
>
> > > - size = adjust_shstk_size(stack_size);
> > > + size = adjust_shstk_size(size);
> > > addr = alloc_shstk(0, size, 0, false);
>
> > Hmm. I didn't test this, but in the copy_process(), copy_mm()
> > happens
> > before this point. So the shadow stack would get mapped in
> > current's MM
> > (i.e. the parent). So in the !CLONE_VM case with
> > shadow_stack_size!=0
> > the SSP in the child will be updated to an area that is not mapped
> > in
> > the child. I think we need to pass tsk->mm into alloc_shstk(). But
> > such
> > an exotic clone usage does give me pause, regarding whether all of
> > this
> > is premature.
>
> Hrm, right. And we then can't use do_mmap() either. I'd be somewhat
> tempted to disallow that specific case for now rather than deal with
> it
> though that's not really in the spirit of just always following what
> the
> user asked for.
Oh, yea. What a pain. It doesn't seem like we could easily even add a
do_mmap() variant that takes an mm either.
I did a quick logging test on a Fedora userspace. systemd (I think)
appears to do a clone(!CLONE_VM) with a stack passed. So maybe the
combo might actually get used with a shadow_stack_size if it used
clone3 some day. At the same time, fixing clone to mmap() in the child
doesn't seem straight forward at all. Checking with some of our MM
folks, the suggestion was to look at doing the child's shadow stack
mapping in dup_mm() to avoid tripping over complications that happen
when a remote MM becomes more "live".
If we just punt on this combination for now, then the documented rules
for args->shadow_stack_size would be something like:
clone3 will use the parents shadow stack when CLONE_VM is not present.
If CLONE_VFORK is set then it will use the parents shadow stack only
when args->shadow_stack_size is non-zero. In the cases when the parents
shadow stack is not used, args->shadow_stack_size is used for the size
whenever non-zero.
I guess it doesn't seem too overly complicated. But I'm not thinking
any of the options seem great. I'd unhappily lean towards not
supporting shadow_stack_size!=0 && !CLONE_VM for now. But it seems like
there may be a user for the unsupported case, so this would be just
improving things a little and kicking the can down the road. I also
wonder if this is a sign to reconsider the earlier token consuming
design.
^ permalink raw reply
* Re: [RFC] proposed libc interface and man page for listmount
From: Karel Zak @ 2023-12-05 18:26 UTC (permalink / raw)
To: Miklos Szeredi
Cc: libc-alpha, linux-man, Alejandro Colomar, Linux API,
Florian Weimer, linux-fsdevel, Ian Kent, David Howells,
Christian Brauner, Amir Goldstein, Arnd Bergmann
In-Reply-To: <CAJfpeguMViqawKfJtM7_M9=m+6WsTcPfa_18t_rM9iuMG096RA@mail.gmail.com>
On Tue, Dec 05, 2023 at 05:27:58PM +0100, Miklos Szeredi wrote:
> Attaching the proposed man page for listing mounts (based on the new
> listmount() syscall).
>
> The raw interface is:
>
> syscall(__NR_listmount, const struct mnt_id_req __user *, req,
> u64 __user *, buf, size_t, bufsize, unsigned int, flags);
>
> The proposed libc API is.
>
> struct listmount *listmount_start(uint64_t mnt_id, unsigned int flags);
> uint64_t listmount_next(struct listmount *lm);
> void listmount_end(struct listmount *lm);
What about:
getmountlist()
nextmountlist()
freemountlist()
For me, _start and _end() sounds strange. For example, We already use
get+free for getaddrinfo().
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [RFC] proposed libc interface and man page for listmount
From: Matthew House @ 2023-12-05 17:51 UTC (permalink / raw)
To: Miklos Szeredi
Cc: libc-alpha, linux-man, Alejandro Colomar, Linux API,
Florian Weimer, linux-fsdevel, Karel Zak, Ian Kent, David Howells,
Christian Brauner, Amir Goldstein, Arnd Bergmann
In-Reply-To: <CAJfpeguMViqawKfJtM7_M9=m+6WsTcPfa_18t_rM9iuMG096RA@mail.gmail.com>
On Tue, Dec 5, 2023 at 11:28 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> Attaching the proposed man page for listing mounts (based on the new
> listmount() syscall).
>
> The raw interface is:
>
> syscall(__NR_listmount, const struct mnt_id_req __user *, req,
> u64 __user *, buf, size_t, bufsize, unsigned int, flags);
>
> The proposed libc API is.
>
> struct listmount *listmount_start(uint64_t mnt_id, unsigned int flags);
> uint64_t listmount_next(struct listmount *lm);
> void listmount_end(struct listmount *lm);
>
> I'm on the opinion that no wrapper is needed for the raw syscall, just
> like there isn't one for getdents(2).
>
> Comments?
One use case I've been thinking of involves inspecting the mount list
between syscall(__NR_clone3) and _exit(), so it has to be async-signal-
safe. It would be nice if there were a libc wrapper that accepted a user-
provided buffer and was async-signal-safe, so that I wouldn't have to add
yet another syscall wrapper and redefine the kernel types just for this
use case. (I can't trust the libc not to make its own funny versions of the
types' layouts for its own ends.)
Thank you,
Matthew House
^ permalink raw reply
* Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support
From: Mark Brown @ 2023-12-05 16:43 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: dietmar.eggemann@arm.com, keescook@chromium.org, shuah@kernel.org,
brauner@kernel.org, dave.hansen@linux.intel.com,
debug@rivosinc.com, mgorman@suse.de, Szabolcs.Nagy@arm.com,
fweimer@redhat.com, linux-kernel@vger.kernel.org,
mingo@redhat.com, hjl.tools@gmail.com, rostedt@goodmis.org,
vincent.guittot@linaro.org, tglx@linutronix.de,
vschneid@redhat.com, catalin.marinas@arm.com, bristot@redhat.com,
will@kernel.org, hpa@zytor.com, peterz@infradead.org,
jannh@google.com, bp@alien8.de, bsegall@google.com,
linux-kselftest@vger.kernel.org, linux-api@vger.kernel.org,
x86@kernel.org, juri.lelli@redhat.com
In-Reply-To: <a6bf192a1568620826dd79124511ea61472873c8.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]
On Tue, Dec 05, 2023 at 04:01:50PM +0000, Edgecombe, Rick P wrote:
> Hmm, I didn't realize you were planning to have the kernel support
> upstream before the libc support was in testable shape.
It's not a "could someone run it" thing - it's about trying ensure that
we get coverage from people who are just running the selftests as part
of general testing coverage rather than with the specific goal of
testing this one feature. Even when things start to land there will be
a considerable delay before they filter out so that all the enablement
is in CI systems off the shelf and it'd be good to have coverage in that
interval.
> > What's the issue with working around the missing support? My
> > understanding was that there should be no ill effects from repeated
> > attempts to enable. We could add a check for things already being
> > enabled
> Normally the loader enables shadow stack and glibc then knows to do
> things in special ways when it is successful. If it instead manually
> enables in the app:
> - The app can't return from main() without disabling shadow stack
> beforehand. Luckily this test directly calls exit()
> - The app can't do longjmp()
> - The app can't do ucontext stuff
> - The enabling code needs to be carefully crafted (the inline problem
> you hit)
> I guess it's not a huge list, and mostly tests will run ok. But it
> doesn't seem right to add somewhat hacky shadow stack crud into generic
> tests.
Right, it's a small and fairly easily auditable list - it's more about
the app than the double enable which was what I thought your concern
was. It's a bit annoying definitely and not something we want to do in
general but for something like this where we're adding specific coverage
for API extensions for the feature it seems like a reasonable tradeoff.
If the x86 toolchain/libc support is widely enough deployed (or you just
don't mind any missing coverage) we could use the toolchain support
there and only have the manual enable for arm64, it'd be inconsistent
but not wildly so.
> So you were planning to enable GCS in this test manually as well? How
> many tests were you planning to add it like this?
Yes, the current version of the arm64 series has the equivalent support
for GCS. I was only planning to do this along with adding specific
coverage for shadow stacks/GCS, general stuff that doesn't have any
specific support can get covered as part of system testing with the
toolchain and libc support.
The only case beyond that I've done is some arm64 specific stress tests
which are written as standalone assembler programs, those wouldn't get
enabled by the toolchain anyway and have some chance of catching context
switch or signal handling issues should they occur. It seemed worth it
for the few lines of assembly it takes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [RFC] proposed libc interface and man page for listmount
From: Miklos Szeredi @ 2023-12-05 16:27 UTC (permalink / raw)
To: libc-alpha, linux-man, Alejandro Colomar, Linux API,
Florian Weimer, linux-fsdevel, Karel Zak, Ian Kent, David Howells,
Christian Brauner, Amir Goldstein, Arnd Bergmann
[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]
Attaching the proposed man page for listing mounts (based on the new
listmount() syscall).
The raw interface is:
syscall(__NR_listmount, const struct mnt_id_req __user *, req,
u64 __user *, buf, size_t, bufsize, unsigned int, flags);
The proposed libc API is.
struct listmount *listmount_start(uint64_t mnt_id, unsigned int flags);
uint64_t listmount_next(struct listmount *lm);
void listmount_end(struct listmount *lm);
I'm on the opinion that no wrapper is needed for the raw syscall, just
like there isn't one for getdents(2).
Comments?
Thanks,
Miklos
Sample implementation:
--------------------------------
#define LM_BUFSIZE 4096
struct listmount {
size_t num;
size_t pos;
uint64_t mnt_id;
unsigned int flags;
uint64_t buf[LM_BUFSIZE];
};
static int do_listmount(struct listmount *lm)
{
struct mnt_id_req req = {
.mnt_id = lm->mnt_id,
.param = lm->buf[LM_BUFSIZE - 1],
};
long res;
res = syscall(__NR_listmount, &req, lm->buf, LM_BUFSIZE, lm->flags);
if (res != -1) {
lm->num = res;
lm->pos = 0;
}
return res;
}
struct listmount *listmount_start(uint64_t mnt_id, unsigned int flags)
{
int res;
struct listmount *lm = calloc(1, sizeof(*lm));
if (lm) {
lm->mnt_id = mnt_id;
lm->flags = flags;
res = do_listmount(lm);
if (res == -1) {
free(lm);
lm = NULL;
}
}
return lm;
}
uint64_t listmount_next(struct listmount *lm)
{
int res;
if (lm->pos == LM_BUFSIZE) {
res = do_listmount(lm);
if (res == -1)
return 0;
}
/* End of list? */
if (lm->pos == lm->num)
return 0;
return lm->buf[lm->pos++];
}
void listmount_end(struct listmount *lm)
{
free(lm);
}
[-- Attachment #2: listmount_start.3 --]
[-- Type: application/x-troff-man, Size: 1998 bytes --]
^ permalink raw reply
* Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support
From: Edgecombe, Rick P @ 2023-12-05 16:01 UTC (permalink / raw)
To: broonie@kernel.org
Cc: dietmar.eggemann@arm.com, keescook@chromium.org, shuah@kernel.org,
brauner@kernel.org, dave.hansen@linux.intel.com,
debug@rivosinc.com, mgorman@suse.de, Szabolcs.Nagy@arm.com,
fweimer@redhat.com, linux-kernel@vger.kernel.org,
mingo@redhat.com, hjl.tools@gmail.com, rostedt@goodmis.org,
vincent.guittot@linaro.org, tglx@linutronix.de,
vschneid@redhat.com, catalin.marinas@arm.com, bristot@redhat.com,
will@kernel.org, hpa@zytor.com, peterz@infradead.org,
jannh@google.com, bp@alien8.de, bsegall@google.com,
linux-kselftest@vger.kernel.org, linux-api@vger.kernel.org,
x86@kernel.org, juri.lelli@redhat.com
In-Reply-To: <345cf31a-3663-4974-9b2a-54d2433e64a7@sirena.org.uk>
On Tue, 2023-12-05 at 15:05 +0000, Mark Brown wrote:
> > But I wonder if the clone3 test should get its shadow stack enabled
> > the
> > conventional elf bit way. So if it's all there (HW, kernel, glibc)
> > then
> > the test will run with shadow stack. Otherwise the test will run
> > without shadow stack.
>
> This creates bootstrapping issues if we do it for arm64 where nothing
> is
> merged yet except for the model and EL3 support - in order to get any
> test coverage you need to be using an OS with the libc and toolchain
> support available and that's not going to be something we can rely on
> for a while (and even when things are merged a lot of the CI systems
> use
> Debian). There is a small risk that the toolchain will generate
> incompatible code if it doesn't know it's specifically targetting
> shadow
> stacks but the toolchain people didn't seem concerned about that risk
> and we've not been running into problems.
>
> It looks x86 is in better shape here with the userspace having run
> ahead
> of the kernel support though I'm not 100% clear if everything is
> fully
> lined up? -mshstk -fcf-protection appears to build fine with gcc 8
> but
> I'm a bit less clear on glibc and any ABI variations.
Right, you would need a shadow stack enabled compiler too. The
check_cc.sh piece in the Makefile will detect that.
Hmm, I didn't realize you were planning to have the kernel support
upstream before the libc support was in testable shape.
>
> > The other reason is that the shadow stack test in the x86 selftest
> > manual enabling is designed to work without a shadow stack enabled
> > glibc and has to be specially crafted to work around the missing
> > support. I'm not sure the more generic selftests should have to
> > know
> > how to do this. So what about something like this instead:
>
> What's the issue with working around the missing support? My
> understanding was that there should be no ill effects from repeated
> attempts to enable. We could add a check for things already being
> enabled
Normally the loader enables shadow stack and glibc then knows to do
things in special ways when it is successful. If it instead manually
enables in the app:
- The app can't return from main() without disabling shadow stack
beforehand. Luckily this test directly calls exit()
- The app can't do longjmp()
- The app can't do ucontext stuff
- The enabling code needs to be carefully crafted (the inline problem
you hit)
I guess it's not a huge list, and mostly tests will run ok. But it
doesn't seem right to add somewhat hacky shadow stack crud into generic
tests.
So you were planning to enable GCS in this test manually as well? How
many tests were you planning to add it like this?
^ permalink raw reply
* [PATCH v2] ELF: supply userspace with available page shifts (AT_PAGE_SHIFT_MASK)
From: Alexey Dobriyan @ 2023-12-05 16:01 UTC (permalink / raw)
To: Andrew Morton; +Cc: Florian Weimer, linux-kernel, linux-arch, linux-api, x86
In-Reply-To: <87v89dvuxg.fsf@oldenburg.str.redhat.com>
Report available page shifts in arch independent manner, so that
userspace developers won't have to parse /proc/cpuinfo hunting
for arch specific strings:
Note!
This is strictly for userspace, if some page size is shutdown due
to kernel command line option or CPU bug workaround, than is must not
be reported in aux vector!
x86_64 machine with 1 GiB pages:
00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
00000040 1d 00 00 00 00 00 00 00 00 10 20 40 00 00 00 00
x86_64 machine with 2 MiB pages only:
00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
00000040 1d 00 00 00 00 00 00 00 00 10 20 00 00 00 00 00
AT_PAGESZ is always 4096 which is not that interesting.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
v2: switch to 1 bit per page shift (bitmask)
arch/x86/include/asm/elf.h | 12 ++++++++++++
fs/binfmt_elf.c | 3 +++
include/uapi/linux/auxvec.h | 14 ++++++++++++++
3 files changed, 29 insertions(+)
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -358,6 +358,18 @@ else if (IS_ENABLED(CONFIG_IA32_EMULATION)) \
#define COMPAT_ELF_ET_DYN_BASE (TASK_UNMAPPED_BASE + 0x1000000)
+#define ARCH_AT_PAGE_SHIFT_MASK \
+ do { \
+ u32 val = 1 << 12; \
+ if (boot_cpu_has(X86_FEATURE_PSE)) { \
+ val |= 1 << 21; \
+ } \
+ if (boot_cpu_has(X86_FEATURE_GBPAGES)) { \
+ val |= 1 << 30; \
+ } \
+ NEW_AUX_ENT(AT_PAGE_SHIFT_MASK, val); \
+ } while (0)
+
#endif /* !CONFIG_X86_32 */
#define VDSO_CURRENT_BASE ((unsigned long)current->mm->context.vdso)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -240,6 +240,9 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
#endif
NEW_AUX_ENT(AT_HWCAP, ELF_HWCAP);
NEW_AUX_ENT(AT_PAGESZ, ELF_EXEC_PAGESIZE);
+#ifdef ARCH_AT_PAGE_SHIFT_MASK
+ ARCH_AT_PAGE_SHIFT_MASK;
+#endif
NEW_AUX_ENT(AT_CLKTCK, CLOCKS_PER_SEC);
NEW_AUX_ENT(AT_PHDR, phdr_addr);
NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr));
--- a/include/uapi/linux/auxvec.h
+++ b/include/uapi/linux/auxvec.h
@@ -33,6 +33,20 @@
#define AT_RSEQ_FEATURE_SIZE 27 /* rseq supported feature size */
#define AT_RSEQ_ALIGN 28 /* rseq allocation alignment */
+/*
+ * Page sizes available for mmap(2) encoded as bitmask.
+ *
+ * Example: x86_64 system with pse, pdpe1gb /proc/cpuinfo flags reports
+ * 4 KiB, 2 MiB and 1 GiB page support.
+ *
+ * $ hexdump -C /proc/self/auxv
+ * 00000030 06 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00
+ * 00000040 1d 00 00 00 00 00 00 00 00 10 20 40 00 00 00 00
+ *
+ * For 2^64 hugepage support please contact your Universe sales representative.
+ */
+#define AT_PAGE_SHIFT_MASK 29
+
#define AT_EXECFN 31 /* filename of program */
#ifndef AT_MINSIGSTKSZ
^ permalink raw reply
* Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()
From: Mark Brown @ 2023-12-05 15:51 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: dietmar.eggemann@arm.com, Szabolcs.Nagy@arm.com,
brauner@kernel.org, dave.hansen@linux.intel.com,
debug@rivosinc.com, mgorman@suse.de, vincent.guittot@linaro.org,
fweimer@redhat.com, mingo@redhat.com, rostedt@goodmis.org,
hjl.tools@gmail.com, tglx@linutronix.de, vschneid@redhat.com,
shuah@kernel.org, bristot@redhat.com, hpa@zytor.com,
peterz@infradead.org, bp@alien8.de, bsegall@google.com,
x86@kernel.org, juri.lelli@redhat.com, keescook@chromium.org,
jannh@google.com, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
linux-api@vger.kernel.org, will@kernel.org
In-Reply-To: <61f80d032c6a630dd641c9b598b37c2eb40d51e8.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 974 bytes --]
On Tue, Dec 05, 2023 at 12:26:57AM +0000, Edgecombe, Rick P wrote:
> On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
> > - size = adjust_shstk_size(stack_size);
> > + size = adjust_shstk_size(size);
> > addr = alloc_shstk(0, size, 0, false);
> Hmm. I didn't test this, but in the copy_process(), copy_mm() happens
> before this point. So the shadow stack would get mapped in current's MM
> (i.e. the parent). So in the !CLONE_VM case with shadow_stack_size!=0
> the SSP in the child will be updated to an area that is not mapped in
> the child. I think we need to pass tsk->mm into alloc_shstk(). But such
> an exotic clone usage does give me pause, regarding whether all of this
> is premature.
Hrm, right. And we then can't use do_mmap() either. I'd be somewhat
tempted to disallow that specific case for now rather than deal with it
though that's not really in the spirit of just always following what the
user asked for.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support
From: Mark Brown @ 2023-12-05 15:05 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: dietmar.eggemann@arm.com, Szabolcs.Nagy@arm.com,
brauner@kernel.org, dave.hansen@linux.intel.com,
debug@rivosinc.com, mgorman@suse.de, vincent.guittot@linaro.org,
fweimer@redhat.com, mingo@redhat.com, rostedt@goodmis.org,
hjl.tools@gmail.com, tglx@linutronix.de, vschneid@redhat.com,
shuah@kernel.org, bristot@redhat.com, hpa@zytor.com,
peterz@infradead.org, bp@alien8.de, bsegall@google.com,
x86@kernel.org, juri.lelli@redhat.com, keescook@chromium.org,
jannh@google.com, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
linux-api@vger.kernel.org, will@kernel.org
In-Reply-To: <4898975452179af46f38daa6979b32ba94001419.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]
On Tue, Dec 05, 2023 at 12:10:20AM +0000, Edgecombe, Rick P wrote:
> Without this diff, the test crashed for me on a shadow stack system:
> -static inline void enable_shadow_stack(void)
> +static inline __attribute__((always_inline)) void
doh.
> But I wonder if the clone3 test should get its shadow stack enabled the
> conventional elf bit way. So if it's all there (HW, kernel, glibc) then
> the test will run with shadow stack. Otherwise the test will run
> without shadow stack.
This creates bootstrapping issues if we do it for arm64 where nothing is
merged yet except for the model and EL3 support - in order to get any
test coverage you need to be using an OS with the libc and toolchain
support available and that's not going to be something we can rely on
for a while (and even when things are merged a lot of the CI systems use
Debian). There is a small risk that the toolchain will generate
incompatible code if it doesn't know it's specifically targetting shadow
stacks but the toolchain people didn't seem concerned about that risk
and we've not been running into problems.
It looks x86 is in better shape here with the userspace having run ahead
of the kernel support though I'm not 100% clear if everything is fully
lined up? -mshstk -fcf-protection appears to build fine with gcc 8 but
I'm a bit less clear on glibc and any ABI variations.
> The other reason is that the shadow stack test in the x86 selftest
> manual enabling is designed to work without a shadow stack enabled
> glibc and has to be specially crafted to work around the missing
> support. I'm not sure the more generic selftests should have to know
> how to do this. So what about something like this instead:
What's the issue with working around the missing support? My
understanding was that there should be no ill effects from repeated
attempts to enable. We could add a check for things already being
enabled
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] ELF: supply userspace with available page shifts (AT_PAGE_SHIFT_LIST)
From: Alexey Dobriyan @ 2023-12-05 14:26 UTC (permalink / raw)
To: Florian Weimer; +Cc: akpm, linux-kernel, linux-arch, linux-api, x86
In-Reply-To: <87v89dvuxg.fsf@oldenburg.str.redhat.com>
On Tue, Dec 05, 2023 at 10:51:39AM +0100, Florian Weimer wrote:
> * Alexey Dobriyan:
>
> > +/*
> > + * Page sizes available for mmap(2) encoded as 1 page shift per byte in
> > + * increasing order.
> > + *
> > + * Thus 32-bit systems get 4 shifts, 64-bit systems get 8 shifts tops.
>
> Couldn't you use the bits in a long instead, to indicate which shifts
> are present? That's always going to be enough.
Yes!
I was so proud of myself for this line:
val |= 21 << (s += 8);
Now it is boring bitmask again :-)
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: Ming Lei @ 2023-12-05 13:59 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, axboe, kbusch, sagi, jejb, martin.petersen,
djwong, viro, brauner, chandan.babu, dchinner, linux-block,
linux-kernel, linux-nvme, linux-xfs, linux-fsdevel, tytso,
jbongio, linux-api
In-Reply-To: <a87d48a7-f2a8-40ae-8d9b-e4534ccc29b1@oracle.com>
On Mon, Dec 04, 2023 at 03:19:15PM +0000, John Garry wrote:
> On 04/12/2023 13:45, Christoph Hellwig wrote:
> > On Tue, Nov 28, 2023 at 05:42:10PM +0000, John Garry wrote:
> > > ok, fine, it would not be required for XFS with CoW. Some concerns still:
> > > a. device atomic write boundary, if any
> > > b. other FSes which do not have CoW support. ext4 is already being used for
> > > "atomic writes" in the field - see dubious amazon torn-write prevention.
> >
> > What is the 'dubious amazon torn-write prevention'?
>
> https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/storage-twp.html
>
> AFAICS, this is without any kernel changes, so no guarantee of unwanted
> splitting or merging of bios.
>
> Anyway, there will still be !CoW FSes which people want to support.
>
> >
> > > About b., we could add the pow-of-2 and file offset alignment requirement
> > > for other FSes, but then need to add some method to advertise that
> > > restriction.
> >
> > We really need a better way to communicate I/O limitations anyway.
> > Something like XFS_IOC_DIOINFO on steroids.
> >
> > > Sure, but to me it is a concern that we have 2x paths to make robust a.
> > > offload via hw, which may involve CoW b. no HW support, i.e. CoW always
> >
> > Relying just on the hardware seems very limited, especially as there is
> > plenty of hardware that won't guarantee anything larger than 4k, and
> > plenty of NVMe hardware without has some other small limit like 32k
> > because it doesn't support multiple atomicy mode.
>
> So what would you propose as the next step? Would it to be first achieve
> atomic write support for XFS with HW support + CoW to ensure contiguous
> extents (and without XFS forcealign)?
>
> >
> > > And for no HW support, if we don't follow the O_ATOMIC model of committing
> > > nothing until a SYNC is issued, would we allocate, write, and later free a
> > > new extent for each write, right?
> >
> > Yes. Then again if you do data journalling you do that anyway, and as
> > one little project I'm doing right now shows that data journling is
> > often the fastest thing we can do for very small writes.
>
> Ignoring FSes, then how is this supposed to work for block devices? We just
> always need HW support, right?
Looks the HW support could be minimized, just like what Google and Amazon did,
16KB physical block size with proper queue limit setting.
Now seems it is easy to make such device with ublk-loop by:
- use one backing disk with 16KB/32KB/.. physical block size
- expose proper physical bs & chunk_sectors & max sectors queue limit
Then any 16KB aligned direct WRITE with N*16KB length(N in [1, 8] with 256
chunk_sectors) can be atomic-write.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: John Garry @ 2023-12-05 11:09 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Christoph Hellwig, axboe, kbusch, sagi, jejb, martin.petersen,
djwong, viro, brauner, chandan.babu, dchinner, linux-block,
linux-kernel, linux-nvme, linux-xfs, linux-fsdevel, jbongio,
linux-api
In-Reply-To: <20231205045539.GH509422@mit.edu>
On 05/12/2023 04:55, Theodore Ts'o wrote:
>> AFAICS, this is without any kernel changes, so no guarantee of unwanted
>> splitting or merging of bios.
> Well, more than one company has audited the kernel paths, and it turns
> out that for selected Kernel versions, after doing desk-check
> verification of the relevant kernel baths, as well as experimental
> verification via testing to try to find torn writes in the kernel, we
> can make it safe for specific kernel versions which might be used in
> hosted MySQL instances where we control the kernel, the mysql server,
> and the emulated block device (and we know the database is doing
> Direct I/O writes --- this won't work for PostgreSQL). I gave a talk
> about this at Google I/O Next '18, five years ago[1].
>
> [1]https://urldefense.com/v3/__https://www.youtube.com/watch?v=gIeuiGg-_iw__;!!ACWV5N9M2RV99hQ!I4iRp4xUyzAT0UwuEcnUBBCPKLXFKfk5FNmysFbKcQYfl0marAll5xEEVyB5mMFDqeckCWLmjU1aCR2Z$
>
> Given the performance gains (see the talk (see the comparison of the
> at time 19:31 and at 29:57) --- it's quite compelling.
>
> Of course, I wouldn't recommend this approach for a naive sysadmin,
> since most database adminsitrators won't know how to audit kernel code
> (see the discussion at time 35:10 of the video), and reverify the
> entire software stack before every kernel upgrade.
Sure
> The challenge is
> how to do this safely.
Right, and that is why I would be concerned about advertising torn-write
protection support, but someone has not gone through the effort of
auditing and verification phase to ensure that this does not happen in
their software stack ever.
>
> The fact remains that both Amazon's EBS and Google's Persistent Disk
> products are implemented in such a way that writes will not be torn
> below the virtual machine, and the guarantees are in fact quite a bit
> stronger than what we will probably end up advertising via NVMe and/or
> SCSI. It wouldn't surprise me if this is the case (or could be made
> to be the case) For Oracle Cloud as well.
>
> The question is how to make this guarantee so that the kernel knows
> when various cloud-provided block devicse do provide these greater
> guarantees, and then how to make it be an architected feature, as
> opposed to a happy implementation detail that has to be verified at
> every kernel upgrade.
The kernel can only judge atomic write support from what the HW product
data tells us, so cloud-provided block devices need to provide that
information as best possible if emulating the some storage technology.
Thanks,
John
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: John Garry @ 2023-12-05 10:49 UTC (permalink / raw)
To: Ming Lei
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <ZW6A0R04Gk/04EHj@fedora>
On 05/12/2023 01:45, Ming Lei wrote:
>> Right, we do expect userspace to use a fixed block size, but we give scope
>> in the API to use variable size.
> Maybe it is enough to just take atomic_write_unit_min_bytes
> only, and allow length to be N * atomic_write_unit_min_bytes.
>
> But it may violate atomic write boundary?
About atomic boundary, we just don't allow a merge which will result in
a write which will straddle a boundary as there are no guarantees of
atomicity then.
Having said this, atomic write boundary is just relevant to NVMe, so if
we don't have merges there, then we could just omit this code.
>
>>> The hardware should recognize unit size by start LBA, and check if length is
>>> valid, so probably the interface might be relaxed to:
>>>
>>> 1) start lba is unit aligned, and this unit is in the supported unit
>>> range(power_2 in [unit_min, unit_max])
>>>
>>> 2) length needs to be:
>>>
>>> - N * this_unit_size
>>> - <= atomic_write_max_bytes
>> Please note that we also need to consider:
>> - any atomic write boundary (from NVMe)
> Can you provide actual NVMe boundary value?
>
> Firstly natural aligned write won't cross boundary, so boundary should
> be >= write_unit_max,
Correct
> see blow code from patch 10/21:
>
> +static bool bio_straddles_atomic_write_boundary(loff_t bi_sector,
> + unsigned int bi_size,
> + unsigned int boundary)
> +{
> + loff_t start = bi_sector << SECTOR_SHIFT;
> + loff_t end = start + bi_size;
> + loff_t start_mod = start % boundary;
> + loff_t end_mod = end % boundary;
> +
> + if (end - start > boundary)
> + return true;
> + if ((start_mod > end_mod) && (start_mod && end_mod))
> + return true;
> +
> + return false;
> +}
> +
>
> Then if the WRITE size is <= boundary, the above function should return
> false, right?
Actually if WRITE > boundary then we must be crossing a boundary and
should return true, which is what the first condition checks.
However 2x naturally-aligned atomic writes could be less than
atomic_write_max_bytes but still straddle if merged.
> Looks like it is power_of(2) & aligned atomic_write_max_bytes?
>
>> - virt boundary (from NVMe)
> virt boundary is applied on bv_offset and bv_len, and NVMe's virt
> bounary is (4k - 1), it shouldn't be one issue in reality.
On a related topic, as I understand, for NVMe we just split bios
according to virt boundary for PRP, but we won't always use PRP. So is
there value in not splitting bio according to PRP if SGL will actually
be used?
>
>> And, as I mentioned elsewhere, I am still not 100% comfortable that we don't
>> pay attention to regular max_sectors_kb...
> max_sectors_kb should be bigger than atomic_write_max_bytes actually,
> then what is your concern?
My concern is that we don't enforce that, so we may issue atomic writes
which exceed max_sectors_kb.
If we did enforce it, then atomic_write_unit_min_bytes,
atomic_write_unit_max_bytes, and atomic_write_max_bytes all need to be
limited according to max_sectors_kb.
Thanks,
John
^ permalink raw reply
* Re: [PATCH] ELF: supply userspace with available page shifts (AT_PAGE_SHIFT_LIST)
From: Florian Weimer @ 2023-12-05 9:51 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: akpm, linux-kernel, linux-arch, linux-api, x86
In-Reply-To: <6b399b86-a478-48b0-92a1-25240a8ede54@p183>
* Alexey Dobriyan:
> +/*
> + * Page sizes available for mmap(2) encoded as 1 page shift per byte in
> + * increasing order.
> + *
> + * Thus 32-bit systems get 4 shifts, 64-bit systems get 8 shifts tops.
Couldn't you use the bits in a long instead, to indicate which shifts
are present? That's always going to be enough.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Ravi Jonnalagadda @ 2023-12-05 9:19 UTC (permalink / raw)
To: mhocko
Cc: Jonathan.Cameron, Ravis.OpenSrc, aneesh.kumar, dan.j.williams,
emirakhur, fvdl, gregory.price, hannes, haowang3, hasanalmaruf,
hezhongkun.hzk, john, linux-api, linux-cxl, linux-kernel,
linux-mm, sthanneeru.opensrc, tj, vtavarespetr, ying.huang
In-Reply-To: <ZW7km-SED5oIGGnZ@tiehlicka>
>On Tue 05-12-23 14:12:17, Srinivasulu Thanneeru wrote:
>>
>>
>> On 12/5/2023 2:05 PM, Michal Hocko wrote:
>> > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>> >
>> >
>> > On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
>> > >
>> > >
>> > > On 12/4/2023 9:13 PM, Michal Hocko wrote:
>> > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>> > > >
>> > > >
>> > > > On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>> > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>> > > > >
>> > > > > The memory tiers feature allows nodes with similar memory types
>> > > > > or performance characteristics to be grouped together in a
>> > > > > memory tier. However, there is currently no provision for
>> > > > > moving a node from one tier to another on demand.
>> > > >
>> > > > Could you expand on why this is really needed/necessary? What is the
>> > > > actual usecase?
>> > >
>> > > Hi Michal Hock,
>> > >
>> > > Following two use-cases we have observed.
>> > > 1. It is not accurate to group similar memory types in the same tier,
>> > > because even similar memory types may have different speed grades.
>> >
>> > Presumably they are grouped based on a HW configuration. Does that mean
>> > that the configuration is wrong? Are you trying to workaround that by
>> > this interface?
>> >
>> > > 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
>> > > we need a way to move the CXL nodes to the correct tier from the user space.
>> >
>> > Again, could you expand a bit more and explain why this cannot be
>> > configured automatically?
>>
>> Yes, in both cases above, if hardware not automatically populated properly,
>> in that case this interface would help to correct it from user space.
>>
>> We had observed case-2 in our setups.
>
>How hard it is to address this at the HW level?
>
>Btw. this is really important piece of context that should be part of
>the changelog. Quite honestly introducing user interfaces solely to
>workaround HW issues seems a rather weak justification. Are there any
>usecases you can think of where this would be useful?
>
>--
>Michal Hocko
>SUSE Labs
Hello Michal Hocko,
It will be useful if we want interleave weights to be applied on memory tiers
instead of nodes.
Also, for near memory processing use cases where some accelerator would like
to have hot pages migrated to a different node with HBM, pmem or CXL instead of
CPU attached memory for performing it's operations quicker.
There was a prior discussion on this functionality in a previous thread, where
Huang Ying thought this might be a useful feature to overcome limitations of
systems where nodes with different bandwidth characteristics are grouped in
a single tier.
https://lore.kernel.org/lkml/87a5rw1wu8.fsf@yhuang6-desk2.ccr.corp.intel.com/
--
Best Regards,
Ravi Jonnalagadda
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Huang, Ying @ 2023-12-05 9:12 UTC (permalink / raw)
To: Srinivasulu Thanneeru
Cc: Michal Hocko, aneesh.kumar, linux-cxl, linux-mm, dan.j.williams,
hannes, hasanalmaruf, haowang3, gregory.price, tj, hezhongkun.hzk,
fvdl, john, emirakhur, vtavarespetr, Ravis.OpenSrc,
Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <1db561a9-6984-418d-9305-a2a5ece93696@micron.com>
Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> writes:
> On 12/4/2023 9:13 PM, Michal Hocko wrote:
>> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments
>> unless you recognize the sender and were expecting this message.
>> On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>>
>>> The memory tiers feature allows nodes with similar memory types
>>> or performance characteristics to be grouped together in a
>>> memory tier. However, there is currently no provision for
>>> moving a node from one tier to another on demand.
>> Could you expand on why this is really needed/necessary? What is the
>> actual usecase?
>
> Hi Michal Hock,
>
> Following two use-cases we have observed.
> 1. It is not accurate to group similar memory types in the same tier,
> because even similar memory types may have different speed grades.
>
> 2. Some systems boots up with CXL devices and DRAM on the same
> memory-tier, we need a way to move the CXL nodes to the correct tier
> from the user space.
The case 2 reminds me a RFC before as follows,
https://lore.kernel.org/linux-mm/20221027065925.476955-1-ying.huang@intel.com/
The basic idea behind is that do we really need to put NUMA nodes with
different performance metrics in one memory tier? Are there use cases?
Will we have a system with so many different types of memory?
As in your case, you don't want to put DRAM and CXL memory in one memory
tier. Do you think we will need to put two types of memory in one
memory tier?
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Michal Hocko @ 2023-12-05 9:09 UTC (permalink / raw)
To: Srinivasulu Thanneeru
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <8eae7d95-fc98-4ac7-8f83-d0caee00bc87@micron.com>
On Tue 05-12-23 14:32:20, Srinivasulu Thanneeru wrote:
>
>
> On 12/5/2023 2:21 PM, Michal Hocko wrote:
> > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> >
> >
> > On Tue 05-12-23 14:12:17, Srinivasulu Thanneeru wrote:
> > >
> > >
> > > On 12/5/2023 2:05 PM, Michal Hocko wrote:
> > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> > > >
> > > >
> > > > On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
> > > > >
> > > > >
> > > > > On 12/4/2023 9:13 PM, Michal Hocko wrote:
> > > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> > > > > >
> > > > > >
> > > > > > On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
> > > > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> > > > > > >
> > > > > > > The memory tiers feature allows nodes with similar memory types
> > > > > > > or performance characteristics to be grouped together in a
> > > > > > > memory tier. However, there is currently no provision for
> > > > > > > moving a node from one tier to another on demand.
> > > > > >
> > > > > > Could you expand on why this is really needed/necessary? What is the
> > > > > > actual usecase?
> > > > >
> > > > > Hi Michal Hock,
> > > > >
> > > > > Following two use-cases we have observed.
> > > > > 1. It is not accurate to group similar memory types in the same tier,
> > > > > because even similar memory types may have different speed grades.
> > > >
> > > > Presumably they are grouped based on a HW configuration. Does that mean
> > > > that the configuration is wrong? Are you trying to workaround that by
> > > > this interface?
> > > >
> > > > > 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
> > > > > we need a way to move the CXL nodes to the correct tier from the user space.
> > > >
> > > > Again, could you expand a bit more and explain why this cannot be
> > > > configured automatically?
> > >
> > > Yes, in both cases above, if hardware not automatically populated properly,
> > > in that case this interface would help to correct it from user space.
> > >
> > > We had observed case-2 in our setups.
> >
> > How hard it is to address this at the HW level?
> >
> > Btw. this is really important piece of context that should be part of
> > the changelog. Quite honestly introducing user interfaces solely to
> > workaround HW issues seems a rather weak justification. Are there any
> > usecases you can think of where this would be useful?
>
> I'm not sure how difficult to fix it in the hardware.
Please explore that. It is sad to see learn that CXL which is a really
new technology is already fighting with misconfigurations.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Srinivasulu Thanneeru @ 2023-12-05 9:02 UTC (permalink / raw)
To: Michal Hocko
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <ZW7km-SED5oIGGnZ@tiehlicka>
On 12/5/2023 2:21 PM, Michal Hocko wrote:
> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>
>
> On Tue 05-12-23 14:12:17, Srinivasulu Thanneeru wrote:
>>
>>
>> On 12/5/2023 2:05 PM, Michal Hocko wrote:
>>> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>>>
>>>
>>> On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
>>>>
>>>>
>>>> On 12/4/2023 9:13 PM, Michal Hocko wrote:
>>>>> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>>>>>
>>>>>
>>>>> On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>>>>>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>>>>>
>>>>>> The memory tiers feature allows nodes with similar memory types
>>>>>> or performance characteristics to be grouped together in a
>>>>>> memory tier. However, there is currently no provision for
>>>>>> moving a node from one tier to another on demand.
>>>>>
>>>>> Could you expand on why this is really needed/necessary? What is the
>>>>> actual usecase?
>>>>
>>>> Hi Michal Hock,
>>>>
>>>> Following two use-cases we have observed.
>>>> 1. It is not accurate to group similar memory types in the same tier,
>>>> because even similar memory types may have different speed grades.
>>>
>>> Presumably they are grouped based on a HW configuration. Does that mean
>>> that the configuration is wrong? Are you trying to workaround that by
>>> this interface?
>>>
>>>> 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
>>>> we need a way to move the CXL nodes to the correct tier from the user space.
>>>
>>> Again, could you expand a bit more and explain why this cannot be
>>> configured automatically?
>>
>> Yes, in both cases above, if hardware not automatically populated properly,
>> in that case this interface would help to correct it from user space.
>>
>> We had observed case-2 in our setups.
>
> How hard it is to address this at the HW level?
>
> Btw. this is really important piece of context that should be part of
> the changelog. Quite honestly introducing user interfaces solely to
> workaround HW issues seems a rather weak justification. Are there any
> usecases you can think of where this would be useful?
I'm not sure how difficult to fix it in the hardware.
Sure, i will capture the use-cases in the change log, will be sending V2
by changing interface from adistance_offset to memtier_overwrite to
avoid complicated math for finding offset at user-level.
Thank you Michal Hocko for the feedback.
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Huang, Ying @ 2023-12-05 8:51 UTC (permalink / raw)
To: Srinivasulu Thanneeru
Cc: Michal Hocko, aneesh.kumar, linux-cxl, linux-mm, dan.j.williams,
hannes, hasanalmaruf, haowang3, gregory.price, tj, hezhongkun.hzk,
fvdl, john, emirakhur, vtavarespetr, Ravis.OpenSrc,
Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <1db561a9-6984-418d-9305-a2a5ece93696@micron.com>
Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> writes:
> On 12/4/2023 9:13 PM, Michal Hocko wrote:
>> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments
>> unless you recognize the sender and were expecting this message.
>> On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>>
>>> The memory tiers feature allows nodes with similar memory types
>>> or performance characteristics to be grouped together in a
>>> memory tier. However, there is currently no provision for
>>> moving a node from one tier to another on demand.
>> Could you expand on why this is really needed/necessary? What is the
>> actual usecase?
>
> Hi Michal Hock,
>
> Following two use-cases we have observed.
> 1. It is not accurate to group similar memory types in the same tier,
> because even similar memory types may have different speed grades.
>
> 2. Some systems boots up with CXL devices and DRAM on the same
> memory-tier, we need a way to move the CXL nodes to the correct tier
> from the user space.
I guess that you need to move all NUMA nodes with same performance
metrics together? If so, That is why we previously proposed to place
the knob in "memory_type".
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Michal Hocko @ 2023-12-05 8:51 UTC (permalink / raw)
To: Srinivasulu Thanneeru
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <2552828e-6865-4fa8-a9c4-8ed76dd85257@micron.com>
On Tue 05-12-23 14:12:17, Srinivasulu Thanneeru wrote:
>
>
> On 12/5/2023 2:05 PM, Michal Hocko wrote:
> > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> >
> >
> > On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
> > >
> > >
> > > On 12/4/2023 9:13 PM, Michal Hocko wrote:
> > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> > > >
> > > >
> > > > On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
> > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> > > > >
> > > > > The memory tiers feature allows nodes with similar memory types
> > > > > or performance characteristics to be grouped together in a
> > > > > memory tier. However, there is currently no provision for
> > > > > moving a node from one tier to another on demand.
> > > >
> > > > Could you expand on why this is really needed/necessary? What is the
> > > > actual usecase?
> > >
> > > Hi Michal Hock,
> > >
> > > Following two use-cases we have observed.
> > > 1. It is not accurate to group similar memory types in the same tier,
> > > because even similar memory types may have different speed grades.
> >
> > Presumably they are grouped based on a HW configuration. Does that mean
> > that the configuration is wrong? Are you trying to workaround that by
> > this interface?
> >
> > > 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
> > > we need a way to move the CXL nodes to the correct tier from the user space.
> >
> > Again, could you expand a bit more and explain why this cannot be
> > configured automatically?
>
> Yes, in both cases above, if hardware not automatically populated properly,
> in that case this interface would help to correct it from user space.
>
> We had observed case-2 in our setups.
How hard it is to address this at the HW level?
Btw. this is really important piece of context that should be part of
the changelog. Quite honestly introducing user interfaces solely to
workaround HW issues seems a rather weak justification. Are there any
usecases you can think of where this would be useful?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Srinivasulu Thanneeru @ 2023-12-05 8:42 UTC (permalink / raw)
To: Michal Hocko
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <ZW7g4ExYF79gMEBU@tiehlicka>
On 12/5/2023 2:05 PM, Michal Hocko wrote:
> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>
>
> On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
>>
>>
>> On 12/4/2023 9:13 PM, Michal Hocko wrote:
>>> CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
>>>
>>>
>>> On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
>>>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>>>
>>>> The memory tiers feature allows nodes with similar memory types
>>>> or performance characteristics to be grouped together in a
>>>> memory tier. However, there is currently no provision for
>>>> moving a node from one tier to another on demand.
>>>
>>> Could you expand on why this is really needed/necessary? What is the
>>> actual usecase?
>>
>> Hi Michal Hock,
>>
>> Following two use-cases we have observed.
>> 1. It is not accurate to group similar memory types in the same tier,
>> because even similar memory types may have different speed grades.
>
> Presumably they are grouped based on a HW configuration. Does that mean
> that the configuration is wrong? Are you trying to workaround that by
> this interface?
>
>> 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
>> we need a way to move the CXL nodes to the correct tier from the user space.
>
> Again, could you expand a bit more and explain why this cannot be
> configured automatically?
Yes, in both cases above, if hardware not automatically populated
properly, in that case this interface would help to correct it from user
space.
We had observed case-2 in our setups.
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [EXT] Re: [RFC PATCH 0/2] Node migration between memory tiers
From: Michal Hocko @ 2023-12-05 8:35 UTC (permalink / raw)
To: Srinivasulu Thanneeru
Cc: aneesh.kumar, linux-cxl, linux-mm, dan.j.williams, hannes,
hasanalmaruf, haowang3, ying.huang, gregory.price, tj,
hezhongkun.hzk, fvdl, john, emirakhur, vtavarespetr,
Ravis.OpenSrc, Jonathan.Cameron, linux-kernel, linux-api
In-Reply-To: <1db561a9-6984-418d-9305-a2a5ece93696@micron.com>
On Tue 05-12-23 01:26:07, Srinivasulu Thanneeru wrote:
>
>
> On 12/4/2023 9:13 PM, Michal Hocko wrote:
> > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.
> >
> >
> > On Fri 01-12-23 03:34:20, sthanneeru.opensrc@micron.com wrote:
> > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> > >
> > > The memory tiers feature allows nodes with similar memory types
> > > or performance characteristics to be grouped together in a
> > > memory tier. However, there is currently no provision for
> > > moving a node from one tier to another on demand.
> >
> > Could you expand on why this is really needed/necessary? What is the
> > actual usecase?
>
> Hi Michal Hock,
>
> Following two use-cases we have observed.
> 1. It is not accurate to group similar memory types in the same tier,
> because even similar memory types may have different speed grades.
Presumably they are grouped based on a HW configuration. Does that mean
that the configuration is wrong? Are you trying to workaround that by
this interface?
> 2. Some systems boots up with CXL devices and DRAM on the same memory-tier,
> we need a way to move the CXL nodes to the correct tier from the user space.
Again, could you expand a bit more and explain why this cannot be
configured automatically?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 17/21] fs: xfs: iomap atomic write support
From: Theodore Ts'o @ 2023-12-05 4:55 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, axboe, kbusch, sagi, jejb, martin.petersen,
djwong, viro, brauner, chandan.babu, dchinner, linux-block,
linux-kernel, linux-nvme, linux-xfs, linux-fsdevel, jbongio,
linux-api
In-Reply-To: <a87d48a7-f2a8-40ae-8d9b-e4534ccc29b1@oracle.com>
On Mon, Dec 04, 2023 at 03:19:15PM +0000, John Garry wrote:
> >
> > What is the 'dubious amazon torn-write prevention'?
>
> https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/storage-twp.html
>
> AFAICS, this is without any kernel changes, so no guarantee of unwanted
> splitting or merging of bios.
Well, more than one company has audited the kernel paths, and it turns
out that for selected Kernel versions, after doing desk-check
verification of the relevant kernel baths, as well as experimental
verification via testing to try to find torn writes in the kernel, we
can make it safe for specific kernel versions which might be used in
hosted MySQL instances where we control the kernel, the mysql server,
and the emulated block device (and we know the database is doing
Direct I/O writes --- this won't work for PostgreSQL). I gave a talk
about this at Google I/O Next '18, five years ago[1].
[1] https://www.youtube.com/watch?v=gIeuiGg-_iw
Given the performance gains (see the talk (see the comparison of the
at time 19:31 and at 29:57) --- it's quite compelling.
Of course, I wouldn't recommend this approach for a naive sysadmin,
since most database adminsitrators won't know how to audit kernel code
(see the discussion at time 35:10 of the video), and reverify the
entire software stack before every kernel upgrade. The challenge is
how to do this safely.
The fact remains that both Amazon's EBS and Google's Persistent Disk
products are implemented in such a way that writes will not be torn
below the virtual machine, and the guarantees are in fact quite a bit
stronger than what we will probably end up advertising via NVMe and/or
SCSI. It wouldn't surprise me if this is the case (or could be made
to be the case) For Oracle Cloud as well.
The question is how to make this guarantee so that the kernel knows
when various cloud-provided block devicse do provide these greater
guarantees, and then how to make it be an architected feature, as
opposed to a happy implementation detail that has to be verified at
every kernel upgrade.
Cheers,
- Ted
^ permalink raw reply
* Re: [PATCH 10/21] block: Add fops atomic write support
From: Ming Lei @ 2023-12-05 1:45 UTC (permalink / raw)
To: John Garry
Cc: axboe, kbusch, hch, sagi, jejb, martin.petersen, djwong, viro,
brauner, chandan.babu, dchinner, linux-block, linux-kernel,
linux-nvme, linux-xfs, linux-fsdevel, tytso, jbongio, linux-api
In-Reply-To: <bd639010-2ad7-4379-ba0a-64b5f6ebec41@oracle.com>
On Mon, Dec 04, 2023 at 01:13:55PM +0000, John Garry wrote:
>
> > >
> > > I added this here (as opposed to the caller), as I was not really worried
> > > about speeding up the failure path. Are you saying to call even earlier in
> > > submission path?
> > atomic_write_unit_min is one hardware property, and it should be checked
> > in blk_queue_atomic_write_unit_min_sectors() from beginning, then you
> > can avoid this check every other where.
>
> ok, but we still need to ensure in the submission path that the block device
> actually supports atomic writes - this was the initial check.
Then you may add one helper bdev_support_atomic_write().
>
> >
> > > > > + if (pos % atomic_write_unit_min_bytes)
> > > > > + return false;
> > > > > + if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
> > > > > + return false;
> > > > > + if (!is_power_of_2(iov_iter_count(iter)))
> > > > > + return false;
> > > > > + if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
> > > > > + return false;
> > > > > + if (pos % iov_iter_count(iter))
> > > > > + return false;
> > > > I am a bit confused about relation between atomic_write_unit_max_bytes and
> > > > atomic_write_max_bytes.
> > > I think that naming could be improved. Or even just drop merging (and
> > > atomic_write_max_bytes concept) until we show it to improve performance.
> > >
> > > So generally atomic_write_unit_max_bytes will be same as
> > > atomic_write_max_bytes, however it could be different if:
> > > a. request queue nr hw segments or other request queue limits needs to
> > > restrict atomic_write_unit_max_bytes
> > > b. atomic_write_unit_max_bytes does not need to be a power-of-2 and
> > > atomic_write_max_bytes does. So essentially:
> > > atomic_write_unit_max_bytes = rounddown_pow_of_2(atomic_write_max_bytes)
> > >
> > plug merge often improves sequential IO perf, so if the hardware supports
> > this way, I think 'atomic_write_max_bytes' should be supported from the
> > beginning, such as:
> >
> > - user space submits sequential N * (4k, 8k, 16k, ...) atomic writes, all can
> > be merged to single IO request, which is issued to driver.
> >
> > Or
> >
> > - user space submits sequential 4k, 4k, 8k, 16K, 32k, 64k atomic writes, all can
> > be merged to single IO request, which is issued to driver.
>
> Right, we do expect userspace to use a fixed block size, but we give scope
> in the API to use variable size.
Maybe it is enough to just take atomic_write_unit_min_bytes
only, and allow length to be N * atomic_write_unit_min_bytes.
But it may violate atomic write boundary?
>
> >
> > The hardware should recognize unit size by start LBA, and check if length is
> > valid, so probably the interface might be relaxed to:
> >
> > 1) start lba is unit aligned, and this unit is in the supported unit
> > range(power_2 in [unit_min, unit_max])
> >
> > 2) length needs to be:
> >
> > - N * this_unit_size
> > - <= atomic_write_max_bytes
>
> Please note that we also need to consider:
> - any atomic write boundary (from NVMe)
Can you provide actual NVMe boundary value?
Firstly natural aligned write won't cross boundary, so boundary should
be >= write_unit_max, see blow code from patch 10/21:
+static bool bio_straddles_atomic_write_boundary(loff_t bi_sector,
+ unsigned int bi_size,
+ unsigned int boundary)
+{
+ loff_t start = bi_sector << SECTOR_SHIFT;
+ loff_t end = start + bi_size;
+ loff_t start_mod = start % boundary;
+ loff_t end_mod = end % boundary;
+
+ if (end - start > boundary)
+ return true;
+ if ((start_mod > end_mod) && (start_mod && end_mod))
+ return true;
+
+ return false;
+}
+
Then if the WRITE size is <= boundary, the above function should return
false, right? Looks like it is power_of(2) & aligned atomic_write_max_bytes?
> - virt boundary (from NVMe)
virt boundary is applied on bv_offset and bv_len, and NVMe's virt
bounary is (4k - 1), it shouldn't be one issue in reality.
>
> And, as I mentioned elsewhere, I am still not 100% comfortable that we don't
> pay attention to regular max_sectors_kb...
max_sectors_kb should be bigger than atomic_write_max_bytes actually,
then what is your concern?
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()
From: Edgecombe, Rick P @ 2023-12-05 0:26 UTC (permalink / raw)
To: dietmar.eggemann@arm.com, broonie@kernel.org,
Szabolcs.Nagy@arm.com, brauner@kernel.org,
dave.hansen@linux.intel.com, debug@rivosinc.com, mgorman@suse.de,
vincent.guittot@linaro.org, fweimer@redhat.com, mingo@redhat.com,
rostedt@goodmis.org, hjl.tools@gmail.com, tglx@linutronix.de,
vschneid@redhat.com, shuah@kernel.org, bristot@redhat.com,
hpa@zytor.com, peterz@infradead.org, bp@alien8.de,
bsegall@google.com, x86@kernel.org, juri.lelli@redhat.com
Cc: keescook@chromium.org, jannh@google.com,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
catalin.marinas@arm.com, linux-api@vger.kernel.org,
will@kernel.org
In-Reply-To: <20231128-clone3-shadow-stack-v4-2-8b28ffe4f676@kernel.org>
On Tue, 2023-11-28 at 18:22 +0000, Mark Brown wrote:
> -unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
> unsigned long clone_flags,
> - unsigned long stack_size)
> +unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
> + const struct kernel_clone_args
> *args)
> {
> struct thread_shstk *shstk = &tsk->thread.shstk;
> + unsigned long clone_flags = args->flags;
> unsigned long addr, size;
>
> /*
> * If shadow stack is not enabled on the new thread, skip any
> - * switch to a new shadow stack.
> + * implicit switch to a new shadow stack and reject attempts
> to
> + * explciitly specify one.
> */
> - if (!features_enabled(ARCH_SHSTK_SHSTK))
> - return 0;
> + if (!features_enabled(ARCH_SHSTK_SHSTK)) {
> + if (args->shadow_stack_size)
> + return (unsigned long)ERR_PTR(-EINVAL);
>
> - /*
> - * For CLONE_VFORK the child will share the parents shadow
> stack.
> - * Make sure to clear the internal tracking of the thread
> shadow
> - * stack so the freeing logic run for child knows to leave it
> alone.
> - */
> - if (clone_flags & CLONE_VFORK) {
> - shstk->base = 0;
> - shstk->size = 0;
> return 0;
> }
>
> /*
> - * For !CLONE_VM the child will use a copy of the parents
> shadow
> - * stack.
> + * If the user specified a shadow stack then do some basic
> + * validation and use it, otherwise fall back to a default
> + * shadow stack size if the clone_flags don't indicate an
> + * allocation is unneeded.
> */
> - if (!(clone_flags & CLONE_VM))
> - return 0;
> + if (args->shadow_stack_size) {
> + size = args->shadow_stack_size;
> + } else {
> + /*
> + * For CLONE_VFORK the child will share the parents
> + * shadow stack. Make sure to clear the internal
> + * tracking of the thread shadow stack so the freeing
> + * logic run for child knows to leave it alone.
> + */
> + if (clone_flags & CLONE_VFORK) {
> + shstk->base = 0;
> + shstk->size = 0;
> + return 0;
> + }
> +
> + /*
> + * For !CLONE_VM the child will use a copy of the
> + * parents shadow stack.
> + */
> + if (!(clone_flags & CLONE_VM))
> + return 0;
> +
> + size = args->stack_size;
> +
> + }
>
> - size = adjust_shstk_size(stack_size);
> + size = adjust_shstk_size(size);
> addr = alloc_shstk(0, size, 0, false);
Hmm. I didn't test this, but in the copy_process(), copy_mm() happens
before this point. So the shadow stack would get mapped in current's MM
(i.e. the parent). So in the !CLONE_VM case with shadow_stack_size!=0
the SSP in the child will be updated to an area that is not mapped in
the child. I think we need to pass tsk->mm into alloc_shstk(). But such
an exotic clone usage does give me pause, regarding whether all of this
is premature.
Otherwise it looked ok from the x86/shstk perspective.
> if (IS_ERR_VALUE(addr))
> return addr;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox