linux-kernel - Re: [PATCH] Freeze bdevs when freezing processes.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200610232119.25373.rjw@sisk.pl>
Date:	Mon, 23 Oct 2006 21:19:24 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Andrew Morton <akpm@...l.org>
Cc:	Pavel Machek <pavel@....cz>, ncunningham@...uxmail.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Freeze bdevs when freezing processes.

On Monday, 23 October 2006 19:50, Andrew Morton wrote:
> > On Mon, 23 Oct 2006 19:14:50 +0200 Pavel Machek <pavel@....cz> wrote:
> > On Mon 2006-10-23 09:55:22, Andrew Morton wrote:
> > > > On Mon, 23 Oct 2006 16:07:16 +0200 "Rafael J. Wysocki" <rjw@...k.pl> wrote:
> > > > > I'm trying to prepare the patches to make swsusp into suspend2.
> > > > 
> > > > Oh, I see.  Please don't do that.
> > > 
> > > Why not?

Generally, we already do many things that suspend2 just duplicates and there's
one thing in suspend2 I really don't like, which is the use of LRU pages as
temporary storage.

> > Last time I checked, suspend2 was 15000 lines of code, including its
> > own plugin system and special user-kernel protocol for drawing
> > progress bar (netlink based). It also did parts of user interface from
> 
> That's different.
> 
> I don't know where these patches are leading, but thus far they look like
> reasonable cleanups and generalisations.  So I suggest we just take them
> one at a time.

Well, the patch for the freezeing of bdevs contains a memory leak that I have
already pointed to Nigel for three times, it leads to some problems in the
error paths, AFAICT, and "solves" a problem that doesn't really exist in the
current code.

Some time ago I posted a very similar patch and we discussed it with the XFS
people.  In conclusion we decided not to apply it.

The patch that causes kernel threads to be thawed temporarily before we
attempt to free some memory doesn't look bad, but it's not needed for the
reason given by Nigel.  It may be needed for another reason, but I think we
should know why we apply it before we do so.

I have no specific objections with respect to the other patches except I'd
like to understand the patch that adds extents for the handling of swap before
I say I like it or not.

> > OTOH, that was half a year ago, but given that uswsusp can now do most
> > of the stuff suspend2 does (and without that 15000 lines of code), I
> > do not think we want to do complete rewrite of swsusp now.
> 
> uswsusp seems like a bad idea to me.

Could you please explain why exactly?

> We'd be better off concentrating on a simple, clean in-kernel thing which
> *works*. 

This depends on the point of view.  I test both swsusp and uswsusp on a regular
basis on 5 different boxes and they both work flawlessly (except when some
rogue patch breaks them, but that's a different story).

> Right now the main problems with swsusp are that it's slow and that there
> are driver problems.  (Actually these are both driver problems).

Well, suspend2 is not about drivers.  It's all about the "core", ie. memory
management, saving/restoring the image etc.

As far as the drivers are concerned, we (I, Pavel, Nigel and even you) can't
fix all of them and it really is a challenge to do something that will cause
them to be fixed.

In the meantime we fix some things that _we_ can address, like the handling
of highmem, the PAE-related issue on i386 etc.

> Fiddling with the top-level interfaces doesn't address either of these core
> problems.

No, it doesn't, but for example it's the correct way of handling the resume
from an initrd image, IMHO.

> Apparently uswsusp has gained support for S3 while the in-kernel driver
> does not support S3.  That's disappointing.

The problem with that is on _many_ boxes we have to handle the video in a
special way before and after S3, and it is only possible from the user
space.  This was one of the reasons why we decided to develop the userland
suspend.

-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/