Re: [linux-safety] [RFC PATCH 1/2] bust_spinlocks: add kernel-doc format doc

From: "Lukas Bulwahn" <lukas.bulwahn@gmail.com>
To: "Paoloni, Gabriele" <gabriele.paoloni@intel.com>
Cc: linux-safety@lists.elisa.tech
Subject: Re: [linux-safety] [RFC PATCH 1/2] bust_spinlocks: add kernel-doc format doc
Date: Wed, 14 Oct 2020 08:02:48 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2010140753210.6186@felia> (raw)
In-Reply-To: <20201013094938.356837-2-gabriele.paoloni@intel.com>

On Tue, 13 Oct 2020, Paoloni, Gabriele wrote:

> In the ELISA Linux Foundation project we are trying to
> improve the functions' documentation to make it more suitable
> to derive functions' specs and write unit tests. This is needed
> to make Linux more usable in functional safety systems.

This motivation is very personal but I think it is inappropriate for a 
commit message.

How about:

Explain the special purpose of bust_spinlocks().

> So I am adding a proper kernel-doc format for bust_spinlocks.
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@intel.com>
> ---
> With respect to this patch I have a question on how to set
> the function context; i.e. I don't know if it can be executed
> in any context or if it has limitations.
> ---
>  lib/bust_spinlocks.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/bust_spinlocks.c b/lib/bust_spinlocks.c
> index 8be59f84eaea..594b270161d9 100644
> --- a/lib/bust_spinlocks.c
> +++ b/lib/bust_spinlocks.c
> @@ -5,8 +5,6 @@
>   * Provides a minimal bust_spinlocks for architectures which don't
>   * have one of their own.
>   *
> - * bust_spinlocks() clears any spinlocks which would prevent oops, die(), BUG()
> - * and panic() information from reaching the user.

This description reads slightly nicer than the new one below.

>   */
>  
>  #include <linux/kernel.h>
> @@ -17,6 +15,15 @@
>  #include <linux/vt_kern.h>
>  #include <linux/console.h>
>  
> +/**
> + * bust_spinlocks - increases or decreases oops_in_progress.
> + * if oops_in_progress != 0 spinlocks which would prevent

Do not explain the implementation, explain the intent.

> + * oops, die(), BUG() and panic() information from reaching
> + * the user are busted.
> + * @yes: input flag; if zero decreases oops_in_progress,
> + * otherwise increases it.

I think the argument name 'yes' is terrible, and the documentation adds 
nothing to resolve the existing terror.

What is the semantics of this argument?

In which cases should I pass 0 as argument and which cases not?

If it is not possible to explain that here, let us not do it and then 
document other functions instead.

> + *
> + */
>  void bust_spinlocks(int yes)
>  {
>  	if (yes) {
> -- 
> 2.25.1
> 
> ---------------------------------------------------------------------
> INTEL CORPORATION ITALIA S.p.A. con unico socio
> Sede: Milanofiori Palazzo E 4 
> CAP 20094 Assago (MI)
> Capitale Sociale Euro 104.000,00 interamente versato
> Partita I.V.A. e Codice Fiscale  04236760155
> Repertorio Economico Amministrativo n. 997124 
> Registro delle Imprese di Milano nr. 183983/5281/33
> Soggetta ad attivita' di direzione e coordinamento di 
> INTEL CORPORATION, USA
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> 
> 
> 
> 
> 
> 