Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Quentin Monnet]

2018-04-30 09:12 UTC-0600 ~ David Ahern 
> On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index 530ff6588d8f..8daef7326bb7 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
>>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>>>   *
>>>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>>>   * Description
>>> - * Return a user or a kernel stack in bpf program provided buffer.
>>> - * To achieve this, the helper needs *ctx*, which is a pointer
>>> + * Return a user or a kernel stack in bpf program provided 
>>> buffer.
>>> + * To achieve this, the helper needs *ctx*, which is a 
>>> pointer
>> I still don't quite get the difference.
>> It's replacing 2 tabs in above with 1 space + 2 tabs ?

Yes, exactly (Plus in this case, the "::" a few line below has a missing
tab).

>> Can you please teach the python script to accept both?
>> I bet that will be recurring mistake and it's impossible to spot in code 
>> review.
> And checkpatch throws an error on the 1 space + 2 tabs so it gets
> confusing on which format should be used.

Sorry about that :/. I will send a patch to make the script more flexible.

Quentin

Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[David Ahern]

On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 530ff6588d8f..8daef7326bb7 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>>   *
>>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>>   *  Description
>> - *  Return a user or a kernel stack in bpf program provided buffer.
>> - *  To achieve this, the helper needs *ctx*, which is a pointer
>> + *  Return a user or a kernel stack in bpf program provided buffer.
>> + *  To achieve this, the helper needs *ctx*, which is a pointer
> 
> I still don't quite get the difference.
> It's replacing 2 tabs in above with 1 space + 2 tabs ?
> Can you please teach the python script to accept both?
> I bet that will be recurring mistake and it's impossible to spot in code 
> review.
> 

And checkpatch throws an error on the 1 space + 2 tabs so it gets
confusing on which format should be used.

Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Alexei Starovoitov]

On Mon, Apr 30, 2018 at 11:39:04AM +0100, Quentin Monnet wrote:
> Fix formatting (indent) for bpf_get_stack() helper documentation, so
> that the doc is rendered correctly with the Python script.
> 
> Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
> Cc: Yonghong Song 
> Signed-off-by: Quentin Monnet 
> ---
> 
> Note: The error was a missing space between the '*' marking the
> comments, and the tabs. This expected mixed indent comes from the fact I
> started to write the doc as a RST, then copied my contents (tabs
> included) in the header file and added a " * " (with a space) prefix
> everywhere.
> 
> On a second thought, using such indent style was maybe... not my best idea
> ever. Anyway, if indent for documenting eBPF helpers really gets to painful, 
> we
> could relax parsing rules in the Python script to make things easier.
> ---
>  include/uapi/linux/bpf.h | 54 
> 
>  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 530ff6588d8f..8daef7326bb7 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1770,33 +1770,33 @@ union bpf_attr {
>   *
>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>   *   Description
> - *   Return a user or a kernel stack in bpf program provided buffer.
> - *   To achieve this, the helper needs *ctx*, which is a pointer
> + *   Return a user or a kernel stack in bpf program provided buffer.
> + *   To achieve this, the helper needs *ctx*, which is a pointer

I still don't quite get the difference.
It's replacing 2 tabs in above with 1 space + 2 tabs ?
Can you please teach the python script to accept both?
I bet that will be recurring mistake and it's impossible to spot in code review.

[PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Quentin Monnet]

Fix formatting (indent) for bpf_get_stack() helper documentation, so
that the doc is rendered correctly with the Python script.

Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
Cc: Yonghong Song 
Signed-off-by: Quentin Monnet 
---

Note: The error was a missing space between the '*' marking the
comments, and the tabs. This expected mixed indent comes from the fact I
started to write the doc as a RST, then copied my contents (tabs
included) in the header file and added a " * " (with a space) prefix
everywhere.

On a second thought, using such indent style was maybe... not my best idea
ever. Anyway, if indent for documenting eBPF helpers really gets to painful, we
could relax parsing rules in the Python script to make things easier.
---
 include/uapi/linux/bpf.h | 54 
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 530ff6588d8f..8daef7326bb7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1770,33 +1770,33 @@ union bpf_attr {
  *
  * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
  * Description
- * Return a user or a kernel stack in bpf program provided buffer.
- * To achieve this, the helper needs *ctx*, which is a pointer
- * to the context on which the tracing program is executed.
- * To store the stacktrace, the bpf program provides *buf* with
- * a nonnegative *size*.
- *
- * The last argument, *flags*, holds the number of stack frames to
- * skip (from 0 to 255), masked with
- * **BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
- * the following flags:
- *
- * **BPF_F_USER_STACK**
- * Collect a user space stack instead of a kernel stack.
- * **BPF_F_USER_BUILD_ID**
- * Collect buildid+offset instead of ips for user stack,
- * only valid if **BPF_F_USER_STACK** is also specified.
- *
- * **bpf_get_stack**\ () can collect up to
- * **PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
- * to sufficient large buffer size. Note that
- * this limit can be controlled with the **sysctl** program, and
- * that it should be manually increased in order to profile long
- * user stacks (such as stacks for Java programs). To do so, use:
- *
- * ::
- *
- * # sysctl kernel.perf_event_max_stack=
+ * Return a user or a kernel stack in bpf program provided buffer.
+ * To achieve this, the helper needs *ctx*, which is a pointer
+ * to the context on which the tracing program is executed.
+ * To store the stacktrace, the bpf program provides *buf* with
+ * a nonnegative *size*.
+ *
+ * The last argument, *flags*, holds the number of stack frames to
+ * skip (from 0 to 255), masked with
+ * **BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
+ * the following flags:
+ *
+ * **BPF_F_USER_STACK**
+ * Collect a user space stack instead of a kernel stack.
+ * **BPF_F_USER_BUILD_ID**
+ * Collect buildid+offset instead of ips for user stack,
+ * only valid if **BPF_F_USER_STACK** is also specified.
+ *
+ * **bpf_get_stack**\ () can collect up to
+ * **PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
+ * to sufficient large buffer size. Note that
+ * this limit can be controlled with the **sysctl** program, and
+ * that it should be manually increased in order to profile long
+ * user stacks (such as stacks for Java programs). To do so, use:
+ *
+ * ::
+ *
+ * # sysctl kernel.perf_event_max_stack=
  *
  * Return
  * a non-negative value equal to or less than size on success, or
-- 
2.14.1