[PATCH] tracing: document buffer_size_kb more precisely

2023-09-25 Thread Christian Loehle
buffer_size_kb no longer shows the requested amount, but the one that
is actually used internally for the ring buffer.

commit 6d98a0f2ac3c ("tracing: Set actual size after ring buffer resize")
changed the sysfs behavior such that value read will always show the
actual size, while previously it showed the size that was requested
through the sysfs interface, even if it was rounded up to fulfill
the request.
So the documentation can state that more precisely now.

Signed-off-by: Christian Loehle 
---
 Documentation/trace/ftrace.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index 23572f6697c0..2e066b3b6edc 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -191,7 +191,7 @@ of ftrace. Here is a list of some of the key files:
A few extra pages may be allocated to accommodate buffer management
meta-data. If the last page allocated has room for more bytes
than requested, the rest of the page will be used,
-   making the actual allocation bigger than requested or shown.
+   making the actual allocation bigger than requested.
( Note, the size may not be a multiple of the page size
due to buffer management meta-data. )
 
-- 
2.34.1



Re: [PATCH] tracing: document buffer_size_kb more precisely

2023-09-25 Thread Zheng Yejian

On 2023/9/25 18:02, Christian Loehle wrote:

buffer_size_kb no longer shows the requested amount, but the one that
is actually used internally for the ring buffer.

commit 6d98a0f2ac3c ("tracing: Set actual size after ring buffer resize")
changed the sysfs behavior such that value read will always show the
actual size, while previously it showed the size that was requested
through the sysfs interface, even if it was rounded up to fulfill
the request.
So the documentation can state that more precisely now.

Signed-off-by: Christian Loehle 
---
  Documentation/trace/ftrace.rst | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index 23572f6697c0..2e066b3b6edc 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -191,7 +191,7 @@ of ftrace. Here is a list of some of the key files:
A few extra pages may be allocated to accommodate buffer management
meta-data. If the last page allocated has room for more bytes
than requested, the rest of the page will be used,
-   making the actual allocation bigger than requested or shown.
+   making the actual allocation bigger than requested.


Hi, the actual allocation should still be bigger than shown due to the
loss of accuracy when doing unit conversion from bytes to kilobytes (see
tracing_entries_read()).

--

Thanks,
Zheng Yejian


( Note, the size may not be a multiple of the page size
due to buffer management meta-data. )
  





Re: [PATCH v11 35/37] x86/syscall: Split IDT syscall setup code into idt_syscall_init()

2023-09-25 Thread H. Peter Anvin
On September 23, 2023 2:42:10 AM PDT, Xin Li  wrote:
>Because FRED uses the ring 3 FRED entrypoint for SYSCALL and SYSENTER and
>ERETU is the only legit instruction to return to ring 3, there is NO need
>to setup SYSCALL and SYSENTER MSRs for FRED, except the IA32_STAR MSR.
>
>Split IDT syscall setup code into idt_syscall_init() to make it easy to
>skip syscall setup code when FRED is enabled.
>
>Suggested-by: Thomas Gleixner 
>Tested-by: Shan Kang 
>Signed-off-by: Xin Li 
>---
> arch/x86/kernel/cpu/common.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>index 20bbedbf6dcb..2ee4e7b597a3 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -2071,10 +2071,8 @@ static void wrmsrl_cstar(unsigned long val)
>   wrmsrl(MSR_CSTAR, val);
> }
> 
>-/* May not be marked __init: used by software suspend */
>-void syscall_init(void)
>+static inline void idt_syscall_init(void)
> {
>-  wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
>   wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
> 
>   if (ia32_enabled()) {
>@@ -2108,6 +2106,15 @@ void syscall_init(void)
>  X86_EFLAGS_AC|X86_EFLAGS_ID);
> }
> 
>+/* May not be marked __init: used by software suspend */
>+void syscall_init(void)
>+{
>+  /* The default user and kernel segments */
>+  wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
>+
>+  idt_syscall_init();
>+}
>+
> #else /* CONFIG_X86_64 */
> 
> #ifdef CONFIG_STACKPROTECTOR

