All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
       [not found] <20221101022342.1345980-1-tgsp002@gmail.com>
@ 2022-11-01  2:23 ` TGSP
  2022-11-01  2:23 ` [PATCH -next 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP
  1 sibling, 0 replies; 9+ messages in thread
From: TGSP @ 2022-11-01  2:23 UTC (permalink / raw)
  To: xiongxin; +Cc: stable

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH -next 2/2] PM: hibernate: add check of preallocate mem for image size pages
       [not found] <20221101022342.1345980-1-tgsp002@gmail.com>
  2022-11-01  2:23 ` [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
@ 2022-11-01  2:23 ` TGSP
  1 sibling, 0 replies; 9+ messages in thread
From: TGSP @ 2022-11-01  2:23 UTC (permalink / raw)
  To: xiongxin; +Cc: stable, huanglei

From: xiongxin <xiongxin@kylinos.cn>

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
  2022-11-04  7:31     ` TGSP
@ 2022-11-05 18:09       ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2022-11-05 18:09 UTC (permalink / raw)
  To: TGSP
  Cc: Rafael J. Wysocki, xiongxin, len.brown, pavel, huanglei,
	linux-pm, linux-kernel, stable

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
  2022-11-03 16:25   ` Rafael J. Wysocki
@ 2022-11-04  7:31     ` TGSP
  2022-11-05 18:09       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: TGSP @ 2022-11-04  7:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: xiongxin, len.brown, pavel, huanglei, linux-pm, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
  2022-11-01  2:28 ` [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
  2022-11-01 12:45   ` Bagas Sanjaya
@ 2022-11-03 16:25   ` Rafael J. Wysocki
  2022-11-04  7:31     ` TGSP
  1 sibling, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2022-11-03 16:25 UTC (permalink / raw)
  To: TGSP
  Cc: rafael, len.brown, pavel, huanglei, linux-pm, linux-kernel,
	xiongxin, stable

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
  2022-11-01  2:28 ` [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
@ 2022-11-01 12:45   ` Bagas Sanjaya
  2022-11-03 16:25   ` Rafael J. Wysocki
  1 sibling, 0 replies; 9+ messages in thread
From: Bagas Sanjaya @ 2022-11-01 12:45 UTC (permalink / raw)
  To: TGSP, rafael, len.brown, pavel, huanglei
  Cc: linux-pm, linux-kernel, xiongxin, stable

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
  2022-11-01  2:28 [PATCH -next 0/2] Fixes to the hibernate_preallocate_memory() TGSP
@ 2022-11-01  2:28 ` TGSP
  2022-11-01 12:45   ` Bagas Sanjaya
  2022-11-03 16:25   ` Rafael J. Wysocki
  0 siblings, 2 replies; 9+ messages in thread
From: TGSP @ 2022-11-01  2:28 UTC (permalink / raw)
  To: rafael, len.brown, pavel, huanglei
  Cc: linux-pm, linux-kernel, xiongxin, stable

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
       [not found] <20221031124837.720118-1-xiongxin@kylinos.cn>
@ 2022-10-31 12:48 ` xiongxin
  0 siblings, 0 replies; 9+ messages in thread
From: xiongxin @ 2022-10-31 12:48 UTC (permalink / raw)
  To: xiongxin; +Cc: stable

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation
       [not found] <20221031124823.719912-1-xiongxin@kylinos.cn>
@ 2022-10-31 12:48 ` xiongxin
  0 siblings, 0 replies; 9+ messages in thread
From: xiongxin @ 2022-10-31 12:48 UTC (permalink / raw)
  To: win239; +Cc: xiongxin, stable

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-11-05 18:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20221101022342.1345980-1-tgsp002@gmail.com>
2022-11-01  2:23 ` [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
2022-11-01  2:23 ` [PATCH -next 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP
2022-11-01  2:28 [PATCH -next 0/2] Fixes to the hibernate_preallocate_memory() TGSP
2022-11-01  2:28 ` [PATCH -next 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
2022-11-01 12:45   ` Bagas Sanjaya
2022-11-03 16:25   ` Rafael J. Wysocki
2022-11-04  7:31     ` TGSP
2022-11-05 18:09       ` Rafael J. Wysocki
     [not found] <20221031124837.720118-1-xiongxin@kylinos.cn>
2022-10-31 12:48 ` xiongxin
     [not found] <20221031124823.719912-1-xiongxin@kylinos.cn>
2022-10-31 12:48 ` xiongxin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.