diff mbox series

[-next,1/2] PM: hibernate: fix spelling mistake for annotation

Message ID 20221101022840.1351163-2-tgsp002@gmail.com
State Superseded
Headers show
Series [-next,1/2] PM: hibernate: fix spelling mistake for annotation | expand

Commit Message

金塔尖 Nov. 1, 2022, 2:28 a.m. UTC
From: xiongxin <xiongxin@kylinos.cn>

The actual calculation formula in the code below is:

max_size = (count - (size + PAGES_FOR_IO)) / 2
	    - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);

But function comments are written differently, the comment is wrong?

By the way, what exactly do the "/ 2" and "2 *" mean?

Cc: stable@vger.kernel.org
Signed-off-by: xiongxin <xiongxin@kylinos.cn>
---
 kernel/power/snapshot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

金塔尖 Nov. 4, 2022, 7:31 a.m. UTC | #1
在 2022/11/4 00:25, Rafael J. Wysocki 写道:
> On Tue, Nov 1, 2022 at 3:28 AM TGSP <tgsp002@gmail.com> wrote:
>>
>> From: xiongxin <xiongxin@kylinos.cn>
>>
>> The actual calculation formula in the code below is:
>>
>> max_size = (count - (size + PAGES_FOR_IO)) / 2
>>              - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);
>>
>> But function comments are written differently, the comment is wrong?
> 
> It is, and it is more serious than just a spelling mistake.
> 
>> By the way, what exactly do the "/ 2" and "2 *" mean?
> 
> Every page in the image is a copy of an existing allocated page, so
> room needs to be made for the two, except for the "IO pages" and
> metadata pages that are not copied.  Hence, the division by 2.
> 
> Now, the "reserved_size" pages will be allocated right before creating
> the image and there will be a copy of each of them in the image, so
> there needs to be room for twice as many.

According to your interpretation, the formula should be:
max_size = (count - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
                - (size + PAGES_FOR_IO)) / 2

Am I right?

> 
> I'll adjust the changelog and queue up the path for 6.2.
> 
>> Cc: stable@vger.kernel.org
> 
> I'll add a Fixes tag instead.
> 
>> Signed-off-by: xiongxin <xiongxin@kylinos.cn>
>> ---
>>   kernel/power/snapshot.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
>> index 2a406753af90..c20ca5fb9adc 100644
>> --- a/kernel/power/snapshot.c
>> +++ b/kernel/power/snapshot.c
>> @@ -1723,8 +1723,8 @@ static unsigned long minimum_image_size(unsigned long saveable)
>>    * /sys/power/reserved_size, respectively).  To make this happen, we compute the
>>    * total number of available page frames and allocate at least
>>    *
>> - * ([page frames total] + PAGES_FOR_IO + [metadata pages]) / 2
>> - *  + 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
>> + * ([page frames total] - PAGES_FOR_IO - [metadata pages]) / 2
>> + *  - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
>>    *
>>    * of them, which corresponds to the maximum size of a hibernation image.
>>    *
>> --
>> 2.25.1
>>
Rafael J. Wysocki Nov. 5, 2022, 6:09 p.m. UTC | #2
On Fri, Nov 4, 2022 at 8:31 AM TGSP <tgsp002@gmail.com> wrote:
>
> 在 2022/11/4 00:25, Rafael J. Wysocki 写道:
> > On Tue, Nov 1, 2022 at 3:28 AM TGSP <tgsp002@gmail.com> wrote:
> >>
> >> From: xiongxin <xiongxin@kylinos.cn>
> >>
> >> The actual calculation formula in the code below is:
> >>
> >> max_size = (count - (size + PAGES_FOR_IO)) / 2
> >>              - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);
> >>
> >> But function comments are written differently, the comment is wrong?
> >
> > It is, and it is more serious than just a spelling mistake.
> >
> >> By the way, what exactly do the "/ 2" and "2 *" mean?
> >
> > Every page in the image is a copy of an existing allocated page, so
> > room needs to be made for the two, except for the "IO pages" and
> > metadata pages that are not copied.  Hence, the division by 2.
> >
> > Now, the "reserved_size" pages will be allocated right before creating
> > the image and there will be a copy of each of them in the image, so
> > there needs to be room for twice as many.
>
> According to your interpretation, the formula should be:
> max_size = (count - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
>                 - (size + PAGES_FOR_IO)) / 2
>
> Am I right?

No, you aren't.

The formula is fine.  I've attempted to explain it to you, but perhaps
it's not been clear enough, sorry about that.
diff mbox series

Patch

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 2a406753af90..c20ca5fb9adc 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1723,8 +1723,8 @@  static unsigned long minimum_image_size(unsigned long saveable)
  * /sys/power/reserved_size, respectively).  To make this happen, we compute the
  * total number of available page frames and allocate at least
  *
- * ([page frames total] + PAGES_FOR_IO + [metadata pages]) / 2
- *  + 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
+ * ([page frames total] - PAGES_FOR_IO - [metadata pages]) / 2
+ *  - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE)
  *
  * of them, which corresponds to the maximum size of a hibernation image.
  *