Am I missing something, or is this patch a noop?



RE: [PATCH v11 35/37] x86/syscall: Split IDT syscall setup code into idt_syscall_init()

2023-09-25 Thread Li, Xin3
> >diff --git a/arch/x86/kernel/cpu/common.c
> >b/arch/x86/kernel/cpu/common.c index 20bbedbf6dcb..2ee4e7b597a3 100644
> >--- a/arch/x86/kernel/cpu/common.c
> >+++ b/arch/x86/kernel/cpu/common.c
> >@@ -2071,10 +2071,8 @@ static void wrmsrl_cstar(unsigned long val)
> > wrmsrl(MSR_CSTAR, val);
> > }
> >
> >-/* May not be marked __init: used by software suspend */ -void
> >syscall_init(void)
> >+static inline void idt_syscall_init(void)
> > {
> >-wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
> > wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
> >
> > if (ia32_enabled()) {
> >@@ -2108,6 +2106,15 @@ void syscall_init(void)
> >X86_EFLAGS_AC|X86_EFLAGS_ID);
> > }
> >
> >+/* May not be marked __init: used by software suspend */ void
> >+syscall_init(void) {
> >+/* The default user and kernel segments */
> >+wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
> >+
> >+idt_syscall_init();
> >+}
> >+
> > #else   /* CONFIG_X86_64 */
> >
> > #ifdef CONFIG_STACKPROTECTOR
> 
> Am I missing something, or is this patch a noop?

Yes, this is a noop, just a cleanup patch w/o functionality change.



RE: [PATCH v11 35/37] x86/syscall: Split IDT syscall setup code into idt_syscall_init()

2023-09-25 Thread H. Peter Anvin
On September 25, 2023 10:56:44 AM PDT, "Li, Xin3"  wrote:
>> >diff --git a/arch/x86/kernel/cpu/common.c
>> >b/arch/x86/kernel/cpu/common.c index 20bbedbf6dcb..2ee4e7b597a3 100644
>> >--- a/arch/x86/kernel/cpu/common.c
>> >+++ b/arch/x86/kernel/cpu/common.c
>> >@@ -2071,10 +2071,8 @@ static void wrmsrl_cstar(unsigned long val)
>> >wrmsrl(MSR_CSTAR, val);
>> > }
>> >
>> >-/* May not be marked __init: used by software suspend */ -void
>> >syscall_init(void)
>> >+static inline void idt_syscall_init(void)
>> > {
>> >-   wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
>> >wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
>> >
>> >if (ia32_enabled()) {
>> >@@ -2108,6 +2106,15 @@ void syscall_init(void)
>> >   X86_EFLAGS_AC|X86_EFLAGS_ID);
>> > }
>> >
>> >+/* May not be marked __init: used by software suspend */ void
>> >+syscall_init(void) {
>> >+   /* The default user and kernel segments */
>> >+   wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
>> >+
>> >+   idt_syscall_init();
>> >+}
>> >+
>> > #else  /* CONFIG_X86_64 */
>> >
>> > #ifdef CONFIG_STACKPROTECTOR
>> 
>> Am I missing something, or is this patch a noop?
>
>Yes, this is a noop, just a cleanup patch w/o functionality change.
>
>

It just seems to be completely redundant. We can just drop it, no? If we aren't 
going to explicitly clobber the registers there is no harm in setting them up 
for IDT unconditionally.



RE: [PATCH v11 35/37] x86/syscall: Split IDT syscall setup code into idt_syscall_init()

2023-09-25 Thread Li, Xin3
> >Yes, this is a noop, just a cleanup patch w/o functionality change.
> >
> It just seems to be completely redundant. We can just drop it, no? If we 
> aren't going
> to explicitly clobber the registers there is no harm in setting them up for 
> IDT
> unconditionally.

If no objection, I can make the change, i.e., unconditionally set these
MSRs up for IDT.