Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jonathan Corbet <corbet@lwn.net>
To:     Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
        rostedt@goodmis.org
Cc:     mingo@redhat.com, linux-kernel@vger.kernel.org,
        Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
        Abaci Robot <abaci@linux.alibaba.com>
Subject: Re: [PATCH] tracing: Fix kernel-doc
In-Reply-To: <20220216075840.63755-1-jiapeng.chong@linux.alibaba.com>
References: <20220216075840.63755-1-jiapeng.chong@linux.alibaba.com>
Date:   Wed, 16 Feb 2022 09:04:34 -0700
Message-ID: <87r1826bn1.fsf@meer.lwn.net>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Jiapeng Chong <jiapeng.chong@linux.alibaba.com> writes:

> Fix the following W=1 kernel warnings:
>
> kernel/trace/trace.c:1181: warning: expecting prototype for
> tracing_snapshot_cond_data(). Prototype was for
> tracing_cond_snapshot_data() instead.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Thank you for working to fix docs warnings.  Unfortunately, this is not
the correct fix.

>  kernel/trace/trace.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 7c2578efde26..326d96304b84 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1122,7 +1122,7 @@ void tracing_snapshot_instance(struct trace_array *tr)
>  	tracing_snapshot_instance_cond(tr, NULL);
>  }
>  
> -/**
> +/*

Here you are removing kerneldoc markup for comments that are entirely
unrelated to the warning in question.  Why are you doing that?  That
will remove this documentation (for an exported function) from the docs
build. 

>   * tracing_snapshot - take a snapshot of the current buffer.
>   *
>   * This causes a swap between the snapshot buffer and the current live
> @@ -1144,7 +1144,7 @@ void tracing_snapshot(void)
>  }
>  EXPORT_SYMBOL_GPL(tracing_snapshot);
>  
> -/**
> +/*
>   * tracing_snapshot_cond - conditionally take a snapshot of the current buffer.

The same question applies here.

>   * @tr:		The tracing instance to snapshot
>   * @cond_data:	The data to be tested conditionally, and possibly saved
> @@ -1163,8 +1163,8 @@ void tracing_snapshot_cond(struct trace_array *tr, void *cond_data)
>  }
>  EXPORT_SYMBOL_GPL(tracing_snapshot_cond);
>  
> -/**
> - * tracing_snapshot_cond_data - get the user data associated with a snapshot
> +/*
> + * tracing_cond_snapshot_data - get the user data associated with a snapshot

Here you got the right fix - getting the name of the function right in
the comment - but you still took out the kerneldoc marker.

These changes have the effect of making the warning go away (and more),
but don't fix the real problem - that tracing_cond_snapshot_data() has
incorrect kerneldoc documentation.  Unless I'm missing something
fundamental (which happens often), the right fix is just the change of
function name in the comment.

Thanks,

jon