[PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[Jordan Niethe]

The hardware trace macros which use the memory provided by memtrace are
able to use trace sizes as small as 16MB. Only memblock aligned values
can be removed from each NUMA node by writing that value to
memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
not possible.  To allow such a trace size, instead align whatever value
is written to memtrace/enable to the memblock size for the purpose of
removing it from each NUMA node but report the written value from
memtrace/enable and memtrace/x/size in debugfs.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/platforms/powernv/memtrace.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 6828108486f8..1188bc8fd090 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -191,7 +191,7 @@ static int memtrace_init_debugfs(void)
 		ent->dir = dir;
 		debugfs_create_file("trace", 0400, dir, ent, &memtrace_fops);
 		debugfs_create_x64("start", 0400, dir, &ent->start);
-		debugfs_create_x64("size", 0400, dir, &ent->size);
+		debugfs_create_x64("size", 0400, dir, &memtrace_size);
 	}
 
 	return ret;
@@ -259,33 +259,25 @@ static int memtrace_enable_set(void *data, u64 val)
 {
 	u64 bytes;
 
-	/*
-	 * Don't attempt to do anything if size isn't aligned to a memory
-	 * block or equal to zero.
-	 */
-	bytes = memory_block_size_bytes();
-	if (val & (bytes - 1)) {
-		pr_err("Value must be aligned with 0x%llx\n", bytes);
-		return -EINVAL;
-	}
-
 	/* Re-add/online previously removed/offlined memory */
 	if (memtrace_size) {
 		if (memtrace_online())
 			return -EAGAIN;
 	}
 
+	memtrace_size = val;
+
 	if (!val)
 		return 0;
 
-	/* Offline and remove memory */
-	if (memtrace_init_regions_runtime(val))
+	/* Offline and remove memory aligned to memory blocks */
+	bytes = memory_block_size_bytes();
+	if (memtrace_init_regions_runtime(ALIGN(val, bytes)))
 		return -EINVAL;
 
 	if (memtrace_init_debugfs())
 		return -EINVAL;
 
-	memtrace_size = val;
 
 	return 0;
 }
-- 
2.17.1

Re: [PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[Michael Neuling]

CC Rashmica Gupta <rashmicy@gmail.com>

On Wed, 2020-11-11 at 16:55 +1100, Jordan Niethe wrote:
> The hardware trace macros which use the memory provided by memtrace are
> able to use trace sizes as small as 16MB. Only memblock aligned values
> can be removed from each NUMA node by writing that value to
> memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
> not possible.  To allow such a trace size, instead align whatever value
> is written to memtrace/enable to the memblock size for the purpose of
> removing it from each NUMA node but report the written value from
> memtrace/enable and memtrace/x/size in debugfs.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
>  arch/powerpc/platforms/powernv/memtrace.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c
> b/arch/powerpc/platforms/powernv/memtrace.c
> index 6828108486f8..1188bc8fd090 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -191,7 +191,7 @@ static int memtrace_init_debugfs(void)
>  		ent->dir = dir;
>  		debugfs_create_file("trace", 0400, dir, ent, &memtrace_fops);
>  		debugfs_create_x64("start", 0400, dir, &ent->start);
> -		debugfs_create_x64("size", 0400, dir, &ent->size);
> +		debugfs_create_x64("size", 0400, dir, &memtrace_size);
>  	}
>  
>  	return ret;
> @@ -259,33 +259,25 @@ static int memtrace_enable_set(void *data, u64 val)
>  {
>  	u64 bytes;
>  
> -	/*
> -	 * Don't attempt to do anything if size isn't aligned to a memory
> -	 * block or equal to zero.
> -	 */
> -	bytes = memory_block_size_bytes();
> -	if (val & (bytes - 1)) {
> -		pr_err("Value must be aligned with 0x%llx\n", bytes);
> -		return -EINVAL;
> -	}
> -
>  	/* Re-add/online previously removed/offlined memory */
>  	if (memtrace_size) {
>  		if (memtrace_online())
>  			return -EAGAIN;
>  	}
>  
> +	memtrace_size = val;
> +
>  	if (!val)
>  		return 0;
>  
> -	/* Offline and remove memory */
> -	if (memtrace_init_regions_runtime(val))
> +	/* Offline and remove memory aligned to memory blocks */
> +	bytes = memory_block_size_bytes();
> +	if (memtrace_init_regions_runtime(ALIGN(val, bytes)))
>  		return -EINVAL;
>  
>  	if (memtrace_init_debugfs())
>  		return -EINVAL;
>  
> -	memtrace_size = val;
>  
>  	return 0;
>  }

Re: [PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[Michael Ellerman]

Jordan Niethe <jniethe5@gmail.com> writes:
> The hardware trace macros which use the memory provided by memtrace are
> able to use trace sizes as small as 16MB. Only memblock aligned values
> can be removed from each NUMA node by writing that value to
> memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
> not possible.  To allow such a trace size, instead align whatever value
> is written to memtrace/enable to the memblock size for the purpose of
> removing it from each NUMA node but report the written value from
> memtrace/enable and memtrace/x/size in debugfs.

Why does it matter if the size that's removed is larger than the size
that was requested?

Is it about constraining the size of the trace? If so that seems like it
should be the job of the tracing tools, not the kernel.

cheers

Re: [PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[Jordan Niethe]

On Mon, Nov 16, 2020 at 11:02 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > The hardware trace macros which use the memory provided by memtrace are
> > able to use trace sizes as small as 16MB. Only memblock aligned values
> > can be removed from each NUMA node by writing that value to
> > memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
> > not possible.  To allow such a trace size, instead align whatever value
> > is written to memtrace/enable to the memblock size for the purpose of
> > removing it from each NUMA node but report the written value from
> > memtrace/enable and memtrace/x/size in debugfs.
>
> Why does it matter if the size that's removed is larger than the size
> that was requested?
>
> Is it about constraining the size of the trace? If so that seems like it
> should be the job of the tracing tools, not the kernel.
Yeah about constraining the size, I'll just do it in the trace tools.
>
> cheers

Re: [PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces

[Michael Ellerman]

Jordan Niethe <jniethe5@gmail.com> writes:
> On Mon, Nov 16, 2020 at 11:02 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Jordan Niethe <jniethe5@gmail.com> writes:
>> > The hardware trace macros which use the memory provided by memtrace are
>> > able to use trace sizes as small as 16MB. Only memblock aligned values
>> > can be removed from each NUMA node by writing that value to
>> > memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
>> > not possible.  To allow such a trace size, instead align whatever value
>> > is written to memtrace/enable to the memblock size for the purpose of
>> > removing it from each NUMA node but report the written value from
>> > memtrace/enable and memtrace/x/size in debugfs.
>>
>> Why does it matter if the size that's removed is larger than the size
>> that was requested?
>>
>> Is it about constraining the size of the trace? If so that seems like it
>> should be the job of the tracing tools, not the kernel.
>
> Yeah about constraining the size, I'll just do it in the trace tools.

OK, I think that would be better. Thanks.

cheers