Re: [PATCH -mm] swsusp: support creating bigger images (rev. 2)

[Nigel Cunningham <ncunningham@cyclades.com>]

[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]

Hi.

On Thursday 11 May 2006 21:35, Pavel Machek wrote:
> On Čt 11-05-06 00:58:18, Rafael J. Wysocki wrote:
> > On Wednesday 10 May 2006 00:27, Andrew Morton wrote:
> > > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > > > Now if the mapped pages that are not mapped by the
> > > >  current task are considered, it turns out that they would change
> > > > only if they were reclaimed by try_to_free_pages().  Thus if we take
> > > > them out of reach of try_to_free_pages(), for example by
> > > > (temporarily) moving them out of their respective LRU lists after
> > > > creating the image, we will be able to include them in the image
> > > > without copying.
> > >
> > > I'm a bit curious about how this is true.  There are all sorts of way
> > > in which there could be activity against these pages - interrupt-time
> > > asynchronous network Tx completion, async interrupt-time direct-io
> > > completion, tasklets, schedule_work(), etc, etc.
> >
> > AFAIK, many of these things are waited for uninterruptibly, and
> > uninterruptible
>
> Well, "many of these things" makes me nervous.
>
> > tasks cannot be frozen.  Theoretically we may have a problem if there's
> > an interruptible task that waits for the completion of an operation that
> > gets finished after snapshotting the system.
>
> I'd prefer not to have even theoretical problems. If we don't _know_
> why patch is safe, I'd prefer not to have it.
>
> Needing bdev freezing is bad sign, too.
>
> We are talking 10% speedup here (on low-mem-machines, IIRC), but whole
> design has just got way more complex. Previous snapshot was really
> atomic, and apart from NMI, it was "independend" from the rest of the
> system.
>
> New design depends on bdev freezing (depending on XFS details we do
> not understand), and depends on all the other parts of kernel using
> uninteruptible (when we know that networking sleeps interruptibly).
>
> Too much uncertainity for 10% speedup, I'm afraid. Yes, it was really
> clever to get this fundamental change down to few hundred lines, but
> design complexity remains. Could we drop that patch?

Could you provide justification for your claim that the speedup is only 10%?

Please also remember that you are introducing complexity in other ways, with 
that swap prefetching code and so on. Any comparison in speed should include 
the time to fault back in pages that have been discarded.

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]