* [PATCH v2 0/2] Fixes to the hibernate_preallocate_memory() @ 2022-11-04 5:41 TGSP 2022-11-04 5:41 ` [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation TGSP 2022-11-04 5:41 ` [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP 0 siblings, 2 replies; 8+ messages in thread From: TGSP @ 2022-11-04 5:41 UTC (permalink / raw) To: rafael, len.brown, pavel, huanglei; +Cc: linux-pm, linux-kernel, TGSP Changes in v2: - Optimize code to resolve compilation warnings; - Add comments, add debug data, easy to view and modify. v1: 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 | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation 2022-11-04 5:41 [PATCH v2 0/2] Fixes to the hibernate_preallocate_memory() TGSP @ 2022-11-04 5:41 ` TGSP 2022-11-04 9:18 ` Greg KH 2022-11-04 5:41 ` [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP 1 sibling, 1 reply; 8+ messages in thread From: TGSP @ 2022-11-04 5:41 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] 8+ messages in thread
* Re: [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation 2022-11-04 5:41 ` [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation TGSP @ 2022-11-04 9:18 ` Greg KH 2022-11-04 9:47 ` TGSP 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2022-11-04 9:18 UTC (permalink / raw) To: TGSP Cc: rafael, len.brown, pavel, huanglei, linux-pm, linux-kernel, xiongxin, stable On Fri, Nov 04, 2022 at 01:41:18PM +0800, 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? > > Cc: stable@vger.kernel.org > Signed-off-by: xiongxin <xiongxin@kylinos.cn> Please do not use an anonymous gmail account for your corporate work like this. Work with your company email admins to allow you to send patches from that address so that they can be verified to actually come from there. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation 2022-11-04 9:18 ` Greg KH @ 2022-11-04 9:47 ` TGSP 0 siblings, 0 replies; 8+ messages in thread From: TGSP @ 2022-11-04 9:47 UTC (permalink / raw) To: Greg KH Cc: xiongxin, rafael, len.brown, pavel, huanglei, linux-pm, linux-kernel, stable 在 2022/11/4 17:18, Greg KH 写道: > On Fri, Nov 04, 2022 at 01:41:18PM +0800, 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? >> >> Cc: stable@vger.kernel.org >> Signed-off-by: xiongxin <xiongxin@kylinos.cn> > > Please do not use an anonymous gmail account for your corporate work > like this. Work with your company email admins to allow you to send > patches from that address so that they can be verified to actually come > from there. > > thanks, > > greg k-h I also wanted to send it directly through the company mailbox, but those leaders didn't take it seriously. Next time I don't use the company email and submit patches as I can as a freelancer. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages 2022-11-04 5:41 [PATCH v2 0/2] Fixes to the hibernate_preallocate_memory() TGSP 2022-11-04 5:41 ` [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation TGSP @ 2022-11-04 5:41 ` TGSP 2022-11-04 6:25 ` Jiri Slaby 1 sibling, 1 reply; 8+ messages in thread From: TGSP @ 2022-11-04 5:41 UTC (permalink / raw) To: rafael, len.brown, pavel, huanglei Cc: linux-pm, linux-kernel, xiongxin, stable 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; The detailed debugging data is as follows: image_size: 3225923584, totalram_pages: 1968948 in hibernate_reserved_size_init(); in hibernate_preallocate_memory(): code pages = minimum_image_size(saveable) = 717992, at this time(line): count: 2030858 avail_normal: 2053753 highmem: 0 totalreserve_pages: 22895 max_size: 1013336 size: 787579 saveable: 1819905 When the code executes to: pages = preallocate_image_memory(alloc, avail_normal), at that time(line): pages_highmem: 0 avail_normal: 1335761 alloc: 1017522 pages: 1017522 So enter the else branch judged by (pages < alloc), When executed to size = preallocate_image_memory(alloc, avail_normal): alloc = max_size - size = 225757; size = preallocate_image_memory(alloc, avail_normal) = 168671, That is, preallocate_image_memory() does not apply for all alloc memory pages, because highmem is not enabled, and size_highmem will return 0 here, so there is a memory page that has not been preallocated, so I think a judgment needs to be added here. But what I can't understand is that although pages are not preallocated enough, "pages -= free_unnecessary_pages()" in the code below can also discard some pages that have been preallocated, so I am not sure whether it is appropriate 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 | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c20ca5fb9adc..546d544cf7de 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -1854,6 +1854,8 @@ int hibernate_preallocate_memory(void) alloc = (count - pages) - size; pages += preallocate_image_highmem(alloc); } else { + unsigned long size_highmem = 0; + /* * There are approximately max_size saveable pages at this point * and we want to reduce this number down to size. @@ -1863,8 +1865,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); - pages += pages_highmem + 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 += pages_highmem + size_highmem + size; } /* -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages 2022-11-04 5:41 ` [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP @ 2022-11-04 6:25 ` Jiri Slaby 2022-11-04 7:27 ` TGSP 0 siblings, 1 reply; 8+ messages in thread From: Jiri Slaby @ 2022-11-04 6:25 UTC (permalink / raw) To: TGSP, rafael, len.brown, pavel, huanglei Cc: linux-pm, linux-kernel, xiongxin, stable On 04. 11. 22, 6:41, TGSP wrote: > 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; > > The detailed debugging data is as follows: > > image_size: 3225923584, totalram_pages: 1968948 in > hibernate_reserved_size_init(); > > in hibernate_preallocate_memory(): > code pages = minimum_image_size(saveable) = 717992, at this time(line): > count: 2030858 > avail_normal: 2053753 > highmem: 0 > totalreserve_pages: 22895 > max_size: 1013336 > size: 787579 > saveable: 1819905 > > When the code executes to: > pages = preallocate_image_memory(alloc, avail_normal), at that > time(line): > pages_highmem: 0 > avail_normal: 1335761 > alloc: 1017522 > pages: 1017522 > > So enter the else branch judged by (pages < alloc), When executed to > size = preallocate_image_memory(alloc, avail_normal): > alloc = max_size - size = 225757; > size = preallocate_image_memory(alloc, avail_normal) = 168671, That is, > preallocate_image_memory() does not apply for all alloc memory pages, > because highmem is not enabled, and size_highmem will return 0 here, so > there is a memory page that has not been preallocated, so I think a > judgment needs to be added here. > > But what I can't understand is that although pages are not preallocated > enough, "pages -= free_unnecessary_pages()" in the code below can also > discard some pages that have been preallocated, so I am not sure whether > it is appropriate 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 | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index c20ca5fb9adc..546d544cf7de 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -1854,6 +1854,8 @@ int hibernate_preallocate_memory(void) > alloc = (count - pages) - size; > pages += preallocate_image_highmem(alloc); > } else { > + unsigned long size_highmem = 0; This needs not be initialized, right? > @@ -1863,8 +1865,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); > - pages += pages_highmem + 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 += pages_highmem + size_highmem + size; > } > > /* -- js suse labs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages 2022-11-04 6:25 ` Jiri Slaby @ 2022-11-04 7:27 ` TGSP 0 siblings, 0 replies; 8+ messages in thread From: TGSP @ 2022-11-04 7:27 UTC (permalink / raw) To: Jiri Slaby, rafael, len.brown, pavel, huanglei Cc: xiongxin, linux-pm, linux-kernel, stable 在 2022/11/4 14:25, Jiri Slaby 写道: > On 04. 11. 22, 6:41, TGSP wrote: >> 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; >> >> The detailed debugging data is as follows: >> >> image_size: 3225923584, totalram_pages: 1968948 in >> hibernate_reserved_size_init(); >> >> in hibernate_preallocate_memory(): >> code pages = minimum_image_size(saveable) = 717992, at this time(line): >> count: 2030858 >> avail_normal: 2053753 >> highmem: 0 >> totalreserve_pages: 22895 >> max_size: 1013336 >> size: 787579 >> saveable: 1819905 >> >> When the code executes to: >> pages = preallocate_image_memory(alloc, avail_normal), at that >> time(line): >> pages_highmem: 0 >> avail_normal: 1335761 >> alloc: 1017522 >> pages: 1017522 >> >> So enter the else branch judged by (pages < alloc), When executed to >> size = preallocate_image_memory(alloc, avail_normal): >> alloc = max_size - size = 225757; >> size = preallocate_image_memory(alloc, avail_normal) = 168671, That is, >> preallocate_image_memory() does not apply for all alloc memory pages, >> because highmem is not enabled, and size_highmem will return 0 here, so >> there is a memory page that has not been preallocated, so I think a >> judgment needs to be added here. >> >> But what I can't understand is that although pages are not preallocated >> enough, "pages -= free_unnecessary_pages()" in the code below can also >> discard some pages that have been preallocated, so I am not sure whether >> it is appropriate 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 | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c >> index c20ca5fb9adc..546d544cf7de 100644 >> --- a/kernel/power/snapshot.c >> +++ b/kernel/power/snapshot.c >> @@ -1854,6 +1854,8 @@ int hibernate_preallocate_memory(void) >> alloc = (count - pages) - size; >> pages += preallocate_image_highmem(alloc); >> } else { >> + unsigned long size_highmem = 0; > > This needs not be initialized, right? If there is no need to make judgments here, then in the (pages < alloc) branch, it is necessary to make judgments on (pages_highmem < alloc)? should be the same; > >> @@ -1863,8 +1865,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); >> - pages += pages_highmem + 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 += pages_highmem + size_highmem + size; >> } >> /* > ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20221108073741.3837659-1-xiongxin@kylinos.cn>]
* [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages [not found] <20221108073741.3837659-1-xiongxin@kylinos.cn> @ 2022-11-08 7:37 ` xiongxin 0 siblings, 0 replies; 8+ messages in thread From: xiongxin @ 2022-11-08 7:37 UTC (permalink / raw) To: xiongxin; +Cc: stable, huanglei 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; The detailed debugging data is as follows: image_size: 3225923584, totalram_pages: 1968948 in hibernate_reserved_size_init(); in hibernate_preallocate_memory(): code pages = minimum_image_size(saveable) = 717992, at this time(line): count: 2030858 avail_normal: 2053753 highmem: 0 totalreserve_pages: 22895 max_size: 1013336 size: 787579 saveable: 1819905 When the code executes to: pages = preallocate_image_memory(alloc, avail_normal), at that time(line): pages_highmem: 0 avail_normal: 1335761 alloc: 1017522 pages: 1017522 So enter the else branch judged by (pages < alloc), When executed to size = preallocate_image_memory(alloc, avail_normal): alloc = max_size - size = 225757; size = preallocate_image_memory(alloc, avail_normal) = 168671, That is, preallocate_image_memory() does not apply for all alloc memory pages, because highmem is not enabled, and size_highmem will return 0 here, so there is a memory page that has not been preallocated, so I think a judgment needs to be added here. But what I can't understand is that although pages are not preallocated enough, "pages -= free_unnecessary_pages()" in the code below can also discard some pages that have been preallocated, so I am not sure whether it is appropriate 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 | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c20ca5fb9adc..546d544cf7de 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -1854,6 +1854,8 @@ int hibernate_preallocate_memory(void) alloc = (count - pages) - size; pages += preallocate_image_highmem(alloc); } else { + unsigned long size_highmem = 0; + /* * There are approximately max_size saveable pages at this point * and we want to reduce this number down to size. @@ -1863,8 +1865,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); - pages += pages_highmem + 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 += pages_highmem + size_highmem + size; } /* -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-08 7:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 5:41 [PATCH v2 0/2] Fixes to the hibernate_preallocate_memory() TGSP
2022-11-04 5:41 ` [PATCH v2 1/2] PM: hibernate: fix spelling mistake for annotation TGSP
2022-11-04 9:18 ` Greg KH
2022-11-04 9:47 ` TGSP
2022-11-04 5:41 ` [PATCH v2 2/2] PM: hibernate: add check of preallocate mem for image size pages TGSP
2022-11-04 6:25 ` Jiri Slaby
2022-11-04 7:27 ` TGSP
[not found] <20221108073741.3837659-1-xiongxin@kylinos.cn>
2022-11-08 7:37 ` 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.