Re: [PATCH v2] power: swap: Convert kernel-doc comments to regular block comments

[Sunday Adelodun <adelodunolaoluwa@yahoo.com>]

On 11/14/25 17:37, Rafael J. Wysocki wrote:
> On Thu, Nov 13, 2025 at 12:31 PM Sunday Adelodun
> <adelodunolaoluwa@yahoo.com> wrote:
>> Several static functions in kernel/power/swap.c were using the kernel-doc
>> comment style (/** ... */) even though they are not exported or referenced
>> in generated documentation. This triggers documentation warnings and makes
>> the comments inconsistent with kernel style for local functions.
>>
>> Convert these comment blocks to regular C-style comments (/* ... */) and
>> update a few function headers to include proper "Return:" descriptions
>> where applicable.
>>
>> No functional changes.
>>
>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com>
>> ---
>> changelog:
>>
>> changes from v1 to v2:
>> - Converted function comment blocks from /** */ to /* */ style for
>>    static functions
>> - Minor reformatting of comment indentation and spacing
>>
>> v1 patch link:
>> https://lore.kernel.org/all/20251106113938.34693-2-adelodunolaoluwa@yahoo.com/
>>
>>   kernel/power/swap.c | 54 ++++++++++++++++++++-------------------------
>>   1 file changed, 24 insertions(+), 30 deletions(-)
>>
>> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
>> index 0beff7eeaaba..076ed590e8c9 100644
>> --- a/kernel/power/swap.c
>> +++ b/kernel/power/swap.c
>> @@ -336,10 +336,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
>>    */
>>   unsigned int swsusp_header_flags;
>>
>> -/**
>> - *     swsusp_swap_check - check if the resume device is a swap device
>> - *     and get its index (if so)
>> - *
>> +/*
>> + *     check if the resume device is a swap device and get its index (if so).
>>    *     This is called before saving image
>>    */
> I'd move this comment (after the change above) into the function body
> (before the first statement) because the function does more than what
> is says.
>
>>   static int swsusp_swap_check(void)
>> @@ -362,11 +360,8 @@ static int swsusp_swap_check(void)
>>          return 0;
>>   }
>>
>> -/**
>> - *     write_page - Write one page to given swap location.
>> - *     @buf:           Address we're writing.
>> - *     @offset:        Offset of the swap page we're writing to.
>> - *     @hb:            bio completion batch
>> +/*
>> + *     Write one page to given swap location.
>>    */
> There is not much value in the comment after the change, please drop
> it altogether.
>
>>   static int write_page(void *buf, sector_t offset, struct hib_bio_batch *hb)
>> @@ -526,8 +521,8 @@ static int swap_writer_finish(struct swap_map_handle *handle,
>>   #define CMP_MIN_RD_PAGES       1024
>>   #define CMP_MAX_RD_PAGES       8192
>>
>> -/**
>> - *     save_image - save the suspend image data
>> +/*
>> + *     save the suspend image data
>>    */
> Same here.
>
>>   static int save_image(struct swap_map_handle *handle,
>> @@ -671,11 +666,8 @@ static int compress_threadfn(void *data)
>>          return 0;
>>   }
>>
>> -/**
>> - * save_compressed_image - Save the suspend image data after compression.
>> - * @handle: Swap map handle to use for saving the image.
>> - * @snapshot: Image to read data from.
>> - * @nr_to_write: Number of pages to save.
>> +/*
>> + * Save the suspend image data after compression.
>>    */
> Same here.
>
>>   static int save_compressed_image(struct swap_map_handle *handle,
>>                                   struct snapshot_handle *snapshot,
>> @@ -904,11 +896,8 @@ static int save_compressed_image(struct swap_map_handle *handle,
>>          return ret;
>>   }
>>
>> -/**
>> - *     enough_swap - Make sure we have enough swap to save the image.
>> - *
>> - *     Returns TRUE or FALSE after checking the total amount of swap
>> - *     space available from the resume partition.
>> +/*
>> + *     Make sure we have enough swap to save the image.
>>    */
> Same here.
>
>>   static int enough_swap(unsigned int nr_pages)
>> @@ -930,6 +919,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.
>>    */
>>
> Please remove the empty line between the comment and the function definition.
>
>>   int swsusp_write(unsigned int flags)
>> @@ -1077,10 +1068,8 @@ static int swap_reader_finish(struct swap_map_handle *handle)
>>          return 0;
>>   }
>>
>> -/**
>> - *     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 the image using the swap map handle
>>    */
>>
>>   static int load_image(struct swap_map_handle *handle,
>> @@ -1190,11 +1179,8 @@ static int decompress_threadfn(void *data)
>>          return 0;
>>   }
>>
>> -/**
>> - * load_compressed_image - Load compressed image data and decompress it.
>> - * @handle: Swap map handle to use for loading data.
>> - * @snapshot: Image to copy uncompressed data into.
>> - * @nr_to_read: Number of pages to load.
>> +/*
>> + * Load compressed image data and decompress it.
>>    */
> This comment is not too useful any more, please drop it.
>
>>   static int load_compressed_image(struct swap_map_handle *handle,
>>                                   struct snapshot_handle *snapshot,
>> @@ -1529,6 +1515,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.
>>    */
>>
> Please remove the empty line between the comment and the function definition.
>
>>   int swsusp_read(unsigned int *flags_p)
>> @@ -1567,6 +1555,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.
> I think that the above can be one line, can't it?

Possibly. I will try compress it into a line.

>
>>    */
> Please remove the empty line between the comment and the function definition.
>
>>   int swsusp_check(bool exclusive)
>> @@ -1631,6 +1623,8 @@ void swsusp_close(void)
>>
>>   /**
>>    *      swsusp_unmark - Unmark swsusp signature in the resume device
>> + *
>> + *      Return: 0 on success, negative error code on failure.
>>    */
> Same here.
>
>>   #ifdef CONFIG_SUSPEND
>> --
> Thanks!
>
Thank you so much for the review and comments.
I will do accordingly and send v3.