public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nigel Cunningham <nigel@tuxonice.net>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Bojan Smojver <bojan@rexursive.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH]: Compress hibernation image with LZO (in-kernel)
Date: Wed, 04 Aug 2010 12:24:57 +1000	[thread overview]
Message-ID: <4C58CF79.4000406@tuxonice.net> (raw)
In-Reply-To: <20100804110248.13a2452b.kamezawa.hiroyu@jp.fujitsu.com>

Hi.

On 04/08/10 12:02, KAMEZAWA Hiroyuki wrote:
> On Wed, 04 Aug 2010 11:50:41 +1000
> Nigel Cunningham<nigel@tuxonice.net>  wrote:
>
>> Hi.
>>
>> Sorry for the delay in replying.
>>
>> On 03/08/10 11:59, Bojan Smojver wrote:
>>>>  From all this, I only got "In swsusp_free()" printed on resume. So, it
>>> seems that save_image() does indeed free those vmalloc()-ed buffers and
>>> they are not saved in the image.
>>>
>>> I even put this in hibernate.c:
>>> ---------------------
>>>           /* Restore control flow magically appears here */
>>>           restore_processor_state();
>>>           if (!in_suspend)
>>>                   platform_leave(platform_mode);
>>>
>>>           printk(KERN_ERR "Resumed, checking swsusp_lzo_buffers.\n");
>>>           if (swsusp_lzo_buffers) {
>>>                   printk (KERN_ERR "Setting vmalloc() buffers.\n");
>>>                   memset(swsusp_lzo_buffers, 0, 80 * PAGE_SIZE);
>>>           }
>>> ---------------------
>>>
>>> This printed just "Resumed, checking swsusp_lzo_buffers.", meaning it
>>> was already set to NULL.
>>>
>>> Any further comments on this? Nigel, what do you reckon?
>>
>> I don't see what all the fuss was about. save_image is called after the
>> snapshot is made (hibernate called hibernation_snapshot to create the
>> image, then swsusp_write which in turn calls save_image), so there's no
>> possibility of the memory allocated by it being included in the image or
>> of a memory leak ocuring as a result.
>>
>
> I'm sorry if I'm wrong. Here is logic in my mind. (I'm reading hibernate(void).
> not sure about user.c )
> (linenumber is from 2.6.34 global, plz ignore)

Yeah, I'm ignoring uswsusp too. That shouldn't matter.

>   624         error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
>   625         if (error)
>   626                 goto Thaw;
>   627
>   628         if (in_suspend) {
>   629                 unsigned int flags = 0;
>   630
>   631                 if (hibernation_mode == HIBERNATION_PLATFORM)
>   632                         flags |= SF_PLATFORM_MODE;
>   633                 pr_debug("PM: writing image.\n");
>   634                 error = swsusp_write(flags);
>
> Then, swsusp_write() actually writes image to the disk. Right ?

Yes

> It finally calls save_image(). save_image() executes following loop.
>
>   433         while (1) {
>   434                 ret = snapshot_read_next(snapshot, PAGE_SIZE);
>   435                 if (ret<= 0)
>   436                         break;
>   437                 ret = swap_write_page(handle, data_of(*snapshot),&bio);
>   438                 if (ret)
>   439                         break;
>   440                 if (!(nr_pages % m))
>   441                         printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
>   442                 nr_pages++;
>   443         }
>
> Ok, here, the image written to disk is got by snapshot_read_next().
> Then, what it does ?
>
> ==
> 1650                         page = pfn_to_page(memory_bm_next_pfn(&copy_bm));
> 1651                         if (PageHighMem(page)) {
> 1652                                 /* Highmem pages are copied to the buffer,
> 1653                                  * because we can't return with a kmapped
> 1654                                  * highmem page (we may not be called again).
> 1655                                  */
> 1656                                 void *kaddr;
> 1657
> 1658                                 kaddr = kmap_atomic(page, KM_USER0);
> 1659                                 memcpy(buffer, kaddr, PAGE_SIZE);
> 1660                                 kunmap_atomic(kaddr, KM_USER0);
> 1661                                 handle->buffer = buffer;
> 1662                         } else {
> 1663                                 handle->buffer = page_address(page);
> 1664                         }
> ==
>
> Ok, what actually written to disk is an image of memory at save_image() is
> called.
>
> What happens at vmalloc ?
> 	1. page tabels for vmalloc() is set up.
> 	2. vm_struct object is allocated.
> 	3. vmap is allcoated.
>
> Above "3" states are all saved into the disk image, as "used".
>
> Then, after resume, all vmalloc() area is resumed as "allocated".
>
> Wrong ?

Yes, because what's being written is the snapshot that was created in 
hibernation_snapshot. Any memory you allocate afterwards is irrelevant 
because it's not part of that snapshot that was made earlier and is now 
being written to disk. Think of the point where hibernation_snapshot is 
called as being like taking a photo, and this later part as like 
printing the photo. Once you've taken the photo, people in the photo can 
move around without making any difference to the photo you've taken. So 
here. Our vmallocs and so on after the snapshot don't affect it.

Regards,

Nigel

  parent reply	other threads:[~2010-08-04  2:25 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-30  4:46 [PATCH]: Compress hibernation image with LZO (in-kernel) Bojan Smojver
2010-07-30 10:44 ` Bojan Smojver
2010-07-30 22:05   ` Nigel Cunningham
2010-07-30 22:19     ` Bojan Smojver
2010-07-30 23:22     ` Bojan Smojver
2010-07-30 23:40       ` Nigel Cunningham
2010-07-31  1:03         ` Bojan Smojver
2010-07-31  1:18           ` Nigel Cunningham
2010-07-31  1:33             ` Bojan Smojver
2010-07-31  4:41               ` Bojan Smojver
2010-07-31  5:03                 ` Bojan Smojver
2010-08-02  0:17                 ` KAMEZAWA Hiroyuki
2010-08-02  0:54                   ` Bojan Smojver
2010-08-02  1:10                     ` KAMEZAWA Hiroyuki
2010-08-02  1:21                       ` Bojan Smojver
2010-08-02  1:27                         ` KAMEZAWA Hiroyuki
2010-08-02  1:43                           ` Bojan Smojver
2010-08-03  1:59                             ` Bojan Smojver
2010-08-03  2:30                               ` Bojan Smojver
2010-08-04  2:42                                 ` Nigel Cunningham
2010-08-04  2:47                                   ` Bojan Smojver
2010-08-04  4:04                                     ` Bojan Smojver
2010-08-04  4:23                                       ` Nigel Cunningham
2010-08-04  5:12                                         ` Bojan Smojver
2010-08-04  5:58                                           ` Bojan Smojver
2010-08-05  1:26                                     ` Bojan Smojver
2010-08-03  6:34                               ` Bojan Smojver
2010-08-04  1:50                               ` Nigel Cunningham
2010-08-04  1:58                                 ` Bojan Smojver
2010-08-04  2:02                                 ` KAMEZAWA Hiroyuki
2010-08-04  2:14                                   ` Bojan Smojver
2010-08-04  2:18                                     ` KAMEZAWA Hiroyuki
2010-08-04  2:37                                       ` Nigel Cunningham
2010-08-04  2:24                                   ` Nigel Cunningham [this message]
2010-08-04  2:24                                     ` KAMEZAWA Hiroyuki
2010-08-04  2:38                                       ` Nigel Cunningham
2010-08-05  6:26 ` Pavel Machek
2010-08-05  6:55   ` Bojan Smojver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C58CF79.4000406@tuxonice.net \
    --to=nigel@tuxonice.net \
    --cc=bojan@rexursive.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox