* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Matthew Wilcox @ 2024-03-19 18:38 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnaMa6x/O68ENsP@memverge.com>
On Tue, Mar 19, 2024 at 02:32:17PM -0400, Gregory Price wrote:
> On Tue, Mar 19, 2024 at 06:20:33PM +0000, Matthew Wilcox wrote:
> > On Tue, Mar 19, 2024 at 02:14:33PM -0400, Gregory Price wrote:
> > > On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> > > > On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > > > > Implement simple ktest that looks up the physical address via
> > > > > /proc/self/pagemap and migrates the page based on that information.
> > > >
> > > > What? LOL. No.
> > > >
> > >
> > > Certainly the test is stupid and requires admin, but I could not
> > > come up an easier test to demonstrate the concept - and the docs
> > > say to include a test with all syscall proposals.
> > >
> > > Am I missing something else important?
> > > (stupid question: of course I am, but alas I must ask it)
> >
> > It's not that the test is stupid. It's the concept that's stupid.
>
> Ok i'll bite.
>
> The 2 major ways page-hotness is detected right now is page-faults
> (induced or otherwise) and things like IBS/PEBS.
>
> page-faults cause overhead, and IBS/PEBS actually miss upwards of ~66%
> of all traffic (if you want the details i can dig up the presentation,
> but TL;DR: prefetcher traffic is missed entirely).
>
> so OCP folks have been proposing hotness-tracking offloaded to the
> memory devices themselves:
>
> https://www.opencompute.org/documents/ocp-cms-hotness-tracking-requirements-white-paper-pdf-1
>
> (it's come along further than this white paper, but i need to dig up
> the new information).
>
> These devices are incapable of providing virtual addressing information,
> and doing reverse lookups of addresses is inordinately expensive from
> user space. This leaves: Do it all in a kernel task, or give user space
> an an interface to operate on data provided by the device.
>
> The syscall design is mostly being posted right now to collaborate via
> public channels, but if the idea is so fundamentally offensive then i'll
> drop it and relay the opinion accordingly.
The syscall design is wrong. Exposing physical addresses to userspace
is never the right answer. Think rowhammer.
I'm vehemently opposed to all of the bullshit around CXL. However, if you
are going to propose something, it should be based around an abstraction.
Say "We have 8 pools of memory. This VMA is backed by memory from pools
3 & 6. The relative hotness of the 8 pools are <vector>. The quantities
of memory in the 8 ppols are <vector>". And then you can say "migrate
this range of memory to pool 2".
That's just an initial response to the idea. I refuse to invest a
serious amount of time in a dead-end idea like CXL memory pooling.
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Gregory Price @ 2024-03-19 18:32 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnXcbd3h3Rj4IIS@casper.infradead.org>
On Tue, Mar 19, 2024 at 06:20:33PM +0000, Matthew Wilcox wrote:
> On Tue, Mar 19, 2024 at 02:14:33PM -0400, Gregory Price wrote:
> > On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> > > On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > > > Implement simple ktest that looks up the physical address via
> > > > /proc/self/pagemap and migrates the page based on that information.
> > >
> > > What? LOL. No.
> > >
> >
> > Certainly the test is stupid and requires admin, but I could not
> > come up an easier test to demonstrate the concept - and the docs
> > say to include a test with all syscall proposals.
> >
> > Am I missing something else important?
> > (stupid question: of course I am, but alas I must ask it)
>
> It's not that the test is stupid. It's the concept that's stupid.
Ok i'll bite.
The 2 major ways page-hotness is detected right now is page-faults
(induced or otherwise) and things like IBS/PEBS.
page-faults cause overhead, and IBS/PEBS actually miss upwards of ~66%
of all traffic (if you want the details i can dig up the presentation,
but TL;DR: prefetcher traffic is missed entirely).
so OCP folks have been proposing hotness-tracking offloaded to the
memory devices themselves:
https://www.opencompute.org/documents/ocp-cms-hotness-tracking-requirements-white-paper-pdf-1
(it's come along further than this white paper, but i need to dig up
the new information).
These devices are incapable of providing virtual addressing information,
and doing reverse lookups of addresses is inordinately expensive from
user space. This leaves: Do it all in a kernel task, or give user space
an an interface to operate on data provided by the device.
The syscall design is mostly being posted right now to collaborate via
public channels, but if the idea is so fundamentally offensive then i'll
drop it and relay the opinion accordingly.
~Gregory
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Matthew Wilcox @ 2024-03-19 18:20 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnWCRwcZJ4KBmSH@memverge.com>
On Tue, Mar 19, 2024 at 02:14:33PM -0400, Gregory Price wrote:
> On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> > On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > > Implement simple ktest that looks up the physical address via
> > > /proc/self/pagemap and migrates the page based on that information.
> >
> > What? LOL. No.
> >
>
> Certainly the test is stupid and requires admin, but I could not
> come up an easier test to demonstrate the concept - and the docs
> say to include a test with all syscall proposals.
>
> Am I missing something else important?
> (stupid question: of course I am, but alas I must ask it)
It's not that the test is stupid. It's the concept that's stupid.
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktesty
From: Gregory Price @ 2024-03-19 18:18 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnWYcFLLVyP2Ng8@memverge.com>
On Tue, Mar 19, 2024 at 02:16:01PM -0400, Gregory Price wrote:
> On Tue, Mar 19, 2024 at 06:08:39PM +0000, Matthew Wilcox wrote:
> > On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> > > On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > > > Implement simple ktest that looks up the physical address via
> > > > /proc/self/pagemap and migrates the page based on that information.
> > >
> > > What? LOL. No.
> >
> > Also, how is this v3 and the first one to land on linux-mm?
> >
> > https://lore.kernel.org/linux-mm/?q=move_phys_pages
> >
> > Also, where is the syscall itself? The only thing here is the ktest.
> >
>
> OH, I see the confusion now.
>
> There were two other versions, and I have experienced this delivery
> failure before, i'm not sure why the other commits have not been delivered.
>
> Let me look into this.
>
> ~Gregory
>
Full set of patches:
https://lore.kernel.org/all/20240319172609.332900-1-gregory.price@memverge.com/
I've experienced silent linux-mm delivery failures like this before, I
still do not understand this issue.
v1:
https://lore.kernel.org/all/20230907075453.350554-1-gregory.price@memverge.com/
v2:
https://lore.kernel.org/all/20230919230909.530174-1-gregory.price@memverge.com/
~Gregory
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktesty
From: Gregory Price @ 2024-03-19 18:16 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnUp7OAr6PFGAq_@casper.infradead.org>
On Tue, Mar 19, 2024 at 06:08:39PM +0000, Matthew Wilcox wrote:
> On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> > On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > > Implement simple ktest that looks up the physical address via
> > > /proc/self/pagemap and migrates the page based on that information.
> >
> > What? LOL. No.
>
> Also, how is this v3 and the first one to land on linux-mm?
>
> https://lore.kernel.org/linux-mm/?q=move_phys_pages
>
> Also, where is the syscall itself? The only thing here is the ktest.
>
OH, I see the confusion now.
There were two other versions, and I have experienced this delivery
failure before, i'm not sure why the other commits have not been delivered.
Let me look into this.
~Gregory
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Gregory Price @ 2024-03-19 18:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Gregory Price, linux-mm, linux-api, linux-arch, linux-kselftest,
linux-kernel, ying.huang, dan.j.williams, honggyu.kim, corbet,
arnd, luto, akpm, shuah
In-Reply-To: <ZfnQ7n_7cZvk9BkC@casper.infradead.org>
On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > Implement simple ktest that looks up the physical address via
> > /proc/self/pagemap and migrates the page based on that information.
>
> What? LOL. No.
>
Certainly the test is stupid and requires admin, but I could not
come up an easier test to demonstrate the concept - and the docs
say to include a test with all syscall proposals.
Am I missing something else important?
(stupid question: of course I am, but alas I must ask it)
~Gregory
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Matthew Wilcox @ 2024-03-19 18:08 UTC (permalink / raw)
To: Gregory Price
Cc: linux-mm, linux-api, linux-arch, linux-kselftest, linux-kernel,
ying.huang, dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm,
shuah, Gregory Price
In-Reply-To: <ZfnQ7n_7cZvk9BkC@casper.infradead.org>
On Tue, Mar 19, 2024 at 05:52:46PM +0000, Matthew Wilcox wrote:
> On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> > Implement simple ktest that looks up the physical address via
> > /proc/self/pagemap and migrates the page based on that information.
>
> What? LOL. No.
Also, how is this v3 and the first one to land on linux-mm?
https://lore.kernel.org/linux-mm/?q=move_phys_pages
Also, where is the syscall itself? The only thing here is the ktest.
^ permalink raw reply
* Re: [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Matthew Wilcox @ 2024-03-19 17:52 UTC (permalink / raw)
To: Gregory Price
Cc: linux-mm, linux-api, linux-arch, linux-kselftest, linux-kernel,
ying.huang, dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm,
shuah, Gregory Price
In-Reply-To: <20240319172609.332900-4-gregory.price@memverge.com>
On Tue, Mar 19, 2024 at 01:26:09PM -0400, Gregory Price wrote:
> Implement simple ktest that looks up the physical address via
> /proc/self/pagemap and migrates the page based on that information.
What? LOL. No.
^ permalink raw reply
* [RFC v3 3/3] ktest: sys_move_phys_pages ktest
From: Gregory Price @ 2024-03-19 17:26 UTC (permalink / raw)
To: linux-mm
Cc: linux-api, linux-arch, linux-kselftest, linux-kernel, ying.huang,
dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm, shuah,
Gregory Price
In-Reply-To: <20240319172609.332900-1-gregory.price@memverge.com>
Implement simple ktest that looks up the physical address via
/proc/self/pagemap and migrates the page based on that information.
Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
tools/testing/selftests/mm/migration.c | 99 ++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
index 6908569ef406..c005c98dbdc1 100644
--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c
@@ -5,6 +5,8 @@
*/
#include "../kselftest_harness.h"
+#include <stdint.h>
+#include <stdio.h>
#include <strings.h>
#include <pthread.h>
#include <numa.h>
@@ -14,11 +16,17 @@
#include <sys/types.h>
#include <signal.h>
#include <time.h>
+#include <unistd.h>
#define TWOMEG (2<<20)
#define RUNTIME (20)
+#define GET_BIT(X, Y) ((X & ((uint64_t)1<<Y)) >> Y)
+#define GET_PFN(X) (X & 0x7FFFFFFFFFFFFFull)
#define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
+#define PAGEMAP_ENTRY 8
+const int __endian_bit = 1;
+#define is_bigendian() ((*(char *)&__endian_bit) == 0)
FIXTURE(migration)
{
@@ -94,6 +102,45 @@ int migrate(uint64_t *ptr, int n1, int n2)
return 0;
}
+int migrate_phys(uint64_t paddr, int n1, int n2)
+{
+ int ret, tmp;
+ int status = 0;
+ struct timespec ts1, ts2;
+
+ if (clock_gettime(CLOCK_MONOTONIC, &ts1))
+ return -1;
+
+ while (1) {
+ if (clock_gettime(CLOCK_MONOTONIC, &ts2))
+ return -1;
+
+ if (ts2.tv_sec - ts1.tv_sec >= RUNTIME)
+ return 0;
+
+ /*
+ * FIXME: move_phys_pages was syscall 462 during RFC.
+ * Update this when an official syscall number is adopted
+ * and the libnuma interface is implemented.
+ */
+ ret = syscall(462, 1, (void **) &paddr, &n2, &status,
+ MPOL_MF_MOVE_ALL);
+ if (ret) {
+ if (ret > 0)
+ printf("Didn't migrate %d pages\n", ret);
+ else
+ perror("Couldn't migrate pages");
+ return -2;
+ }
+
+ tmp = n2;
+ n2 = n1;
+ n1 = tmp;
+ }
+
+ return 0;
+}
+
void *access_mem(void *ptr)
{
volatile uint64_t y = 0;
@@ -199,4 +246,56 @@ TEST_F_TIMEOUT(migration, private_anon_thp, 2*RUNTIME)
ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
}
+/*
+ * Same as the basic migration, but test move_phys_pages.
+ */
+TEST_F_TIMEOUT(migration, phys_addr, 2*RUNTIME)
+{
+ uint64_t *ptr;
+ uint64_t pagemap_val, paddr, file_offset;
+ unsigned char c_buf[PAGEMAP_ENTRY];
+ int i, c, status;
+ FILE *f;
+
+ if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
+ SKIP(return, "Not enough threads or NUMA nodes available");
+
+ ptr = mmap(NULL, TWOMEG, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ ASSERT_NE(ptr, MAP_FAILED);
+
+ memset(ptr, 0xde, TWOMEG);
+
+ /* PFN of ptr from /proc/self/pagemap */
+ f = fopen("/proc/self/pagemap", "rb");
+ file_offset = ((uint64_t)ptr) / getpagesize() * PAGEMAP_ENTRY;
+ status = fseek(f, file_offset, SEEK_SET);
+ ASSERT_EQ(status, 0);
+ for (i = 0; i < PAGEMAP_ENTRY; i++) {
+ c = getc(f);
+ ASSERT_NE(c, EOF);
+ /* handle endiand differences */
+ if (is_bigendian())
+ c_buf[i] = c;
+ else
+ c_buf[PAGEMAP_ENTRY - i - 1] = c;
+ }
+ fclose(f);
+
+ for (i = 0; i < PAGEMAP_ENTRY; i++)
+ pagemap_val = (pagemap_val << 8) + c_buf[i];
+
+ ASSERT_TRUE(GET_BIT(pagemap_val, 63));
+ /* This reports a pfn, we need to shift this by page size */
+ paddr = GET_PFN(pagemap_val) << __builtin_ctz(getpagesize());
+
+ for (i = 0; i < self->nthreads - 1; i++)
+ if (pthread_create(&self->threads[i], NULL, access_mem, ptr))
+ perror("Couldn't create thread");
+
+ ASSERT_EQ(migrate_phys(paddr, self->n1, self->n2), 0);
+ for (i = 0; i < self->nthreads - 1; i++)
+ ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+}
+
TEST_HARNESS_MAIN
--
2.39.1
^ permalink raw reply related
* [RFC v3 2/3] mm/migrate: Create move_phys_pages syscall
From: Gregory Price @ 2024-03-19 17:26 UTC (permalink / raw)
To: linux-mm
Cc: linux-api, linux-arch, linux-kselftest, linux-kernel, ying.huang,
dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm, shuah,
Gregory Price
In-Reply-To: <20240319172609.332900-1-gregory.price@memverge.com>
Similar to the move_pages system call, instead of taking a pid and
list of virtual addresses, this system call takes a list of physical
addresses.
Because there is no task to validate the memory policy against, each
page needs to be interrogated to determine whether the migration is
valid, and all tasks that map it need to be interrogated.
This is accomplished in via a rmap_walk on the folio containing
the page, and an interrogation of all tasks that map the page (by
way of each task's vma).
Each page must be interrogated individually, which should be
considered when using this to migrate shared regions.
The remaining logic is the same as the move_pages syscall. One
change to do_pages_move is made (to check whether an mm_struct is
passed) in order to re-use the existing migration code.
Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 8 +-
kernel/sys_ni.c | 1 +
mm/migrate.c | 206 +++++++++++++++++++++++-
tools/include/uapi/asm-generic/unistd.h | 8 +-
7 files changed, 222 insertions(+), 8 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 5f8591ce7f25..250c00281029 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -466,3 +466,4 @@
459 i386 lsm_get_self_attr sys_lsm_get_self_attr
460 i386 lsm_set_self_attr sys_lsm_set_self_attr
461 i386 lsm_list_modules sys_lsm_list_modules
+462 i386 move_phys_pages sys_move_phys_pages
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f4147f..a928df7c6f52 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -383,6 +383,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common move_phys_pages sys_move_phys_pages
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 77eb9b0e7685..575ba9d26e30 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -840,6 +840,11 @@ asmlinkage long sys_move_pages(pid_t pid, unsigned long nr_pages,
const int __user *nodes,
int __user *status,
int flags);
+asmlinkage long sys_move_phys_pages(unsigned long nr_pages,
+ const void __user * __user *pages,
+ const int __user *nodes,
+ int __user *status,
+ int flags);
asmlinkage long sys_rt_tgsigqueueinfo(pid_t tgid, pid_t pid, int sig,
siginfo_t __user *uinfo);
asmlinkage long sys_perf_event_open(
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 75f00965ab15..13bc8dd16d6b 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -842,8 +842,14 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+/* CONFIG_MMU only */
+#ifndef __ARCH_NOMMU
+#define __NR_move_phys_pages 462
+__SYSCALL(__NR_move_phys_pages, sys_move_phys_pages)
+#endif
+
#undef __NR_syscalls
-#define __NR_syscalls 462
+#define __NR_syscalls 463
/*
* 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index faad00cce269..254915fd1e2c 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -196,6 +196,7 @@ COND_SYSCALL(migrate_pages);
COND_SYSCALL(move_pages);
COND_SYSCALL(set_mempolicy_home_node);
COND_SYSCALL(cachestat);
+COND_SYSCALL(move_phys_pages);
COND_SYSCALL(perf_event_open);
COND_SYSCALL(accept4);
diff --git a/mm/migrate.c b/mm/migrate.c
index 27071a07ffbb..7213703441f8 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2182,9 +2182,119 @@ static int move_pages_and_store_status(int node,
return store_status(status, start, node, i - start);
}
+struct rmap_page_ctxt {
+ bool found;
+ bool migratable;
+ bool node_allowed;
+ int node;
+};
+
+/*
+ * Walks each vma mapping a given page and determines if those
+ * vma's are both migratable, and that the target node is within
+ * the allowed cpuset of the owning task.
+ */
+static bool phys_page_migratable(struct folio *folio,
+ struct vm_area_struct *vma,
+ unsigned long address,
+ void *arg)
+{
+ struct rmap_page_ctxt *ctxt = arg;
+#ifdef CONFIG_MEMCG
+ struct task_struct *owner = vma->vm_mm->owner;
+ nodemask_t task_nodes = cpuset_mems_allowed(owner);
+#else
+ nodemask_t task_nodes = node_possible_map;
+#endif
+
+ ctxt->found = true;
+ ctxt->migratable &= vma_migratable(vma);
+ ctxt->node_allowed &= node_isset(ctxt->node, task_nodes);
+
+ return ctxt->migratable && ctxt->node_allowed;
+}
+
+static struct folio *phys_migrate_get_folio(struct page *page)
+{
+ struct folio *folio;
+
+ folio = page_folio(page);
+ if (!folio_test_lru(folio) || !folio_try_get(folio))
+ return NULL;
+ if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) {
+ folio_put(folio);
+ folio = NULL;
+ }
+ return folio;
+}
+
+/*
+ * Validates the physical address is online and migratable. Walks the folio
+ * containing the page to validate the vma is migratable and the cpuset node
+ * restrictions. Then calls add_page_for_migration to isolate it from the
+ * LRU and place it into the given pagelist.
+ * Returns:
+ * errno - if the page is not online, migratable, or can't be isolated
+ * 0 - when it doesn't have to be migrated because it is already on the
+ * target node
+ * 1 - when it has been queued
+ */
+static int add_phys_page_for_migration(const void __user *p, int node,
+ struct list_head *pagelist,
+ bool migrate_all)
+{
+ unsigned long pfn;
+ struct page *page;
+ struct folio *folio;
+ int err;
+ struct rmap_page_ctxt rmctxt = {
+ .found = false,
+ .migratable = true,
+ .node_allowed = true,
+ .node = node
+ };
+ struct rmap_walk_control rwc = {
+ .rmap_one = phys_page_migratable,
+ .arg = &rmctxt
+ };
+
+ pfn = ((unsigned long)p) >> PAGE_SHIFT;
+ page = pfn_to_online_page(pfn);
+ if (!page || PageTail(page))
+ return -ENOENT;
+
+ folio = phys_migrate_get_folio(page);
+ if (!folio)
+ return -ENOENT;
+
+ rmap_walk(folio, &rwc);
+
+ if (!rmctxt.found)
+ err = -ENOENT;
+ else if (!rmctxt.migratable)
+ err = -EFAULT;
+ else if (!rmctxt.node_allowed)
+ err = -EACCES;
+ else
+ err = add_page_for_migration(page, folio, node, pagelist,
+ migrate_all);
+
+ folio_put(folio);
+
+ return err;
+}
+
/*
* Migrate an array of page address onto an array of nodes and fill
* the corresponding array of status.
+ *
+ * When the mm argument is not NULL, task_nodes is expected to be the
+ * cpuset nodemask for the task which owns the mm_struct, and the
+ * values located in (*pages) are expected to be virtual addresses.
+ *
+ * When the mm argument is NULL, the values located at (*pages) are
+ * expected to be physical addresses, and task_nodes is expected to
+ * be empty.
*/
static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
unsigned long nr_pages,
@@ -2226,7 +2336,14 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
goto out_flush;
err = -EACCES;
- if (!node_isset(node, task_nodes))
+ /*
+ * if mm is NULL, then the pages are addressed via physical
+ * address and the task_nodes structure is empty. Validation
+ * of migratability is deferred to add_phys_page_for_migration
+ * where vma's that map the address will have their node_mask
+ * checked to ensure the requested node bit is set.
+ */
+ if (mm && !node_isset(node, task_nodes))
goto out_flush;
if (current_node == NUMA_NO_NODE) {
@@ -2243,10 +2360,17 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
/*
* Errors in the page lookup or isolation are not fatal and we simply
- * report them via status
+ * report them via status.
+ *
+ * If mm is NULL, then p treated as is a physical address.
*/
- err = add_virt_page_for_migration(mm, p, current_node, &pagelist,
- flags & MPOL_MF_MOVE_ALL);
+ if (mm)
+ err = add_virt_page_for_migration(mm, p, current_node, &pagelist,
+ flags & MPOL_MF_MOVE_ALL);
+ else
+ err = add_phys_page_for_migration(p, current_node, &pagelist,
+ flags & MPOL_MF_MOVE_ALL);
+
if (err > 0) {
/* The page is successfully queued for migration */
@@ -2334,6 +2458,37 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages,
mmap_read_unlock(mm);
}
+/*
+ * Determine the nodes pages pointed to by the physical addresses in the
+ * pages array, and store those node values in the status array
+ */
+static void do_phys_pages_stat_array(unsigned long nr_pages,
+ const void __user **pages, int *status)
+{
+ unsigned long i;
+
+ for (i = 0; i < nr_pages; i++) {
+ unsigned long pfn = (unsigned long)(*pages) >> PAGE_SHIFT;
+ struct page *page = pfn_to_online_page(pfn);
+ int err = -ENOENT;
+
+ if (!page)
+ goto set_status;
+
+ get_page(page);
+
+ if (!is_zone_device_page(page))
+ err = page_to_nid(page);
+
+ put_page(page);
+set_status:
+ *status = err;
+
+ pages++;
+ status++;
+ }
+}
+
static int get_compat_pages_array(const void __user *chunk_pages[],
const void __user * __user *pages,
unsigned long chunk_nr)
@@ -2376,7 +2531,10 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
break;
}
- do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
+ if (mm)
+ do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
+ else
+ do_phys_pages_stat_array(chunk_nr, chunk_pages, chunk_status);
if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
break;
@@ -2449,7 +2607,7 @@ static int kernel_move_pages(pid_t pid, unsigned long nr_pages,
nodemask_t task_nodes;
/* Check flags */
- if (flags & ~(MPOL_MF_MOVE|MPOL_MF_MOVE_ALL))
+ if (flags & ~(MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
return -EINVAL;
if ((flags & MPOL_MF_MOVE_ALL) && !capable(CAP_SYS_NICE))
@@ -2477,6 +2635,42 @@ SYSCALL_DEFINE6(move_pages, pid_t, pid, unsigned long, nr_pages,
return kernel_move_pages(pid, nr_pages, pages, nodes, status, flags);
}
+/*
+ * Move a list of physically-addressed pages to the list of target nodes
+ */
+static int kernel_move_phys_pages(unsigned long nr_pages,
+ const void __user * __user *pages,
+ const int __user *nodes,
+ int __user *status, int flags)
+{
+ nodemask_t dummy_nodes;
+
+ if (flags & ~(MPOL_MF_MOVE|MPOL_MF_MOVE_ALL))
+ return -EINVAL;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (!nodes)
+ return do_pages_stat(NULL, nr_pages, pages, status);
+
+ /*
+ * When the mm argument to do_pages_move is null, the task_nodes
+ * argument is ignored, so pass in an empty nodemask as a dummy.
+ */
+ nodes_clear(dummy_nodes);
+ return do_pages_move(NULL, dummy_nodes, nr_pages, pages, nodes, status,
+ flags);
+}
+
+SYSCALL_DEFINE5(move_phys_pages, unsigned long, nr_pages,
+ const void __user * __user *, pages,
+ const int __user *, nodes,
+ int __user *, status, int, flags)
+{
+ return kernel_move_phys_pages(nr_pages, pages, nodes, status, flags);
+}
+
#ifdef CONFIG_NUMA_BALANCING
/*
* Returns true if this is a safe migration target node for misplaced NUMA
diff --git a/tools/include/uapi/asm-generic/unistd.h b/tools/include/uapi/asm-generic/unistd.h
index 75f00965ab15..13bc8dd16d6b 100644
--- a/tools/include/uapi/asm-generic/unistd.h
+++ b/tools/include/uapi/asm-generic/unistd.h
@@ -842,8 +842,14 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+/* CONFIG_MMU only */
+#ifndef __ARCH_NOMMU
+#define __NR_move_phys_pages 462
+__SYSCALL(__NR_move_phys_pages, sys_move_phys_pages)
+#endif
+
#undef __NR_syscalls
-#define __NR_syscalls 462
+#define __NR_syscalls 463
/*
* 32 bit systems traditionally used different
--
2.39.1
^ permalink raw reply related
* [RFC v3 1/3] mm/migrate: refactor add_page_for_migration for code re-use
From: Gregory Price @ 2024-03-19 17:26 UTC (permalink / raw)
To: linux-mm
Cc: linux-api, linux-arch, linux-kselftest, linux-kernel, ying.huang,
dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm, shuah,
Gregory Price
In-Reply-To: <20240319172609.332900-1-gregory.price@memverge.com>
add_page_for_migration presently does two actions:
1) validates the page is present and migratable
2) isolates the page from LRU and puts it into the migration list
Break add_page_for_migration into 2 functions:
add_page_for_migration - isolate the page from LUR and add to list
add_virt_page_for_migration - validate the page and call the above
add_page_for_migration does not require the mm_struct and so can be
re-used for a physical addressing version of move_pages
Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
mm/migrate.c | 84 +++++++++++++++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 34 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index c27b1f8097d4..27071a07ffbb 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2066,6 +2066,46 @@ static int do_move_pages_to_node(struct list_head *pagelist, int node)
return err;
}
+/*
+ * Isolates the page from the LRU and puts it into the given pagelist
+ * Returns:
+ * errno - if the page cannot be isolated
+ * 0 - when it doesn't have to be migrated because it is already on the
+ * target node
+ * 1 - when it has been queued
+ */
+static int add_page_for_migration(struct page *page,
+ struct folio *folio,
+ int node,
+ struct list_head *pagelist,
+ bool migrate_all)
+{
+ if (folio_is_zone_device(folio))
+ return -ENOENT;
+
+ if (folio_nid(folio) == node)
+ return 0;
+
+ if (page_mapcount(page) > 1 && !migrate_all)
+ return -EACCES;
+
+ if (folio_test_hugetlb(folio)) {
+ if (isolate_hugetlb(folio, pagelist))
+ return 1;
+ return -EBUSY;
+ }
+
+ if (!folio_isolate_lru(folio))
+ return -EBUSY;
+
+ list_add_tail(&folio->lru, pagelist);
+ node_stat_mod_folio(folio,
+ NR_ISOLATED_ANON + folio_is_file_lru(folio),
+ folio_nr_pages(folio));
+
+ return 1;
+}
+
/*
* Resolves the given address to a struct page, isolates it from the LRU and
* puts it to the given pagelist.
@@ -2075,19 +2115,19 @@ static int do_move_pages_to_node(struct list_head *pagelist, int node)
* target node
* 1 - when it has been queued
*/
-static int add_page_for_migration(struct mm_struct *mm, const void __user *p,
- int node, struct list_head *pagelist, bool migrate_all)
+static int add_virt_page_for_migration(struct mm_struct *mm,
+ const void __user *p, int node, struct list_head *pagelist,
+ bool migrate_all)
{
struct vm_area_struct *vma;
unsigned long addr;
struct page *page;
struct folio *folio;
- int err;
+ int err = -EFAULT;
mmap_read_lock(mm);
addr = (unsigned long)untagged_addr_remote(mm, p);
- err = -EFAULT;
vma = vma_lookup(mm, addr);
if (!vma || !vma_migratable(vma))
goto out;
@@ -2095,41 +2135,17 @@ static int add_page_for_migration(struct mm_struct *mm, const void __user *p,
/* FOLL_DUMP to ignore special (like zero) pages */
page = follow_page(vma, addr, FOLL_GET | FOLL_DUMP);
- err = PTR_ERR(page);
- if (IS_ERR(page))
- goto out;
-
err = -ENOENT;
if (!page)
goto out;
- folio = page_folio(page);
- if (folio_is_zone_device(folio))
- goto out_putfolio;
-
- err = 0;
- if (folio_nid(folio) == node)
- goto out_putfolio;
+ err = PTR_ERR(page);
+ if (IS_ERR(page))
+ goto out;
- err = -EACCES;
- if (page_mapcount(page) > 1 && !migrate_all)
- goto out_putfolio;
+ folio = page_folio(page);
+ err = add_page_for_migration(page, folio, node, pagelist, migrate_all);
- err = -EBUSY;
- if (folio_test_hugetlb(folio)) {
- if (isolate_hugetlb(folio, pagelist))
- err = 1;
- } else {
- if (!folio_isolate_lru(folio))
- goto out_putfolio;
-
- err = 1;
- list_add_tail(&folio->lru, pagelist);
- node_stat_mod_folio(folio,
- NR_ISOLATED_ANON + folio_is_file_lru(folio),
- folio_nr_pages(folio));
- }
-out_putfolio:
/*
* Either remove the duplicate refcount from folio_isolate_lru()
* or drop the folio ref if it was not isolated.
@@ -2229,7 +2245,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
* Errors in the page lookup or isolation are not fatal and we simply
* report them via status
*/
- err = add_page_for_migration(mm, p, current_node, &pagelist,
+ err = add_virt_page_for_migration(mm, p, current_node, &pagelist,
flags & MPOL_MF_MOVE_ALL);
if (err > 0) {
--
2.39.1
^ permalink raw reply related
* [RFC v3 0/3] move_phys_pages syscall - migrate page contents given
From: Gregory Price @ 2024-03-19 17:26 UTC (permalink / raw)
To: linux-mm
Cc: linux-api, linux-arch, linux-kselftest, linux-kernel, ying.huang,
dan.j.williams, honggyu.kim, corbet, arnd, luto, akpm, shuah,
Gregory Price
v3:
- pull forward to v6.8
- style and small fixups recommended by jcameron
- update syscall number (will do all archs when RFC tag drops)
- update for new folio code
- added OCP link to device-tracked address hotness proposal
- kept void* over __u64 simply because it integrates cleanly with
existing migration code. If there's strong opinions, I can refactor.
This patch set is a proposal for a syscall analogous to move_pages,
that migrates pages between NUMA nodes using physical addressing.
The intent is to better enable user-land system-wide memory tiering
as CXL devices begin to provide memory resources on the PCIe bus.
For example, user-land software which is making decisions based on
data sources which expose physical address information no longer
must convert that information to virtual addressing to act upon it
(see background for info on how physical addresses are acquired).
The syscall requires CAP_SYS_ADMIN, since physical address source
information is typically protected by the same (or CAP_SYS_NICE).
This patch set broken into 3 patches:
1) refactor of existing migration code for code reuse
2) The sys_move_phys_pages system call.
3) ktest of the syscall
The sys_move_phys_pages system call validates the page may be
migrated by checking migratable-status of each vma mapping the page,
and the intersection of cpuset policies each vma's task.
Background:
Userspace job schedulers, memory managers, and tiering software
solutions depend on page migration syscalls to reallocate resources
across NUMA nodes. Currently, these calls enable movement of memory
associated with a specific PID. Moves can be requested in coarse,
process-sized strokes (as with migrate_pages), and on specific virtual
pages (via move_pages).
However, a number of profiling mechanisms provide system-wide information
that would benefit from a physical-addressing version move_pages.
There are presently at least 4 ways userland can acquire physical
address information for use with this interface, and 1 hardware offload
mechanism being proposed by opencompute.
1) /proc/pid/pagemap: can be used to do page table translations.
This is only really useful for testing, and the ktest was
written using this functionality.
2) X86: IBS (AMD) and PEBS (Intel) can be configured to return physical
and/or vitual address information.
3) zoneinfo: /proc/zoneinfo exposes the start PFN of zones
4) /sys/kernel/mm/page_idle: A way to query whether a PFN is idle.
So long as the page size is known, this can be used to identify
system-wide idle pages that could be migrated to lower tiers.
https://docs.kernel.org/admin-guide/mm/idle_page_tracking.html
5) CXL Offloaded Hotness Monitoring (Proposed): a CXL memory device
may provide hot/cold information about its memory. For example,
it may report the hottest device addresses (0-based) or a physical
address (if it has access to decoders for convert bases).
DPA can be cheaply converted to HPA by combining it with data
exposed by /sys/bus/cxl/ information (region address bases).
See: https://www.opencompute.org/documents/ocp-cms-hotness-tracking-requirements-white-paper-pdf-1
Information from these sources facilitates systemwide resource management,
but with the limitations of migrate_pages and move_pages applying to
individual tasks, their outputs must be converted back to virtual addresses
and re-associated with specific PIDs.
Doing this reverse-translation outside of the kernel requires considerable
space and compute, and it will have to be performed again by the existing
system calls. Much of this work can be avoided if the pages can be
migrated directly with physical memory addressing.
Gregory Price (3):
mm/migrate: refactor add_page_for_migration for code re-use
mm/migrate: Create move_phys_pages syscall
ktest: sys_move_phys_pages ktest
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 8 +-
kernel/sys_ni.c | 1 +
mm/migrate.c | 288 ++++++++++++++++++++----
tools/include/uapi/asm-generic/unistd.h | 8 +-
tools/testing/selftests/mm/migration.c | 99 ++++++++
8 files changed, 370 insertions(+), 41 deletions(-)
--
2.39.1
^ permalink raw reply
* Process synchronize with CAS
From: qiqi tang @ 2024-03-18 14:17 UTC (permalink / raw)
To: linux-api
Hi, everyone
I am interested in the interface of linux. As we all know, there
is CAS way to accomplish the lockless ring buffer with the kernel .
The kernel api can realize the datastruct Such as
__sync_fetch_and_add type __sync_fetch_and_sub (type * ptr, type
value, ...) and so on
But I find the limitation that this CAS is work good for the thread
model rather than Process model . I want to create a shared Memory
and use mmap API to mount this shared memory into producter and
consumer process. What should I do to realize the synchronization
using the CAS algorithm . Are there have any API can over cross
Multi-Process just like the thread model? I would appreciate that
someone can provide the information for this email. Thank you!
Regards,
Jack
^ permalink raw reply
* Re: [PATCH v3 2/2] VT: Allow to get max font width and height
From: Helge Deller @ 2024-03-15 9:16 UTC (permalink / raw)
To: legion, Greg Kroah-Hartman, Jiri Slaby
Cc: LKML, kbd, linux-api, linux-fbdev, linux-serial
In-Reply-To: <78fcb9ad77b88edee8768806ce6a4d23f6d33118.1710252966.git.legion@kernel.org>
On 3/12/24 15:23, legion@kernel.org wrote:
> From: Alexey Gladkov <legion@kernel.org>
>
> The Console drivers has more restrictive font size limits than vt_ioctl.
> This leads to errors that are difficult to handle. If a font whose size
> is not supported is used, an EINVAL error will be returned, which is
> also returned in case of errors in the font itself. At the moment there
> is no way to understand what font sizes the current console driver
> supports.
>
> To solve this problem, we need to transfer information about the
> supported font to userspace from the console driver.
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
Acked-by: Helge Deller <deller@gmx.de>
Helge
> ---
> drivers/video/console/newport_con.c | 21 +++++++++++++++++----
> drivers/video/console/sticon.c | 25 +++++++++++++++++++++++--
> drivers/video/console/vgacon.c | 21 ++++++++++++++++++++-
> drivers/video/fbdev/core/fbcon.c | 22 +++++++++++++++++++++-
> 4 files changed, 81 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
> index e8e4f82cd4a1..87f174a95fa8 100644
> --- a/drivers/video/console/newport_con.c
> +++ b/drivers/video/console/newport_con.c
> @@ -33,6 +33,9 @@
>
> #define NEWPORT_LEN 0x10000
>
> +#define NEWPORT_MAX_FONT_WIDTH 8
> +#define NEWPORT_MAX_FONT_HEIGHT 16
> +
> #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
>
> static unsigned char *font_data[MAX_NR_CONSOLES];
> @@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
> {
> int cols, rows;
>
> - cols = newport_xsize / 8;
> - rows = newport_ysize / 16;
> + cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
> + rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
> vc->vc_can_do_color = 1;
> if (init) {
> vc->vc_cols = cols;
> @@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
>
> /* ladis: when I grow up, there will be a day... and more sizes will
> * be supported ;-) */
> - if ((w != 8) || (h != 16) || (vpitch != 32)
> - || (op->charcount != 256 && op->charcount != 512))
> + if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
> + (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
> return -EINVAL;
>
> if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
> @@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
> return newport_set_def_font(vc->vc_num, op);
> }
>
> +static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> + info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
> + info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
> + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> + return 0;
> +}
> +
> static int newport_font_set(struct vc_data *vc, struct console_font *font,
> unsigned int vpitch, unsigned int flags)
> {
> @@ -688,6 +700,7 @@ const struct consw newport_con = {
> .con_scroll = newport_scroll,
> .con_switch = newport_switch,
> .con_blank = newport_blank,
> + .con_font_info = newport_font_info,
> .con_font_set = newport_font_set,
> .con_font_default = newport_font_default,
> .con_save_screen = newport_save_screen
> diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
> index 992a4fa431aa..d32ca458eb77 100644
> --- a/drivers/video/console/sticon.c
> +++ b/drivers/video/console/sticon.c
> @@ -56,6 +56,11 @@
> #define BLANK 0
> static int vga_is_gfx;
>
> +#define STICON_MIN_FONT_WIDTH 6
> +#define STICON_MIN_FONT_HEIGHT 6
> +#define STICON_MAX_FONT_WIDTH 32
> +#define STICON_MAX_FONT_HEIGHT 32
> +
> #define STI_DEF_FONT sticon_sti->font
>
> /* borrowed from fbcon.c */
> @@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
> struct sti_cooked_font *cooked_font;
> unsigned char *data = op->data, *p;
>
> - if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
> - || (op->charcount != 256 && op->charcount != 512))
> + if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
> + !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
> + (vpitch != 32) ||
> + (op->charcount != 256 && op->charcount != 512))
> return -EINVAL;
> pitch = ALIGN(w, 8) / 8;
> bpc = pitch * h;
> @@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
> return sticon_set_font(vc, font, vpitch);
> }
>
> +static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> + info->min_width = STICON_MIN_FONT_WIDTH;
> + info->min_height = STICON_MIN_FONT_HEIGHT;
> +
> + info->max_width = STICON_MAX_FONT_WIDTH;
> + info->max_height = STICON_MAX_FONT_HEIGHT;
> +
> + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> + return 0;
> +}
> +
> static void sticon_init(struct vc_data *c, int init)
> {
> struct sti_struct *sti = sticon_sti;
> @@ -371,6 +391,7 @@ static const struct consw sti_con = {
> .con_scroll = sticon_scroll,
> .con_switch = sticon_switch,
> .con_blank = sticon_blank,
> + .con_font_info = sticon_font_info,
> .con_font_set = sticon_font_set,
> .con_font_default = sticon_font_default,
> .con_build_attr = sticon_build_attr,
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 8ef1579fa57f..b75d31ef3353 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -61,6 +61,10 @@ static struct vgastate vgastate;
> #define BLANK 0x0020
>
> #define VGA_FONTWIDTH 8 /* VGA does not support fontwidths != 8 */
> +
> +#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
> +#define VGACON_MAX_FONT_HEIGHT 32
> +
> /*
> * Interface used by the world
> */
> @@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
> return 0;
> }
>
> +static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> + info->min_width = VGACON_MAX_FONT_WIDTH;
> + info->min_height = 0;
> +
> + info->max_width = VGACON_MAX_FONT_WIDTH;
> + info->max_height = VGACON_MAX_FONT_HEIGHT;
> +
> + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> + return 0;
> +}
> +
> static int vgacon_font_set(struct vc_data *c, struct console_font *font,
> unsigned int vpitch, unsigned int flags)
> {
> @@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
> if (vga_video_type < VIDEO_TYPE_EGAM)
> return -EINVAL;
>
> - if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
> + if (font->width != VGACON_MAX_FONT_WIDTH ||
> + font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
> (charcount != 256 && charcount != 512))
> return -EINVAL;
>
> @@ -1177,6 +1195,7 @@ const struct consw vga_con = {
> .con_scroll = vgacon_scroll,
> .con_switch = vgacon_switch,
> .con_blank = vgacon_blank,
> + .con_font_info = vgacon_font_info,
> .con_font_set = vgacon_font_set,
> .con_font_get = vgacon_font_get,
> .con_resize = vgacon_resize,
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 46823c2e2ba1..e10abe416159 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -101,6 +101,9 @@ enum {
> FBCON_LOGO_DONTSHOW = -3 /* do not show the logo */
> };
>
> +#define FBCON_MAX_FONT_WIDTH 32
> +#define FBCON_MAX_FONT_HEIGHT 32
> +
> static struct fbcon_display fb_display[MAX_NR_CONSOLES];
>
> static struct fb_info *fbcon_registered_fb[FB_MAX];
> @@ -2456,6 +2459,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> return ret;
> }
>
> +
> +static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> + info->min_width = 0;
> + info->min_height = 0;
> +
> + info->max_width = FBCON_MAX_FONT_WIDTH;
> + info->max_height = FBCON_MAX_FONT_HEIGHT;
> +
> + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> + return 0;
> +}
> +
> +
> /*
> * User asked to set font; we are guaranteed that charcount does not exceed 512
> * but lets not assume that, since charcount of 512 is small for unicode support.
> @@ -2483,7 +2501,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
> h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
> return -EINVAL;
>
> - if (font->width > 32 || font->height > 32)
> + if (font->width > FBCON_MAX_FONT_WIDTH ||
> + font->height > FBCON_MAX_FONT_HEIGHT)
> return -EINVAL;
>
> /* Make sure drawing engine can handle the font */
> @@ -3158,6 +3177,7 @@ static const struct consw fb_con = {
> .con_scroll = fbcon_scroll,
> .con_switch = fbcon_switch,
> .con_blank = fbcon_blank,
> + .con_font_info = fbcon_font_info,
> .con_font_set = fbcon_set_font,
> .con_font_get = fbcon_get_font,
> .con_font_default = fbcon_set_def_font,
^ permalink raw reply
* Re: [PATCH v3 1/2] VT: Add KDFONTINFO ioctl
From: Helge Deller @ 2024-03-15 9:15 UTC (permalink / raw)
To: legion, Greg Kroah-Hartman, Jiri Slaby
Cc: LKML, kbd, linux-api, linux-fbdev, linux-serial
In-Reply-To: <ed056326540f04b72c97a276fbcc316e1b2f6371.1710252966.git.legion@kernel.org>
On 3/12/24 15:23, legion@kernel.org wrote:
> From: Alexey Gladkov <legion@kernel.org>
>
> Each driver has its own restrictions on font size. There is currently no
> way to understand what the requirements are. The new ioctl allows
> userspace to get the minmum and maximum font size values.
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
Acked-by: Helge Deller <deller@gmx.de>
Helge
> ---
> drivers/tty/vt/vt.c | 24 ++++++++++++++++++++++++
> drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
> include/linux/console.h | 2 ++
> include/linux/vt_kern.h | 1 +
> include/uapi/linux/kd.h | 13 ++++++++++++-
> 5 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 156efda7c80d..8c2a3d98b5ec 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4680,6 +4680,30 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
> return -ENOSYS;
> }
>
> +int con_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> + int rc = -EINVAL;
> +
> + info->min_height = 0;
> + info->max_height = max_font_height;
> +
> + info->min_width = 0;
> + info->max_width = max_font_width;
> +
> + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> + console_lock();
> + if (vc->vc_mode != KD_TEXT)
> + rc = -EINVAL;
> + else if (vc->vc_sw->con_font_info)
> + rc = vc->vc_sw->con_font_info(vc, info);
> + else
> + rc = -ENOSYS;
> + console_unlock();
> +
> + return rc;
> +}
> +
> /*
> * Interface exported to selection and vcs.
> */
> diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
> index 8c685b501404..b3b4e4b69366 100644
> --- a/drivers/tty/vt/vt_ioctl.c
> +++ b/drivers/tty/vt/vt_ioctl.c
> @@ -479,6 +479,19 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
> break;
> }
>
> + case KDFONTINFO: {
> + struct console_font_info fnt_info;
> +
> + if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
> + return -EFAULT;
> + ret = con_font_info(vc, &fnt_info);
> + if (ret)
> + return ret;
> + if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
> + return -EFAULT;
> + break;
> + }
> +
> default:
> return -ENOIOCTLCMD;
> }
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 779d388af8a0..5bea6f6c2042 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -20,6 +20,7 @@
> #include <linux/types.h>
>
> struct vc_data;
> +struct console_font_info;
> struct console_font_op;
> struct console_font;
> struct module;
> @@ -59,6 +60,7 @@ struct consw {
> unsigned int lines);
> int (*con_switch)(struct vc_data *vc);
> int (*con_blank)(struct vc_data *vc, int blank, int mode_switch);
> + int (*con_font_info)(struct vc_data *vc, struct console_font_info *info);
> int (*con_font_set)(struct vc_data *vc, struct console_font *font,
> unsigned int vpitch, unsigned int flags);
> int (*con_font_get)(struct vc_data *vc, struct console_font *font,
> diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
> index c1f5aebef170..6bda4cc1fe6f 100644
> --- a/include/linux/vt_kern.h
> +++ b/include/linux/vt_kern.h
> @@ -32,6 +32,7 @@ void do_blank_screen(int entering_gfx);
> void do_unblank_screen(int leaving_gfx);
> void poke_blanked_console(void);
> int con_font_op(struct vc_data *vc, struct console_font_op *op);
> +int con_font_info(struct vc_data *vc, struct console_font_info *info);
> int con_set_cmap(unsigned char __user *cmap);
> int con_get_cmap(unsigned char __user *cmap);
> void scrollback(struct vc_data *vc);
> diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h
> index 6b384065c013..781e086e55bf 100644
> --- a/include/uapi/linux/kd.h
> +++ b/include/uapi/linux/kd.h
> @@ -183,8 +183,19 @@ struct console_font {
>
> #define KD_FONT_FLAG_DONT_RECALC 1 /* Don't recalculate hw charcell size [compat] */
>
> +#define KDFONTINFO 0x4B73 /* font information */
> +
> +#define KD_FONT_INFO_FLAG_LOW_SIZE (1U << 0) /* 256 */
> +#define KD_FONT_INFO_FLAG_HIGH_SIZE (1U << 1) /* 512 */
> +
> +struct console_font_info {
> + unsigned int min_width, min_height; /* minimal font size */
> + unsigned int max_width, max_height; /* maximum font size */
> + unsigned int flags; /* KD_FONT_INFO_FLAG_* */
> +};
> +
> /* note: 0x4B00-0x4B4E all have had a value at some time;
> don't reuse for the time being */
> -/* note: 0x4B60-0x4B6D, 0x4B70-0x4B72 used above */
> +/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
>
> #endif /* _UAPI_LINUX_KD_H */
^ permalink raw reply
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Thomas Gleixner @ 2024-03-14 18:42 UTC (permalink / raw)
To: Sagi Maimon
Cc: richardcochran, luto, datglx, mingo, bp, dave.hansen, x86, hpa,
arnd, geert, peterz, hannes, sohil.mehta, rick.p.edgecombe,
nphamcs, palmer, keescook, legion, mark.rutland, mszeredi, casey,
reibax, davem, brauner, linux-kernel, linux-api, linux-arch,
netdev
In-Reply-To: <CAMuE1bHOm2Y1bOpggStMOjZhN5TaxoC1gJea5Mdrc+mormQg0g@mail.gmail.com>
On Thu, Mar 14 2024 at 17:46, Sagi Maimon wrote:
Can you please trim your replies? I really have better things to do than
doing detective work to find 10 new lines within 200+ irrelevant ones.
> On Thu, Mar 14, 2024 at 1:12 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> Please read and follow the documentation provided at:
>>
>> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
>>
> I have missed this part on prviews reply.
> I have read the documentation above and I think that the variable
> declarations at the beginning of a function is in reverse fir tree
> order meaning from big to small, but I guess that I am missing something,
> can you please explain what is wrong with the variable declaration,
> so I can fix it.
>> > + struct timespec64 ts_a, ts_a1, ts_b, ts_a2;
>> > + struct system_device_crosststamp xtstamp_a1, xtstamp_a2, xtstamp_b;
>> > + const struct k_clock *kc_a, *kc_b;
>> > + ktime_t ktime_a;
>> > + s64 ts_offs_err = 0;
>> > + int error = 0;
>> > + bool crosstime_support_a = false;
>> > + bool crosstime_support_b = false;
It's not about the data type. Look at the three layouts and figure out
which one is better to parse.
^ permalink raw reply
* Re: [PATCH v3] LSM: use 32 bit compatible data types in LSM syscalls.
From: Paul Moore @ 2024-03-14 18:18 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Casey Schaufler, LSM List, Linux kernel mailing list, linux-api,
Mickaël Salaün, James Morris, Serge Hallyn,
John Johansen, Tetsuo Handa, Stephen Smalley
In-Reply-To: <20240314180143.GA26431@altlinux.org>
On Thu, Mar 14, 2024 at 2:01 PM Dmitry V. Levin <ldv@strace.io> wrote:
> On Thu, Mar 14, 2024 at 11:30:53AM -0400, Paul Moore wrote:
> > On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> > > >
> > > > LSM: use 32 bit compatible data types in LSM syscalls.
> > > >
> > > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr()
> > > > and lsm_get_self_attr() from size_t to u32. This avoids the need to
> > > > have different interfaces for 32 and 64 bit systems.
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io>
> > > > ---
> > > > include/linux/lsm_hook_defs.h | 4 ++--
> > > > include/linux/security.h | 8 ++++----
> > > > security/apparmor/lsm.c | 4 ++--
> > > > security/lsm_syscalls.c | 10 +++++-----
> > > > security/security.c | 12 ++++++------
> > > > security/selinux/hooks.c | 4 ++--
> > > > security/smack/smack_lsm.c | 4 ++--
> > > > tools/testing/selftests/lsm/common.h | 6 +++---
> > > > tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 10 +++++-----
> > > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++----
> > > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++---
> > > > 11 files changed, 38 insertions(+), 38 deletions(-)
> > >
> > > Okay, this looks better, I'm going to merge this into lsm/stable-6.9
> > > and put it through the usual automated testing as well as a kselftest
> > > run to make sure everything there is still okay. Assuming all goes
> > > well and no one raises any objections, I'll likely send this up to
> > > Linus tomorrow.
> >
> > I had to squash the code snippet below into the patch to address a
> > build problem identified by the kernel build robot. I'm going to keep
> > Casey's sign-off and Dmitry's reported-reviewed tag as I feel this
> > change is minor, but if anyone has any objections please let me know
> > soon.
> >
> > [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea]
> >
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 77eb9b0e7685..e619ac10cd23 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd,
> > struct cachestat __user *cstat, unsigned int flags);
> > asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un
> > signed int flags);
> > asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> > - size_t *size, __u32 flags);
> > + u32 *size, u32 flags);
> > asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> > - size_t size, __u32 flags);
> > -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags);
> > + u32 size, u32 flags);
> > +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
>
> Fine with me, thanks.
>
> btw, with the change above, u32 will become about twice more popular
> in include/linux/syscalls.h than __u32.
I was looking at that when I was putting the patch together this
morning, trying to decide which was the "correct" choice between 'u32'
and '__u32' and wasn't able to find a good explanation of which is the
"right" option in this file. Ultimately I went with 'u32' as I tend
to follow some old guidance of: '__u32' for userspace headers, 'u32'
for kernel headers.
If it should be the other way, please let me know. I just want to
keep it consistent across the LSM syscalls.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Thomas Gleixner @ 2024-03-14 18:08 UTC (permalink / raw)
To: Sagi Maimon
Cc: richardcochran, luto, tglx, mingo, bp, dave.hansen, x86, hpa,
arnd, geert, peterz, hannes, sohil.mehta, rick.p.edgecombe,
nphamcs, palmer, keescook, legion, mark.rutland, mszeredi, casey,
reibax, davem, brauner, linux-kernel, linux-api, linux-arch,
netdev
In-Reply-To: <CAMuE1bH_H9E+Zx365G9AtmWSmhW-kPPB+-=8s2rH4hpxqE+dHQ@mail.gmail.com>
Sagi!
On Thu, Mar 14 2024 at 14:19, Sagi Maimon wrote:
> On Thu, Mar 14, 2024 at 1:12 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Thu, Mar 14 2024 at 11:05, Sagi Maimon wrote:
>> > Some user space applications need to read a couple of different clocks.
>> > Each read requires moving from user space to kernel space.
>> > Reading each clock separately (syscall) introduces extra
>> > unpredictable/unmeasurable delay. Minimizing this delay contributes to user
>> > space actions on these clocks (e.g. synchronization etc).
>>
>> I asked for a proper description of the actual problem several times now
>> and you still provide some handwaving blurb. Feel free to ignore me, but
>> then please don't be surprised if I ignore you too.
>>
> Nobody is ignoring your notes, and I address any notes given by any
> maintainer in the most serious way.
> As far as explaining the actual problem this is the best that I can,
> but let me try to explain better:
> We did many tests with different CPU loading and compared sampling the
> same clock twice,
> once in user space and once by using the system call.
> We have noticed an improvement up to hundreds of nanoseconds while
> using the system call.
> Those results improved our ability to sync different PHCs
So let me express how I understand the problem - as far as I decoded it
from your writeups:
Synchronizing two PHCs requires to read timestamps from both and
correlate them. Currently this requires several seperate system calls.
This is subject to unmeasurable delays due to system call overhead,
preemption and interrupts which makes the correlation imprecise.
Therefore you want a system call, which samples two clocks at once, to
make the correlation more precise.
Right? For the further comments I assume this is what you are trying to
say and to solve.
So far so good, except that I do not agree with that reasoning at all:
1. The delays are measurable and as precise as the cross time stamp
mechanism (hardware or software based) allows.
2. The system call overhead is completely irrelevant.
3. The time deltas between the sample points are irrelevant within a
reasonable upper bound to the time delta between the two outer
sample points.
4. The alledged higher precision is based on a guesstimate and not on
correctness. Just because it behaves slightly better in testing
does not make it any more correct.
5. The problem can be solved with maximal possible accuracy by using
the existing PTP IOCTLs.
See below.
>> Also why does reading two random clocks make any sense at all? Your code
>> allows to read CLOCK_TAI and CLOCK_THREAD_CPUTIME_ID. What for?
>>
> Initially we needed to sync some different PHCs for our user space
> application, that is why we came with this idea.
> The first idea was an IOCTL that returned samples of several PHCs for
> the need of synchronization.
> Richard Cochran suggested a system call instead, which will add the
> ability to get various system clocks, while this
> implementation is more complex then IOCTL, I think that he was right,
> for future usage.
Which future usage? We are not introducing swiss army knife interfaces
just because there might be an illusional use case somewhere in the
unspecified future.
Adding a system call needs a proper design and justification. Handwaving
future usage is not enough.
Documentation/process/adding-syscalls.rst is very clear about what is
required for a new system call.
>> This needs to be split up into:
>>
>> 1) Infrastructure in posix-timers.c
>> 2) Wire up the syscall in x86
>> 3) Infrastructure in posix-clock.c
>> 4) Usage in ptp_clock.c
>>
>> and not as a big lump of everything.
>>
> I know, but I think the benefit worth it
It's worth it because it makes review easier. It's well documented in
the process documentation that patches should do one thing and not a
whole lump of changes.
>> > + if (!error) {
>> > + if (clock_b == CLOCK_MONOTONIC_RAW) {
>> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_monoraw);
>> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
>> > + goto out;
>> > + } else if (clock_b == CLOCK_REALTIME) {
>> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_realtime);
>> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
>> > + goto out;
>> > + } else {
>> > + crosstime_support_a = true;
>>
>> Right. If clock_b is anything else than CLOCK_MONOTONIC_RAW or
>> CLOCK_REALTIME then this is true.
>>
>> > + }
>> > + }
>>
>> So in case of an error, this just keeps going with an uninitialized
>> xtstamp_a1 and if the clock_b part succeeds it continues and operates on
>> garbage.
>>
> On error xtstamp_a1 will be taken again using clock_get_crosstimespec
> so no one will be operating on garbage.
It will not, because crosstime_support_a == false. It will therefore
fall back to kc_a->clock_get_timespec(), no?
Sorry, I misread the code vs. using the uninitialized value, but this is
just unneccesary hard to follow.
>> > + if (crosstime_support_a)
>> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a2);
>> > + else
>> > + error = kc_a->clock_get_timespec(clock_a, &ts_a2);
>> > +
>> > + if (error)
>> > + return error;
>>
>> The logic and the code flow here are unreadable garbage and there are
>> zero comments what this is supposed to do.
>>
> I will add comments.
> please no need to use negative words like "garbage" (not the first time),
> please keep it professional and civilized.
Let me rephrase:
The code and the logic is incomprehensible unless I waste an unjustified
amount of time to decode it. Sorry, I don't have that time.
>> > + if (crosstime_support_a) {
>> > + ktime_a = ktime_sub(xtstamp_a2.device, xtstamp_a1.device);
>> > + ts_offs_err = ktime_divns(ktime_a, 2);
>> > + ktime_a = ktime_add_ns(xtstamp_a1.device, (u64)ts_offs_err);
>> > + ts_a1 = ktime_to_timespec64(ktime_a);
>>
>> This is just wrong.
>>
>> read(a1);
>> read(b);
>> read(a2);
>>
>> You _CANNOT_ assume that (a1 + ((a2 - a1) / 2) is anywhere close to the
>> point in time where 'b' is read. This code is preemtible and
>> interruptible. I explained this to you before.
>>
>> Your explanation in the comment above the function is just wishful
>> thinking.
>>
> you explained it before, but still it is better then two consecutive
> user space calls which are also preemptible
> and the userspace to kernel context switch time is added.
It might be marginally better, but it is still just _pretending_ that it
does the right thing, is correct and better than the existing IOCTLs.
If your user space implementation has the same algorithm, then I'm
absolutely not surprised that the results are not useful. Why?
You simply cannot use the midpoint of the outer samples if you want to
have precise results if there is no guarantee that b was sampled exactly
in the midpoint of a1 and a2. A hardware implementation might give that
guarantee, but the kernel cannot.
But why using the midpoint in the first place?
There is absolutely no reason to do so because the sampling points a1, b
and a2 can be precisely determined with the precision of the cross time
stamp mechanism, which is best with a hardware based cross time stamp
obviously.
The whole point of ptp::info::getcrosststamp() is to get properly
correlated clock samples of
1) PHC clock
2) CLOCK_MONOTONIC_RAW
3) CLOCK_REALTIME
So if you take 3 samples:
get_cross_timestamp(a1);
get_cross_timestamp(b);
get_cross_timestamp(a2);
then each of them provides:
- device time
- correlated CLOCK_MONOTONIC_RAW
- correlated CLOCK_REALTIME
Ergo the obvious thing to do is:
d1 = b.sys_monoraw - a1.sys_monoraw;
d2 = a2.sys_monoraw - a1.sys_monoraw;
tsa = a1.device + ((a2.device - a1.device) * d1) / d2;
Which is maximaly precise under the assumption that in the time between
the sample points a1 and a2 neither the system clock nor the PCH clocks
are changing their frequency significantly. That is a valid assumption
when you put a reasonable upper bound on d2.
Even when the device does not implement getcrosststamp() then loop based
sampling like it is implemented in the PTP_SYS_OFFSET[_EXTENDED] IOTCLs
is providing reasonably accurate results to the extent possible.
Your algorithm is imprecise by definition and you can apply as much
testing as you want, it won't become magically correct. It's still a
guesstimate, i.e. an estimate made without using adequate or complete
information.
Now why a new syscall?
This can be done from user space with existing interfaces and the very
same precision today:
ioctl(fda, PTP_SYS_OFFSET*, &a1);
ioctl(fdb, PTP_SYS_OFFSET*, &b);
ioctl(fda, PTP_SYS_OFFSET*, &a2);
u64 d1 = timespec_delta_ns(b.sys_monoraw, a1.sys_monoraw);
u64 d2 = timespec_delta_ns(a2.sys_monoraw, a1.sys_monoraw);
u64 td = (timespec_delta_ns(a2.device, a1.device) * d1) / d2
tsa = timespec_add_ns(a1.device, td);
tsb = b.device;
with the extra benefit of:
1) The correct CLOCK_REALTIME at that sample point,
i.e. b.sys_realtime
2) The correct CLOCK_MONOTONIC_RAW at that sample point,
i.e. b.sys_monoraw
It works with PTP_SYS_OFFSET_PRECISE and PTP_SYS_OFFSET[_EXTENDED], with
the obvious limitations of PTP_SYS_OFFSET[_EXTENDED], which are still
vastly superior to your proposed (a2 - a1) / 2 guestimate which is just
reading the PCH clocks with clock_get_timespec().
It is completely independent of the load, the syscall overhead and the
actual time delta between the sample points when you apply a reasonable
upper bound for d2, i.e. the time delta between the sample points a1 and
a2 to eliminate the issue that system clock and/or the PCH clocks change
their frequency significantly during that time. You'd need to do that in
the kernel too.
The actual frequency difference between PCH A and system clock is
completely irrelevant when the frequencies of both are stable accross
the sample period.
You might still argue that the time delta between the sample points a1
and a2 matters and is slightly shorter in the kernel, but that is a
non-argument because:
1) The kernel implementation does not guarantee atomicity of the
consecutive samples either. The time delta is just statistically
better, which is obviously useless when you want to make
guarantees.
2) It does not matter when the time delta is slightly larger because
you need a large frequency change of the involved clocks in the
sample interval between the sample points a1 and a2 to make an
actual difference in the resulting accuracy.
A typical temperature drift of a high quality cyrstal is less than
1ppm per degree Celsius and even if you assume that the overall
system drift is 10ppm per degree Celsius then still the actual
error for a bound time delta between the sample points a1 and a2 is
just somewhere in the irrelevant noise, unless you manage to blow
torch or ice spray your crystals during the sample interval.
If your clocks are not stable enough then nothing can cure it and
you cannot do high precision timekeeping with them.
So what is your new syscall solving that can't be done with the existing
IOCTLs other than providing worse precision results based on
guesstimates and some handwavy future use for random clock ids?
Nothing as far as I can tell, but I might be missing something important
here.
Thanks,
tglx
---
arch/x86/kernel/tsc.c:119: "Math is hard, let's go shopping." - John Stultz
^ permalink raw reply
* Re: [PATCH v3] LSM: use 32 bit compatible data types in LSM syscalls.
From: Dmitry V. Levin @ 2024-03-14 18:01 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, LSM List, Linux kernel mailing list, linux-api,
Mickaël Salaün, James Morris, Serge Hallyn,
John Johansen, Tetsuo Handa, Stephen Smalley
In-Reply-To: <CAHC9VhQc-DEf=kSxbG-Mvz8jq-gxkaCe2jHb2a9LsJLQydj1zQ@mail.gmail.com>
On Thu, Mar 14, 2024 at 11:30:53AM -0400, Paul Moore wrote:
> On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> > >
> > > LSM: use 32 bit compatible data types in LSM syscalls.
> > >
> > > Change the size parameters in lsm_list_modules(), lsm_set_self_attr()
> > > and lsm_get_self_attr() from size_t to u32. This avoids the need to
> > > have different interfaces for 32 and 64 bit systems.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io>
> > > ---
> > > include/linux/lsm_hook_defs.h | 4 ++--
> > > include/linux/security.h | 8 ++++----
> > > security/apparmor/lsm.c | 4 ++--
> > > security/lsm_syscalls.c | 10 +++++-----
> > > security/security.c | 12 ++++++------
> > > security/selinux/hooks.c | 4 ++--
> > > security/smack/smack_lsm.c | 4 ++--
> > > tools/testing/selftests/lsm/common.h | 6 +++---
> > > tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 10 +++++-----
> > > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++----
> > > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++---
> > > 11 files changed, 38 insertions(+), 38 deletions(-)
> >
> > Okay, this looks better, I'm going to merge this into lsm/stable-6.9
> > and put it through the usual automated testing as well as a kselftest
> > run to make sure everything there is still okay. Assuming all goes
> > well and no one raises any objections, I'll likely send this up to
> > Linus tomorrow.
>
> I had to squash the code snippet below into the patch to address a
> build problem identified by the kernel build robot. I'm going to keep
> Casey's sign-off and Dmitry's reported-reviewed tag as I feel this
> change is minor, but if anyone has any objections please let me know
> soon.
>
> [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea]
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 77eb9b0e7685..e619ac10cd23 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd,
> struct cachestat __user *cstat, unsigned int flags);
> asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un
> signed int flags);
> asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> - size_t *size, __u32 flags);
> + u32 *size, u32 flags);
> asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> - size_t size, __u32 flags);
> -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags);
> + u32 size, u32 flags);
> +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
Fine with me, thanks.
btw, with the change above, u32 will become about twice more popular
in include/linux/syscalls.h than __u32.
--
ldv
^ permalink raw reply
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Mark Rutland @ 2024-03-14 15:59 UTC (permalink / raw)
To: Sagi Maimon
Cc: Thomas Gleixner, richardcochran, luto, datglx, mingo, bp,
dave.hansen, x86, hpa, arnd, geert, peterz, hannes, sohil.mehta,
rick.p.edgecombe, nphamcs, palmer, keescook, legion, mszeredi,
casey, reibax, davem, brauner, linux-kernel, linux-api,
linux-arch, netdev
In-Reply-To: <CAMuE1bH_H9E+Zx365G9AtmWSmhW-kPPB+-=8s2rH4hpxqE+dHQ@mail.gmail.com>
On Thu, Mar 14, 2024 at 02:19:39PM +0200, Sagi Maimon wrote:
> On Thu, Mar 14, 2024 at 1:12 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, Mar 14 2024 at 11:05, Sagi Maimon wrote:
> > > + if (crosstime_support_a) {
> > > + ktime_a = ktime_sub(xtstamp_a2.device, xtstamp_a1.device);
> > > + ts_offs_err = ktime_divns(ktime_a, 2);
> > > + ktime_a = ktime_add_ns(xtstamp_a1.device, (u64)ts_offs_err);
> > > + ts_a1 = ktime_to_timespec64(ktime_a);
> >
> > This is just wrong.
> >
> > read(a1);
> > read(b);
> > read(a2);
> >
> > You _CANNOT_ assume that (a1 + ((a2 - a1) / 2) is anywhere close to the
> > point in time where 'b' is read. This code is preemtible and
> > interruptible. I explained this to you before.
> >
> > Your explanation in the comment above the function is just wishful
> > thinking.
> >
> you explained it before, but still it is better then two consecutive
> user space calls which are also preemptible
> and the userspace to kernel context switch time is added.
How much "better" is that in reality?
The time for a user<->kernel transition should be trivial relative to the time
a task spends not running after having been preempted.
Either:
(a) Your userspace application can handle the arbitrary delta resulting from a
preemption, in which case the trivial cost shouldn't matter.
i.e. this patch *is not necessary* to solve your problem.
(b) Your userspace application cannot handle the arbitrary delta resulting from
a preemption, in which case you need to do something to handle that, which
you haven't described at all.
i.e. with the information you have provided so far, this patch is
*insufficient* to solve your problem.
> > > + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> > > + * average these times – to be as close as possible to the time we read clock_b.
> >
> > Can you please sit down and provide a precise technical description of
> > the problem you are trying to solve and explain your proposed solution
> > at the conceptual level instead of throwing out random implementations
> > every few days?
100% agreed.
Please, explain the actual problem you are solving here. What *specifically*
are you trying to do in userspace with these values? "Synchronization" is too
vague a description.
Making what is already the best case *marginally better* without handling the
common and worst cases is a waste of time. It doesn't actually solve the
problem, and it misleads people into thinknig that a problem is solved when it
is not.
Mark.
^ permalink raw reply
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Sagi Maimon @ 2024-03-14 15:46 UTC (permalink / raw)
To: Thomas Gleixner
Cc: richardcochran, luto, datglx, mingo, bp, dave.hansen, x86, hpa,
arnd, geert, peterz, hannes, sohil.mehta, rick.p.edgecombe,
nphamcs, palmer, keescook, legion, mark.rutland, mszeredi, casey,
reibax, davem, brauner, linux-kernel, linux-api, linux-arch,
netdev
In-Reply-To: <87a5n1m5j1.ffs@tglx>
On Thu, Mar 14, 2024 at 1:12 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Mar 14 2024 at 11:05, Sagi Maimon wrote:
> > Some user space applications need to read a couple of different clocks.
> > Each read requires moving from user space to kernel space.
> > Reading each clock separately (syscall) introduces extra
> > unpredictable/unmeasurable delay. Minimizing this delay contributes to user
> > space actions on these clocks (e.g. synchronization etc).
>
> I asked for a proper description of the actual problem several times now
> and you still provide some handwaving blurb. Feel free to ignore me, but
> then please don't be surprised if I ignore you too.
>
> Also why does reading two random clocks make any sense at all? Your code
> allows to read CLOCK_TAI and CLOCK_THREAD_CPUTIME_ID. What for?
>
> This is about PTP, no?
>
> Again you completely fail to explain why the existing PTP ioctls are not
> sufficient for the purpose and why you need to provide a new interface
> which is completely ill defined.
>
> > arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> > drivers/ptp/ptp_clock.c | 34 ++++--
> > include/linux/posix-clock.h | 2 +
> > include/linux/syscalls.h | 4 +
> > include/uapi/asm-generic/unistd.h | 5 +-
> > kernel/time/posix-clock.c | 25 +++++
> > kernel/time/posix-timers.c | 138 +++++++++++++++++++++++++
> > kernel/time/posix-timers.h | 2 +
>
> This needs to be split up into:
>
> 1) Infrastructure in posix-timers.c
> 2) Wire up the syscall in x86
> 3) Infrastructure in posix-clock.c
> 4) Usage in ptp_clock.c
>
> and not as a big lump of everything.
>
> > +/**
> > + * clock_compare - Get couple of clocks time stamps
>
> So the name of the syscall suggest that it compares two clocks, but the
> actual functionality is to read two clocks.
>
> This does not make any sense at all. Naming matters.
>
> > + * @clock_a: clock a ID
> > + * @clock_b: clock b ID
> > + * @tp_a: Pointer to a user space timespec64 for clock a storage
> > + * @tp_b: Pointer to a user space timespec64 for clock b storage
> > + *
> > + * clock_compare gets time sample of two clocks.
> > + * Supported clocks IDs: PHC, virtual PHC and various system clocks.
> > + *
> > + * In case of PHC that supports crosstimespec and the other clock is Monotonic raw
> > + * or system time, crosstimespec will be used to synchronously capture
> > + * system/device time stamp.
> > + *
> > + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> > + * average these times – to be as close as possible to the time we read clock_b.
> > + *
> > + * Returns:
> > + * 0 Success. @tp_a and @tp_b contains the time stamps
> > + * -EINVAL @clock a or b ID is not a valid clock ID
> > + * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
> > + * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
> > + **/
> > +SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const clockid_t, clock_b,
> > + struct __kernel_timespec __user *, tp_a, struct __kernel_timespec __user *,
> > + tp_b, s64 __user *, offs_err)
> > +{
> > + struct timespec64 ts_a, ts_a1, ts_b, ts_a2;
> > + struct system_device_crosststamp xtstamp_a1, xtstamp_a2, xtstamp_b;
> > + const struct k_clock *kc_a, *kc_b;
> > + ktime_t ktime_a;
> > + s64 ts_offs_err = 0;
> > + int error = 0;
> > + bool crosstime_support_a = false;
> > + bool crosstime_support_b = false;
>
> Please read and follow the documentation provided at:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
>
I have missed this part on prviews reply.
I have read the documentation above and I think that the variable
declarations at the beginning of a function is in reverse fir tree
order
meaning from big to small, but I guess that I am missing something,
can you please explain what is wrong with the variable declaration,
so I can fix it.
> > + kc_a = clockid_to_kclock(clock_a);
> > + if (!kc_a) {
> > + error = -EINVAL;
> > + return error;
>
> What's wrong about 'return -EINVAL;' ?
>
> > + }
> > +
> > + kc_b = clockid_to_kclock(clock_b);
> > + if (!kc_b) {
> > + error = -EINVAL;
> > + return error;
> > + }
> > +
> > + // In case crosstimespec supported and b clock is Monotonic raw or system
> > + // time, synchronously capture system/device time stamp
>
> Please don't use C++ comments.
>
> > + if (clock_a < 0) {
>
> This is just wrong. posix-clocks ar not the only ones which have a
> negative clock id. See clockid_to_kclock()
>
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
>
> What's a crosstimespec?
>
> This function fills in a system_device_crosststamp, so why not name it
> so that the purpose of the function is obvious?
>
> ptp_clock::info::getcrosststamp() has a reasonable name. So why do you
> need to come up with something which makes the code obfuscated?
>
> > + if (!error) {
> > + if (clock_b == CLOCK_MONOTONIC_RAW) {
> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_monoraw);
> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> > + goto out;
> > + } else if (clock_b == CLOCK_REALTIME) {
> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_realtime);
> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> > + goto out;
> > + } else {
> > + crosstime_support_a = true;
>
> Right. If clock_b is anything else than CLOCK_MONOTONIC_RAW or
> CLOCK_REALTIME then this is true.
>
> > + }
> > + }
>
> So in case of an error, this just keeps going with an uninitialized
> xtstamp_a1 and if the clock_b part succeeds it continues and operates on
> garbage.
>
> > + }
> > +
> > + // In case crosstimespec supported and a clock is Monotonic raw or system
> > + // time, synchronously capture system/device time stamp
> > + if (clock_b < 0) {
> > + // Synchronously capture system/device time stamp
> > + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> > + if (!error) {
> > + if (clock_a == CLOCK_MONOTONIC_RAW) {
> > + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_monoraw);
> > + ts_b = ktime_to_timespec64(xtstamp_b.device);
> > + goto out;
> > + } else if (clock_a == CLOCK_REALTIME) {
> > + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_realtime);
> > + ts_b = ktime_to_timespec64(xtstamp_b.device);
> > + goto out;
> > + } else {
> > + crosstime_support_b = true;
> > + }
> > + }
> > + }
> > +
> > + if (crosstime_support_a)
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
>
> What? crosstime_support_a is only true when the exactly same call
> returned success. Why does it need to be called here again?
>
> > + else
> > + error = kc_a->clock_get_timespec(clock_a, &ts_a1);
> > +
> > + if (error)
> > + return error;
> > +
> > + if (crosstime_support_b)
> > + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> > + else
> > + error = kc_b->clock_get_timespec(clock_b, &ts_b);
> > +
> > + if (error)
> > + return error;
> > +
> > + if (crosstime_support_a)
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a2);
> > + else
> > + error = kc_a->clock_get_timespec(clock_a, &ts_a2);
> > +
> > + if (error)
> > + return error;
>
> The logic and the code flow here are unreadable garbage and there are
> zero comments what this is supposed to do.
>
> > + if (crosstime_support_a) {
> > + ktime_a = ktime_sub(xtstamp_a2.device, xtstamp_a1.device);
> > + ts_offs_err = ktime_divns(ktime_a, 2);
> > + ktime_a = ktime_add_ns(xtstamp_a1.device, (u64)ts_offs_err);
> > + ts_a1 = ktime_to_timespec64(ktime_a);
>
> This is just wrong.
>
> read(a1);
> read(b);
> read(a2);
>
> You _CANNOT_ assume that (a1 + ((a2 - a1) / 2) is anywhere close to the
> point in time where 'b' is read. This code is preemtible and
> interruptible. I explained this to you before.
>
> Your explanation in the comment above the function is just wishful
> thinking.
>
> > + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> > + * average these times – to be as close as possible to the time we read clock_b.
>
> Can you please sit down and provide a precise technical description of
> the problem you are trying to solve and explain your proposed solution
> at the conceptual level instead of throwing out random implementations
> every few days?
>
> Thanks,
>
> tglx
>
>
^ permalink raw reply
* Re: [PATCH v3] LSM: use 32 bit compatible data types in LSM syscalls.
From: Casey Schaufler @ 2024-03-14 15:45 UTC (permalink / raw)
To: Paul Moore, Dmitry V. Levin, LSM List
Cc: Linux kernel mailing list, linux-api, Mickaël Salaün,
James Morris, Serge Hallyn, John Johansen, Tetsuo Handa,
Stephen Smalley, Casey Schaufler
In-Reply-To: <CAHC9VhQc-DEf=kSxbG-Mvz8jq-gxkaCe2jHb2a9LsJLQydj1zQ@mail.gmail.com>
On 3/14/2024 8:30 AM, Paul Moore wrote:
> On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
>> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> LSM: use 32 bit compatible data types in LSM syscalls.
>>>
>>> Change the size parameters in lsm_list_modules(), lsm_set_self_attr()
>>> and lsm_get_self_attr() from size_t to u32. This avoids the need to
>>> have different interfaces for 32 and 64 bit systems.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
>>> Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io>
>>> ---
>>> include/linux/lsm_hook_defs.h | 4 ++--
>>> include/linux/security.h | 8 ++++----
>>> security/apparmor/lsm.c | 4 ++--
>>> security/lsm_syscalls.c | 10 +++++-----
>>> security/security.c | 12 ++++++------
>>> security/selinux/hooks.c | 4 ++--
>>> security/smack/smack_lsm.c | 4 ++--
>>> tools/testing/selftests/lsm/common.h | 6 +++---
>>> tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 10 +++++-----
>>> tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++----
>>> tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++---
>>> 11 files changed, 38 insertions(+), 38 deletions(-)
>> Okay, this looks better, I'm going to merge this into lsm/stable-6.9
>> and put it through the usual automated testing as well as a kselftest
>> run to make sure everything there is still okay. Assuming all goes
>> well and no one raises any objections, I'll likely send this up to
>> Linus tomorrow.
> I had to squash the code snippet below into the patch to address a
> build problem identified by the kernel build robot. I'm going to keep
> Casey's sign-off and Dmitry's reported-reviewed tag as I feel this
> change is minor, but if anyone has any objections please let me know
> soon.
Looks fine to me. Thank you.
>
> [NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea]
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 77eb9b0e7685..e619ac10cd23 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd,
> struct cachestat __user *cstat, unsigned int flags);
> asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un
> signed int flags);
> asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> - size_t *size, __u32 flags);
> + u32 *size, u32 flags);
> asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> - size_t size, __u32 flags);
> -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags);
> + u32 size, u32 flags);
> +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
>
> /*
> * Architecture-specific system calls
>
^ permalink raw reply
* Re: [PATCH v3] LSM: use 32 bit compatible data types in LSM syscalls.
From: Paul Moore @ 2024-03-14 15:30 UTC (permalink / raw)
To: Casey Schaufler, Dmitry V. Levin, LSM List
Cc: Linux kernel mailing list, linux-api, Mickaël Salaün,
James Morris, Serge Hallyn, John Johansen, Tetsuo Handa,
Stephen Smalley
In-Reply-To: <ef972e0088964722adffc596d38b0463@paul-moore.com>
On Wed, Mar 13, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
> On Mar 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > LSM: use 32 bit compatible data types in LSM syscalls.
> >
> > Change the size parameters in lsm_list_modules(), lsm_set_self_attr()
> > and lsm_get_self_attr() from size_t to u32. This avoids the need to
> > have different interfaces for 32 and 64 bit systems.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: a04a1198088a: ("LSM: syscalls for current process attributes")
> > Fixes: ad4aff9ec25f: ("LSM: Create lsm_list_modules system call")
> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > Reported-and-reviewed-by: Dmitry V. Levin <ldv@strace.io>
> > ---
> > include/linux/lsm_hook_defs.h | 4 ++--
> > include/linux/security.h | 8 ++++----
> > security/apparmor/lsm.c | 4 ++--
> > security/lsm_syscalls.c | 10 +++++-----
> > security/security.c | 12 ++++++------
> > security/selinux/hooks.c | 4 ++--
> > security/smack/smack_lsm.c | 4 ++--
> > tools/testing/selftests/lsm/common.h | 6 +++---
> > tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 10 +++++-----
> > tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++----
> > tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++---
> > 11 files changed, 38 insertions(+), 38 deletions(-)
>
> Okay, this looks better, I'm going to merge this into lsm/stable-6.9
> and put it through the usual automated testing as well as a kselftest
> run to make sure everything there is still okay. Assuming all goes
> well and no one raises any objections, I'll likely send this up to
> Linus tomorrow.
I had to squash the code snippet below into the patch to address a
build problem identified by the kernel build robot. I'm going to keep
Casey's sign-off and Dmitry's reported-reviewed tag as I feel this
change is minor, but if anyone has any objections please let me know
soon.
[NOTE: cut-n-paste'd into email, likely whitespace damage, but you get the idea]
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 77eb9b0e7685..e619ac10cd23 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd,
struct cachestat __user *cstat, unsigned int flags);
asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, un
signed int flags);
asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
- size_t *size, __u32 flags);
+ u32 *size, u32 flags);
asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
- size_t size, __u32 flags);
-asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags);
+ u32 size, u32 flags);
+asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
/*
* Architecture-specific system calls
--
paul-moore.com
^ permalink raw reply related
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Sagi Maimon @ 2024-03-14 12:19 UTC (permalink / raw)
To: Thomas Gleixner
Cc: richardcochran, luto, datglx, mingo, bp, dave.hansen, x86, hpa,
arnd, geert, peterz, hannes, sohil.mehta, rick.p.edgecombe,
nphamcs, palmer, keescook, legion, mark.rutland, mszeredi, casey,
reibax, davem, brauner, linux-kernel, linux-api, linux-arch,
netdev
In-Reply-To: <87a5n1m5j1.ffs@tglx>
hi Thomas
On Thu, Mar 14, 2024 at 1:12 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Mar 14 2024 at 11:05, Sagi Maimon wrote:
> > Some user space applications need to read a couple of different clocks.
> > Each read requires moving from user space to kernel space.
> > Reading each clock separately (syscall) introduces extra
> > unpredictable/unmeasurable delay. Minimizing this delay contributes to user
> > space actions on these clocks (e.g. synchronization etc).
>
> I asked for a proper description of the actual problem several times now
> and you still provide some handwaving blurb. Feel free to ignore me, but
> then please don't be surprised if I ignore you too.
>
Nobody is ignoring your notes, and I address any notes given by any
maintainer in the most serious way.
As far as explaining the actual problem this is the best that I can,
but let me try to explain better:
We did many tests with different CPU loading and compared sampling the
same clock twice,
once in user space and once by using the system call.
We have noticed an improvement up to hundreds of nanoseconds while
using the system call.
Those results improved our ability to sync different PHCs
> Also why does reading two random clocks make any sense at all? Your code
> allows to read CLOCK_TAI and CLOCK_THREAD_CPUTIME_ID. What for?
>
Initially we needed to sync some different PHCs for our user space
application, that is why we came with this idea.
The first idea was an IOCTL that returned samples of several PHCs for
the need of synchronization.
Richard Cochran suggested a system call instead, which will add the
ability to get various system clocks, while this
implementation is more complex then IOCTL, I think that he was right,
for future usage.
> This is about PTP, no?
>
yes
> Again you completely fail to explain why the existing PTP ioctls are not
> sufficient for the purpose and why you need to provide a new interface
> which is completely ill defined.
>
The same as my answer above .....
> > arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> > drivers/ptp/ptp_clock.c | 34 ++++--
> > include/linux/posix-clock.h | 2 +
> > include/linux/syscalls.h | 4 +
> > include/uapi/asm-generic/unistd.h | 5 +-
> > kernel/time/posix-clock.c | 25 +++++
> > kernel/time/posix-timers.c | 138 +++++++++++++++++++++++++
> > kernel/time/posix-timers.h | 2 +
>
> This needs to be split up into:
>
> 1) Infrastructure in posix-timers.c
> 2) Wire up the syscall in x86
> 3) Infrastructure in posix-clock.c
> 4) Usage in ptp_clock.c
>
> and not as a big lump of everything.
>
I know, but I think the benefit worth it
I agree that an IOCTL won't require such a big changes in the kernel code
> > +/**
> > + * clock_compare - Get couple of clocks time stamps
>
> So the name of the syscall suggest that it compares two clocks, but the
> actual functionality is to read two clocks.
>
> This does not make any sense at all. Naming matters.
>
you are right the name was suggested by Richard when the main idea was
returning the offset between the clocks.
If you have a better name, please tell me
> > + * @clock_a: clock a ID
> > + * @clock_b: clock b ID
> > + * @tp_a: Pointer to a user space timespec64 for clock a storage
> > + * @tp_b: Pointer to a user space timespec64 for clock b storage
> > + *
> > + * clock_compare gets time sample of two clocks.
> > + * Supported clocks IDs: PHC, virtual PHC and various system clocks.
> > + *
> > + * In case of PHC that supports crosstimespec and the other clock is Monotonic raw
> > + * or system time, crosstimespec will be used to synchronously capture
> > + * system/device time stamp.
> > + *
> > + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> > + * average these times – to be as close as possible to the time we read clock_b.
> > + *
> > + * Returns:
> > + * 0 Success. @tp_a and @tp_b contains the time stamps
> > + * -EINVAL @clock a or b ID is not a valid clock ID
> > + * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
> > + * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
> > + **/
> > +SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const clockid_t, clock_b,
> > + struct __kernel_timespec __user *, tp_a, struct __kernel_timespec __user *,
> > + tp_b, s64 __user *, offs_err)
> > +{
> > + struct timespec64 ts_a, ts_a1, ts_b, ts_a2;
> > + struct system_device_crosststamp xtstamp_a1, xtstamp_a2, xtstamp_b;
> > + const struct k_clock *kc_a, *kc_b;
> > + ktime_t ktime_a;
> > + s64 ts_offs_err = 0;
> > + int error = 0;
> > + bool crosstime_support_a = false;
> > + bool crosstime_support_b = false;
>
> Please read and follow the documentation provided at:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
>
> > + kc_a = clockid_to_kclock(clock_a);
> > + if (!kc_a) {
> > + error = -EINVAL;
> > + return error;
>
> What's wrong about 'return -EINVAL;' ?
>
correct will be fixed on next patch
> > + }
> > +
> > + kc_b = clockid_to_kclock(clock_b);
> > + if (!kc_b) {
> > + error = -EINVAL;
> > + return error;
> > + }
> > +
> > + // In case crosstimespec supported and b clock is Monotonic raw or system
> > + // time, synchronously capture system/device time stamp
>
> Please don't use C++ comments.
>
will be fixed on next patch
> > + if (clock_a < 0) {
>
> This is just wrong. posix-clocks ar not the only ones which have a
> negative clock id. See clockid_to_kclock()
>
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
>
> What's a crosstimespec?
>
> This function fills in a system_device_crosststamp, so why not name it
> so that the purpose of the function is obvious?
>
correct , it will be fixed on next patch
> ptp_clock::info::getcrosststamp() has a reasonable name. So why do you
> need to come up with something which makes the code obfuscated?
>
correct , it will be fixed on next patch
> > + if (!error) {
> > + if (clock_b == CLOCK_MONOTONIC_RAW) {
> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_monoraw);
> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> > + goto out;
> > + } else if (clock_b == CLOCK_REALTIME) {
> > + ts_b = ktime_to_timespec64(xtstamp_a1.sys_realtime);
> > + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> > + goto out;
> > + } else {
> > + crosstime_support_a = true;
>
> Right. If clock_b is anything else than CLOCK_MONOTONIC_RAW or
> CLOCK_REALTIME then this is true.
>
> > + }
> > + }
>
> So in case of an error, this just keeps going with an uninitialized
> xtstamp_a1 and if the clock_b part succeeds it continues and operates on
> garbage.
>
On error xtstamp_a1 will be taken again using clock_get_crosstimespec
so no one will be operating on garbage.
Please explain the problem better, because I don't see it.
> > + }
> > +
> > + // In case crosstimespec supported and a clock is Monotonic raw or system
> > + // time, synchronously capture system/device time stamp
> > + if (clock_b < 0) {
> > + // Synchronously capture system/device time stamp
> > + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> > + if (!error) {
> > + if (clock_a == CLOCK_MONOTONIC_RAW) {
> > + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_monoraw);
> > + ts_b = ktime_to_timespec64(xtstamp_b.device);
> > + goto out;
> > + } else if (clock_a == CLOCK_REALTIME) {
> > + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_realtime);
> > + ts_b = ktime_to_timespec64(xtstamp_b.device);
> > + goto out;
> > + } else {
> > + crosstime_support_b = true;
> > + }
> > + }
> > + }
> > +
> > + if (crosstime_support_a)
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
>
> What? crosstime_support_a is only true when the exactly same call
> returned success. Why does it need to be called here again?
>
I wanted to take the three samples as close as possible to each other.
minimum code between the calls, that is why the call is done again.
> > + else
> > + error = kc_a->clock_get_timespec(clock_a, &ts_a1);
> > +
> > + if (error)
> > + return error;
> > +
> > + if (crosstime_support_b)
> > + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> > + else
> > + error = kc_b->clock_get_timespec(clock_b, &ts_b);
> > +
> > + if (error)
> > + return error;
> > +
> > + if (crosstime_support_a)
> > + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a2);
> > + else
> > + error = kc_a->clock_get_timespec(clock_a, &ts_a2);
> > +
> > + if (error)
> > + return error;
>
> The logic and the code flow here are unreadable garbage and there are
> zero comments what this is supposed to do.
>
I will add comments.
please no need to use negative words like "garbage" (not the first time),
please keep it professional and civilized.
> > + if (crosstime_support_a) {
> > + ktime_a = ktime_sub(xtstamp_a2.device, xtstamp_a1.device);
> > + ts_offs_err = ktime_divns(ktime_a, 2);
> > + ktime_a = ktime_add_ns(xtstamp_a1.device, (u64)ts_offs_err);
> > + ts_a1 = ktime_to_timespec64(ktime_a);
>
> This is just wrong.
>
> read(a1);
> read(b);
> read(a2);
>
> You _CANNOT_ assume that (a1 + ((a2 - a1) / 2) is anywhere close to the
> point in time where 'b' is read. This code is preemtible and
> interruptible. I explained this to you before.
>
> Your explanation in the comment above the function is just wishful
> thinking.
>
you explained it before, but still it is better then two consecutive
user space calls which are also preemptible
and the userspace to kernel context switch time is added.
> > + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> > + * average these times – to be as close as possible to the time we read clock_b.
>
> Can you please sit down and provide a precise technical description of
> the problem you are trying to solve and explain your proposed solution
> at the conceptual level instead of throwing out random implementations
> every few days?
>
> Thanks,
>
> tglx
>
>
^ permalink raw reply
* Re: [PATCH v7] posix-timers: add clock_compare system call
From: Thomas Gleixner @ 2024-03-14 11:12 UTC (permalink / raw)
To: Sagi Maimon, richardcochran, luto, datglx, mingo, bp, dave.hansen,
x86, hpa, arnd, geert, peterz, hannes, sohil.mehta,
rick.p.edgecombe, nphamcs, palmer, maimon.sagi, keescook, legion,
mark.rutland, mszeredi, casey, reibax, davem, brauner
Cc: linux-kernel, linux-api, linux-arch, netdev
In-Reply-To: <20240314090540.14091-1-maimon.sagi@gmail.com>
On Thu, Mar 14 2024 at 11:05, Sagi Maimon wrote:
> Some user space applications need to read a couple of different clocks.
> Each read requires moving from user space to kernel space.
> Reading each clock separately (syscall) introduces extra
> unpredictable/unmeasurable delay. Minimizing this delay contributes to user
> space actions on these clocks (e.g. synchronization etc).
I asked for a proper description of the actual problem several times now
and you still provide some handwaving blurb. Feel free to ignore me, but
then please don't be surprised if I ignore you too.
Also why does reading two random clocks make any sense at all? Your code
allows to read CLOCK_TAI and CLOCK_THREAD_CPUTIME_ID. What for?
This is about PTP, no?
Again you completely fail to explain why the existing PTP ioctls are not
sufficient for the purpose and why you need to provide a new interface
which is completely ill defined.
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> drivers/ptp/ptp_clock.c | 34 ++++--
> include/linux/posix-clock.h | 2 +
> include/linux/syscalls.h | 4 +
> include/uapi/asm-generic/unistd.h | 5 +-
> kernel/time/posix-clock.c | 25 +++++
> kernel/time/posix-timers.c | 138 +++++++++++++++++++++++++
> kernel/time/posix-timers.h | 2 +
This needs to be split up into:
1) Infrastructure in posix-timers.c
2) Wire up the syscall in x86
3) Infrastructure in posix-clock.c
4) Usage in ptp_clock.c
and not as a big lump of everything.
> +/**
> + * clock_compare - Get couple of clocks time stamps
So the name of the syscall suggest that it compares two clocks, but the
actual functionality is to read two clocks.
This does not make any sense at all. Naming matters.
> + * @clock_a: clock a ID
> + * @clock_b: clock b ID
> + * @tp_a: Pointer to a user space timespec64 for clock a storage
> + * @tp_b: Pointer to a user space timespec64 for clock b storage
> + *
> + * clock_compare gets time sample of two clocks.
> + * Supported clocks IDs: PHC, virtual PHC and various system clocks.
> + *
> + * In case of PHC that supports crosstimespec and the other clock is Monotonic raw
> + * or system time, crosstimespec will be used to synchronously capture
> + * system/device time stamp.
> + *
> + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> + * average these times – to be as close as possible to the time we read clock_b.
> + *
> + * Returns:
> + * 0 Success. @tp_a and @tp_b contains the time stamps
> + * -EINVAL @clock a or b ID is not a valid clock ID
> + * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
> + * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
> + **/
> +SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const clockid_t, clock_b,
> + struct __kernel_timespec __user *, tp_a, struct __kernel_timespec __user *,
> + tp_b, s64 __user *, offs_err)
> +{
> + struct timespec64 ts_a, ts_a1, ts_b, ts_a2;
> + struct system_device_crosststamp xtstamp_a1, xtstamp_a2, xtstamp_b;
> + const struct k_clock *kc_a, *kc_b;
> + ktime_t ktime_a;
> + s64 ts_offs_err = 0;
> + int error = 0;
> + bool crosstime_support_a = false;
> + bool crosstime_support_b = false;
Please read and follow the documentation provided at:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
> + kc_a = clockid_to_kclock(clock_a);
> + if (!kc_a) {
> + error = -EINVAL;
> + return error;
What's wrong about 'return -EINVAL;' ?
> + }
> +
> + kc_b = clockid_to_kclock(clock_b);
> + if (!kc_b) {
> + error = -EINVAL;
> + return error;
> + }
> +
> + // In case crosstimespec supported and b clock is Monotonic raw or system
> + // time, synchronously capture system/device time stamp
Please don't use C++ comments.
> + if (clock_a < 0) {
This is just wrong. posix-clocks ar not the only ones which have a
negative clock id. See clockid_to_kclock()
> + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
What's a crosstimespec?
This function fills in a system_device_crosststamp, so why not name it
so that the purpose of the function is obvious?
ptp_clock::info::getcrosststamp() has a reasonable name. So why do you
need to come up with something which makes the code obfuscated?
> + if (!error) {
> + if (clock_b == CLOCK_MONOTONIC_RAW) {
> + ts_b = ktime_to_timespec64(xtstamp_a1.sys_monoraw);
> + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> + goto out;
> + } else if (clock_b == CLOCK_REALTIME) {
> + ts_b = ktime_to_timespec64(xtstamp_a1.sys_realtime);
> + ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
> + goto out;
> + } else {
> + crosstime_support_a = true;
Right. If clock_b is anything else than CLOCK_MONOTONIC_RAW or
CLOCK_REALTIME then this is true.
> + }
> + }
So in case of an error, this just keeps going with an uninitialized
xtstamp_a1 and if the clock_b part succeeds it continues and operates on
garbage.
> + }
> +
> + // In case crosstimespec supported and a clock is Monotonic raw or system
> + // time, synchronously capture system/device time stamp
> + if (clock_b < 0) {
> + // Synchronously capture system/device time stamp
> + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> + if (!error) {
> + if (clock_a == CLOCK_MONOTONIC_RAW) {
> + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_monoraw);
> + ts_b = ktime_to_timespec64(xtstamp_b.device);
> + goto out;
> + } else if (clock_a == CLOCK_REALTIME) {
> + ts_a1 = ktime_to_timespec64(xtstamp_b.sys_realtime);
> + ts_b = ktime_to_timespec64(xtstamp_b.device);
> + goto out;
> + } else {
> + crosstime_support_b = true;
> + }
> + }
> + }
> +
> + if (crosstime_support_a)
> + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
What? crosstime_support_a is only true when the exactly same call
returned success. Why does it need to be called here again?
> + else
> + error = kc_a->clock_get_timespec(clock_a, &ts_a1);
> +
> + if (error)
> + return error;
> +
> + if (crosstime_support_b)
> + error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
> + else
> + error = kc_b->clock_get_timespec(clock_b, &ts_b);
> +
> + if (error)
> + return error;
> +
> + if (crosstime_support_a)
> + error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a2);
> + else
> + error = kc_a->clock_get_timespec(clock_a, &ts_a2);
> +
> + if (error)
> + return error;
The logic and the code flow here are unreadable garbage and there are
zero comments what this is supposed to do.
> + if (crosstime_support_a) {
> + ktime_a = ktime_sub(xtstamp_a2.device, xtstamp_a1.device);
> + ts_offs_err = ktime_divns(ktime_a, 2);
> + ktime_a = ktime_add_ns(xtstamp_a1.device, (u64)ts_offs_err);
> + ts_a1 = ktime_to_timespec64(ktime_a);
This is just wrong.
read(a1);
read(b);
read(a2);
You _CANNOT_ assume that (a1 + ((a2 - a1) / 2) is anywhere close to the
point in time where 'b' is read. This code is preemtible and
interruptible. I explained this to you before.
Your explanation in the comment above the function is just wishful
thinking.
> + * In other cases: Read clock_a twice (before, and after reading clock_b) and
> + * average these times – to be as close as possible to the time we read clock_b.
Can you please sit down and provide a precise technical description of
the problem you are trying to solve and explain your proposed solution
at the conceptual level instead of throwing out random implementations
every few days?
Thanks,
tglx
^ 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