Message-ID: <b6f467c5-4a70-46a2-8497-1c351b3bfea9@yahoo.com>
Date: Fri, 7 Nov 2025 20:58:47 +0100
From: Sunday Adelodun <adelodunolaoluwa@...oo.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: lenb@...nel.org, pavel@...nel.org, anna-maria@...utronix.de,
 frederic@...nel.org, mingo@...nel.org, tglx@...utronix.de,
 linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] power/swap: add missing params and Return:
 descriptions to kernel-doc comments

On 11/7/25 16:36, Rafael J. Wysocki wrote:
> On Thu, Nov 6, 2025 at 12:40 PM Sunday Adelodun
> <adelodunolaoluwa@...oo.com> wrote:
>> Kernel-doc checks (scripts/kernel-doc) reported a number of warnings
>> for missing parameters and `Return:` descriptions in kernel/power/swap.c.
>> These missing return descriptions make the generated documentation
>> noisy and break doc-build when -Werror is used.
>>
>> Update the kernel-doc comment blocks to add explicit
>> Return: lines (and a few parameter tags where helpful) for the functions
>> that were triggering warnings. No functional code changes are made.
>>
>> Example warnings that motivated this change:
>>   - Warning: kernel/power/swap.c:535 No description found for return value
>>     of 'save_image'
>>   - Warning: kernel/power/swap.c:687 No description found for return value
>>    of 'save_compressed_image'
>>   - Warning: kernel/power/swap.c:941 No description found for return value
>>     of 'swsusp_write'
>>
>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@...oo.com>
> These comments need more changes to become proper kerneldocs and in
> some cases it is not even necessary because the functions in question
> are static.
>
> If the goal is to avoid warnings, why don't you change them all to
> non-kerneldoc regular comments?

Thank you very much for the feedback.

For the functions that are not static, could you please suggest the 
appropriate changes

needed to make their comments proper? I would like to improve them 
accordingly.

My initial goal was to address the warnings, but if converting them to 
regular comments

is the preferred approach, I will go ahead and update them that way in v2.

Thanks again for your guidance.

Best,
Sunday Adelodun

>
>> ---
>>   kernel/power/swap.c | 35 +++++++++++++++++++++++++++++++----
>>   1 file changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
>> index 0beff7eeaaba..3f0df7a26bc3 100644
>> --- a/kernel/power/swap.c
>> +++ b/kernel/power/swap.c
>> @@ -341,6 +341,8 @@ unsigned int swsusp_header_flags;
>>    *     and get its index (if so)
>>    *
>>    *     This is called before saving image
>> + *
>> + *     Return: 0 on success, negative error code on failure.
>>    */
>>   static int swsusp_swap_check(void)
>>   {
>> @@ -367,6 +369,8 @@ static int swsusp_swap_check(void)
>>    *     @buf:           Address we're writing.
>>    *     @offset:        Offset of the swap page we're writing to.
>>    *     @hb:            bio completion batch
>> + *
>> + *     Return: 0 on success, negative error code on failure.
>>    */
>>
>>   static int write_page(void *buf, sector_t offset, struct hib_bio_batch *hb)
>> @@ -528,6 +532,11 @@ static int swap_writer_finish(struct swap_map_handle *handle,
>>
>>   /**
>>    *     save_image - save the suspend image data
>> + *     @handle: swap map handle for writing the image
>> + *     @snapshot: snapshot handle to read pages from
>> + *     @nr_to_write: number of pages to write
>> + *
>> + *     Return: 0 on success, negative error on failure
>>    */
>>
>>   static int save_image(struct swap_map_handle *handle,
>> @@ -676,6 +685,8 @@ static int compress_threadfn(void *data)
>>    * @handle: Swap map handle to use for saving the image.
>>    * @snapshot: Image to read data from.
>>    * @nr_to_write: Number of pages to save.
>> + *
>> + * Return: 0 on success, negative error code on failure.
>>    */
>>   static int save_compressed_image(struct swap_map_handle *handle,
>>                                   struct snapshot_handle *snapshot,
>> @@ -906,8 +917,9 @@ static int save_compressed_image(struct swap_map_handle *handle,
>>
>>   /**
>>    *     enough_swap - Make sure we have enough swap to save the image.
>> + *     @nr_pages: number of pages we need to save
>>    *
>> - *     Returns TRUE or FALSE after checking the total amount of swap
>> + *     Return: TRUE or FALSE after checking the total amount of swap
>>    *     space available from the resume partition.
>>    */
>>
>> @@ -930,6 +942,8 @@ static int enough_swap(unsigned int nr_pages)
>>    *     them synced (in case something goes wrong) but we DO not want to mark
>>    *     filesystem clean: it is not. (And it does not matter, if we resume
>>    *     correctly, we'll mark system clean, anyway.)
>> + *
>> + *     Return: 0 on success, negative error code on failure.
>>    */
>>
>>   int swsusp_write(unsigned int flags)
>> @@ -1078,9 +1092,12 @@ static int swap_reader_finish(struct swap_map_handle *handle)
>>   }
>>
>>   /**
>> - *     load_image - load the image using the swap map handle
>> - *     @handle and the snapshot handle @snapshot
>> - *     (assume there are @nr_pages pages to load)
>> + *     load_image - load the image using the swap map and snapshot handles
>> + *     @handle: pointer to the swap map used for reading swap pages
>> + *     @snapshot: pointer to the snapshot handle that tracks image pages
>> + *     @nr_to_read: number of pages to read from the snapshot
>> + *
>> + *     Return: 0 on success, negative error code on failure.
>>    */
>>
>>   static int load_image(struct swap_map_handle *handle,
>> @@ -1195,6 +1212,8 @@ static int decompress_threadfn(void *data)
>>    * @handle: Swap map handle to use for loading data.
>>    * @snapshot: Image to copy uncompressed data into.
>>    * @nr_to_read: Number of pages to load.
>> + *
>> + * Return: 0 on success, negative error code on failure.
>>    */
>>   static int load_compressed_image(struct swap_map_handle *handle,
>>                                   struct snapshot_handle *snapshot,
>> @@ -1529,6 +1548,8 @@ static int load_compressed_image(struct swap_map_handle *handle,
>>    *     swsusp_read - read the hibernation image.
>>    *     @flags_p: flags passed by the "frozen" kernel in the image header should
>>    *               be written into this memory location
>> + *
>> + *     Return: 0 on success, negative error code on failure.
>>    */
>>
>>   int swsusp_read(unsigned int *flags_p)
>> @@ -1567,6 +1588,10 @@ static void *swsusp_holder;
>>   /**
>>    * swsusp_check - Open the resume device and check for the swsusp signature.
>>    * @exclusive: Open the resume device exclusively.
>> + *
>> + * Return:
>> + *        0 if a valid hibernation image is found,
>> + *        negative error code on failure.
>>    */
>>
>>   int swsusp_check(bool exclusive)
>> @@ -1631,6 +1656,8 @@ void swsusp_close(void)
>>
>>   /**
>>    *      swsusp_unmark - Unmark swsusp signature in the resume device
>> + *
>> + *      Return: 0 on success, negative error code on failure.
>>    */
>>
>>   #ifdef CONFIG_SUSPEND
>> --
>> 2.43.0
>>

Powered by blists - more mailing lists