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

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

[xiongxin]

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(-)

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


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH -next 2/2] PM: hibernate: add check of preallocate mem for image size pages

[xiongxin]

Added a check on the return value of preallocate_image_highmem(). If
memory preallocate is insufficient, S4 cannot be done;

I am playing 4K video on a machine with AMD or other graphics card and
only 8GiB memory, and the kernel is not configured with CONFIG_HIGHMEM.
When doing the S4 test, the analysis found that when the pages get from
minimum_image_size() is large enough, The preallocate_image_memory() and
preallocate_image_highmem() calls failed to obtain enough memory. Add
the judgment that memory preallocate is insufficient;

"pages -= free_unnecessary_pages()" below will let pages to drop a lot,
so I wonder if it makes sense to add a judgment here.

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

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index c20ca5fb9adc..670abf89cf31 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1738,6 +1738,7 @@ int hibernate_preallocate_memory(void)
 	struct zone *zone;
 	unsigned long saveable, size, max_size, count, highmem, pages = 0;
 	unsigned long alloc, save_highmem, pages_highmem, avail_normal;
+	unsigned long size_highmem;
 	ktime_t start, stop;
 	int error;
 
@@ -1863,7 +1864,13 @@ int hibernate_preallocate_memory(void)
 		pages_highmem += size;
 		alloc -= size;
 		size = preallocate_image_memory(alloc, avail_normal);
-		pages_highmem += preallocate_image_highmem(alloc - size);
+		size_highmem += preallocate_image_highmem(alloc - size);
+		if (size_highmem < (alloc - size)) {
+			pr_err("Image allocation is %lu pages short, exit\n",
+				alloc - size - pages_highmem);
+			goto err_out;
+		}
+		pages_highmem += size_highmem;
 		pages += pages_highmem + size;
 	}
 
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

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

[xiongxin]

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(-)

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


No virus found
		Checked by Hillstone Network AntiVirus

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

[TGSP]

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(-)

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

[PATCH -next 0/2] Fixes to the hibernate_preallocate_memory()

[TGSP]

From: xiongxin <xiongxin@kylinos.cn>

hibernate_preallocate_memory() function is still quite obscure, can
anyone add a little more description?

It seems that the max_size calculation formula is wrong in the comment
part, correct it;

It is also found that when mem preallocate is not enough, it goes
directly down without checking.

xiongxin (2):
  PM: hibernate: fix spelling mistake for annotation
  PM: hibernate: add check of preallocate mem for image size pages

 kernel/power/snapshot.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.25.1

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

[TGSP]

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(-)

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

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

[Bagas Sanjaya]

On 11/1/22 09:28, TGSP 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?
> 
> By the way, what exactly do the "/ 2" and "2 *" mean?
> 

Shouldn't the description above better below triple-dash (just
before diffstat)?

-- 
An old man doll... just what I always wanted! - Clara

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

[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.

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
>

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

[TGSP]

在 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
>>

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

[Rafael J. Wysocki]

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.