Re: [PATCH 2/3] docs: s390: unify and update s390dbf kdocs at debug.c

Christian Borntraeger
2019-07-05T10:54:29Z

On 03.07.19 12:19, Steffen Maier wrote:
> For non-static-inlines, debug.c already had non-compliant function
> header docs. So move the pure prototype kdocs of
> ("s390: include/asm/debug.h add kerneldoc markups")
> from debug.h to debug.c and merge them with the old function docs.
> Also, I had the impression that kdoc typically is at the implementation
> in the compile unit rather than at the prototype in the header file.
>
> While at it, update the short kdoc description to distinguish the
> different functions. And a few more consistency cleanups.
>
> Added a new kdoc for debug_set_critical() since debug.h comments it
> as part of the API.
>
> Signed-off-by: Steffen Maier <maier@linux.ibm.com>

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
> Documentation/s390/s390dbf.rst | 1 +
> arch/s390/include/asm/debug.h | 112 ++++++-----------------------------------
> arch/s390/kernel/debug.c | 105 +++++++++++++++++++++++++++++++-------
> 3 files changed, 102 insertions(+), 116 deletions(-)
>
> diff --git a/Documentation/s390/s390dbf.rst b/Documentation/s390/s390dbf.rst
> index 01d66251643d..be42892b159e 100644
> --- a/Documentation/s390/s390dbf.rst
> +++ b/Documentation/s390/s390dbf.rst
> @@ -107,6 +107,7 @@ will stay deactivated.
> Kernel Interfaces:
> ------------------
>
> +.. kernel-doc:: arch/s390/kernel/debug.c
> .. kernel-doc:: arch/s390/include/asm/debug.h
>
> Predefined views:
> diff --git a/arch/s390/include/asm/debug.h b/arch/s390/include/asm/debug.h
> index 02c36eedd780..310134015541 100644
> --- a/arch/s390/include/asm/debug.h
> +++ b/arch/s390/include/asm/debug.h
> @@ -95,77 +95,19 @@ debug_entry_t *debug_exception_common(debug_info_t *id, int level,
>
> /* Debug Feature API: */
>
> -/**
> - * debug_register() - allocates memory for a debug log.
> - *
> - * @name: Name of debug log (e.g. used for debugfs entry)
> - * @pages: Number of pages, which will be allocated per area
> - * @nr_areas: Number of debug areas
> - * @buf_size: Size of data area in each debug entry
> - *
> - * Return:
> - * - Handler for generated debug area
> - * - %NULL if register failed
> - *
> - * Must not be called within an interrupt handler.
> - */
> debug_info_t *debug_register(const char *name, int pages, int nr_areas,
> int buf_size);
>
> -/**
> - * debug_register_mode() - allocates memory for a debug log.
> - *
> - * @name: Name of debug log (e.g. used for debugfs entry)
> - * @pages: Number of pages, which will be allocated per area
> - * @nr_areas: Number of debug areas
> - * @buf_size: Size of data area in each debug entry
> - * @mode: File mode for debugfs files. E.g. S_IRWXUGO
> - * @uid: User ID for debugfs files. Currently only 0 is supported.
> - * @gid: Group ID for debugfs files. Currently only 0 is supported.
> - *
> - * Return:
> - * - Handler for generated debug area
> - * - %NULL if register failed
> - *
> - * Must not be called within an interrupt handler
> - */
> debug_info_t *debug_register_mode(const char *name, int pages, int nr_areas,
> int buf_size, umode_t mode, uid_t uid,
> gid_t gid);
>
> -/**
> - * debug_unregister() - frees memory for a debug log and removes all
> - * registered debug
> - * views.
> - *
> - * @id: handle for debug log
> - *
> - * Return:
> - * none
> - *
> - * Must not be called within an interrupt handler
> - */
> void debug_unregister(debug_info_t *id);
>
> -/**
> - * debug_set_level() - Sets new actual debug level if new_level is valid.
> - *
> - * @id: handle for debug log
> - * @new_level: new debug level
> - *
> - * Return:
> - * none
> - */
> void debug_set_level(debug_info_t *id, int new_level);
>
> void debug_set_critical(void);
>
> -/**
> - * debug_stop_all() - stops the debug feature if stopping is allowed.
> - *
> - * Return:
> - * - none
> - */
> void debug_stop_all(void);
>
> /**
> @@ -184,7 +126,7 @@ static inline bool debug_level_enabled(debug_info_t *id, int level)
> }
>
> /**
> - * debug_event() - writes debug entry to active debug area
> + * debug_event() - writes binary debug entry to active debug area
> * (if level <= actual debug level)
> *
> * @id: handle for debug log
> @@ -194,6 +136,7 @@ static inline bool debug_level_enabled(debug_info_t *id, int level)
> *
> * Return:
> * - Address of written debug entry
> + * - %NULL if error
> */
> static inline debug_entry_t *debug_event(debug_info_t *id, int level,
> void *data, int length)
> @@ -204,7 +147,7 @@ static inline debug_entry_t *debug_event(debug_info_t *id, int level,
> }
>
> /**
> - * debug_int_event() - writes debug entry to active debug area
> + * debug_int_event() - writes unsigned integer debug entry to active debug area
> * (if level <= actual debug level)
> *
> * @id: handle for debug log
> @@ -226,12 +169,12 @@ static inline debug_entry_t *debug_int_event(debug_info_t *id, int level,
> }
>
> /**
> - * debug_long_event() - writes debug entry to active debug area
> + * debug_long_event() - writes unsigned long debug entry to active debug area
> * (if level <= actual debug level)
> *
> * @id: handle for debug log
> * @level: debug level
> - * @tag: integer value for debug entry
> + * @tag: long integer value for debug entry
> *
> * Return:
> * - Address of written debug entry
> @@ -248,7 +191,7 @@ static inline debug_entry_t *debug_long_event(debug_info_t *id, int level,
> }
>
> /**
> - * debug_text_event() - writes debug entry in ascii format to active
> + * debug_text_event() - writes string debug entry in ascii format to active
> * debug area (if level <= actual debug level)
> *
> * @id: handle for debug log
> @@ -306,9 +249,9 @@ static inline debug_entry_t *debug_text_event(debug_info_t *id, int level,
> })
>
> /**
> - * debug_exception() - writes debug entry to active debug area
> - * (if level <= actual debug level) and switches
> - * to next debug area
> + * debug_exception() - writes binary debug entry to active debug area
> + * (if level <= actual debug level)
> + * and switches to next debug area
> *
> * @id: handle for debug log
> * @level: debug level
> @@ -328,7 +271,7 @@ static inline debug_entry_t *debug_exception(debug_info_t *id, int level,
> }
>
> /**
> - * debug_int_exception() - writes debug entry to active debug area
> + * debug_int_exception() - writes unsigned int debug entry to active debug area
> * (if level <= actual debug level)
> * and switches to next debug area
> *
> @@ -351,13 +294,13 @@ static inline debug_entry_t *debug_int_exception(debug_info_t *id, int level,
> }
>
> /**
> - * debug_long_exception() - writes debug entry to active debug area
> + * debug_long_exception() - writes long debug entry to active debug area
> * (if level <= actual debug level)
> * and switches to next debug area
> *
> * @id: handle for debug log
> * @level: debug level
> - * @tag: integer value for debug entry
> + * @tag: long integer value for debug entry
> *
> * Return:
> * - Address of written debug entry
> @@ -374,9 +317,9 @@ static inline debug_entry_t *debug_long_exception (debug_info_t *id, int level,
> }
>
> /**
> - * debug_text_exception() - writes debug entry in ascii format to active
> + * debug_text_exception() - writes string debug entry in ascii format to active
> * debug area (if level <= actual debug level)
> - * and switches to next debug
> + * and switches to next debug area
> * area
> *
> * @id: handle for debug log
> @@ -407,7 +350,7 @@ static inline debug_entry_t *debug_text_exception(debug_info_t *id, int level,
> /**
> * debug_sprintf_exception() - writes debug entry with format string and
> * varargs (longs) to active debug area
> - * (if level $<=$ actual debug level)
> + * (if level <= actual debug level)
> * and switches to next debug area.
> *
> * @_id: handle for debug log
> @@ -435,33 +378,8 @@ static inline debug_entry_t *debug_text_exception(debug_info_t *id, int level,
> __ret; \
> })
>
> -/**
> - * debug_register_view() - registers new debug view and creates debugfs
> - * dir entry
> - *
> - * @id: handle for debug log
> - * @view: pointer to debug view struct
> - *
> - * Return:
> - * - 0 : ok
> - * - < 0: Error
> - */
> int debug_register_view(debug_info_t *id, struct debug_view *view);
>
> -/**
> - * debug_unregister_view()
> - *
> - * @id: handle for debug log
> - * @view: pointer to debug view struct
> - *
> - * Return:
> - * - 0 : ok
> - * - < 0: Error
> - *
> - *
> - * unregisters debug view and removes debugfs dir entry
> - */
> -
> int debug_unregister_view(debug_info_t *id, struct debug_view *view);
>
> /*
> diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
> index 0ebf08c3b35e..70a44bad625f 100644
> --- a/arch/s390/kernel/debug.c
> +++ b/arch/s390/kernel/debug.c
> @@ -647,11 +647,23 @@ static int debug_close(struct inode *inode, struct file *file)
> return 0; /* success */
> }
>
> -/*
> - * debug_register_mode:
> - * - Creates and initializes debug area for the caller
> - * The mode parameter allows to specify access rights for the s390dbf files
> - * - Returns handle for debug area
> +/**
> + * debug_register_mode() - creates and initializes debug area.
> + *
> + * @name: Name of debug log (e.g. used for debugfs entry)
> + * @pages_per_area: Number of pages, which will be allocated per area
> + * @nr_areas: Number of debug areas
> + * @buf_size: Size of data area in each debug entry
> + * @mode: File mode for debugfs files. E.g. S_IRWXUGO
> + * @uid: User ID for debugfs files. Currently only 0 is supported.
> + * @gid: Group ID for debugfs files. Currently only 0 is supported.
> + *
> + * Return:
> + * - Handle for generated debug area
> + * - %NULL if register failed
> + *
> + * Allocates memory for a debug log.
> + * Must not be called within an interrupt handler.
> */
> debug_info_t *debug_register_mode(const char *name, int pages_per_area,
> int nr_areas, int buf_size, umode_t mode,
> @@ -681,10 +693,21 @@ debug_info_t *debug_register_mode(const char *name, int pages_per_area,
> }
> EXPORT_SYMBOL(debug_register_mode);
>
> -/*
> - * debug_register:
> - * - creates and initializes debug area for the caller
> - * - returns handle for debug area
> +/**
> + * debug_register() - creates and initializes debug area with default file mode.
> + *
> + * @name: Name of debug log (e.g. used for debugfs entry)
> + * @pages_per_area: Number of pages, which will be allocated per area
> + * @nr_areas: Number of debug areas
> + * @buf_size: Size of data area in each debug entry
> + *
> + * Return:
> + * - Handle for generated debug area
> + * - %NULL if register failed
> + *
> + * Allocates memory for a debug log.
> + * The debugfs file mode access permisions are read and write for user.
> + * Must not be called within an interrupt handler.
> */
> debug_info_t *debug_register(const char *name, int pages_per_area,
> int nr_areas, int buf_size)
> @@ -694,9 +717,13 @@ debug_info_t *debug_register(const char *name, int pages_per_area,
> }
> EXPORT_SYMBOL(debug_register);
>
> -/*
> - * debug_unregister:
> - * - give back debug area
> +/**
> + * debug_unregister() - give back debug area.
> + *
> + * @id: handle for debug log
> + *
> + * Return:
> + * none
> */
> void debug_unregister(debug_info_t *id)
> {
> @@ -745,9 +772,14 @@ static int debug_set_size(debug_info_t *id, int nr_areas, int pages_per_area)
> return rc;
> }
>
> -/*
> - * debug_set_level:
> - * - set actual debug level
> +/**
> + * debug_set_level() - Sets new actual debug level if new_level is valid.
> + *
> + * @id: handle for debug log
> + * @new_level: new debug level
> + *
> + * Return:
> + * none
> */
> void debug_set_level(debug_info_t *id, int new_level)
> {
> @@ -873,6 +905,14 @@ static int s390dbf_procactive(struct ctl_table *table, int write,
>
> static struct ctl_table_header *s390dbf_sysctl_header;
>
> +/**
> + * debug_stop_all() - stops the debug feature if stopping is allowed.
> + *
> + * Return:
> + * - none
> + *
> + * Currently used in case of a kernel oops.
> + */
> void debug_stop_all(void)
> {
> if (debug_stoppable)
> @@ -880,6 +920,17 @@ void debug_stop_all(void)
> }
> EXPORT_SYMBOL(debug_stop_all);
>
> +/**
> + * debug_set_critical() - event/exception functions try lock instead of spin.
> + *
> + * Return:
> + * - none
> + *
> + * Currently used in case of stopping all CPUs but the current one.
> + * Once in this state, functions to write a debug entry for an
> + * event or exception no longer spin on the debug area lock,
> + * but only try to get it and fail if they do not get the lock.
> + */
> void debug_set_critical(void)
> {
> debug_critical = 1;
> @@ -1036,8 +1087,16 @@ debug_entry_t *__debug_sprintf_exception(debug_info_t *id, int level, char *stri
> }
> EXPORT_SYMBOL(__debug_sprintf_exception);
>
> -/*
> - * debug_register_view:
> +/**
> + * debug_register_view() - registers new debug view and creates debugfs
> + * dir entry
> + *
> + * @id: handle for debug log
> + * @view: pointer to debug view struct
> + *
> + * Return:
> + * - 0 : ok
> + * - < 0: Error
> */
> int debug_register_view(debug_info_t *id, struct debug_view *view)
> {
> @@ -1077,8 +1136,16 @@ int debug_register_view(debug_info_t *id, struct debug_view *view)
> }
> EXPORT_SYMBOL(debug_register_view);
>
> -/*
> - * debug_unregister_view:
> +/**
> + * debug_unregister_view() - unregisters debug view and removes debugfs
> + * dir entry
> + *
> + * @id: handle for debug log
> + * @view: pointer to debug view struct
> + *
> + * Return:
> + * - 0 : ok
> + * - < 0: Error
> */
> int debug_unregister_view(debug_info_t *id, struct debug_view *view)
> {
>