| Message ID | 20221101022840.1351163-2-tgsp002@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | Fixes to the hibernate_preallocate_memory() | expand |
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)?
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 >
在 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 >>
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 --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. *