Re: suspend2 review [was Re: Which is simpler? (Was Re: [Suspend2-devel] Re: [ 00/10] [Suspend2] Modules support.)]

[Olivier Galibert]

On Mon, Feb 20, 2006 at 06:10:00PM +0100, Pavel Machek wrote:
> On Po 20-02-06 18:05:37, Olivier Galibert wrote:
> > On Mon, Feb 20, 2006 at 01:49:37PM +0100, Pavel Machek wrote:
> > > > > Yep, if you do it all in userspace, this vanishes. 340 lines down.
> > > > 
> > > > And you gain? Let's try not to be too biased :).
> > > 
> > > I gain 340 less lines to review. For me to review, for akpm to review,
> > > and for Linus to review. That's important.
> > 
> > Pavel, if you mean that the userspace code will not be reviewed to
> > standards the kernel code is, kill uswsusp _NOW_ before it does too
> > much damage.  Unreliable suspend eats filesystems for breakfast.  The
> > other userspace components of the kernels services are either optional
> > (udev) or not that important (alsa).
> 
> At least it will be only me reviewing it, and not akpm and Linus.

Ok, your answer was saying the contrary (that you wouldn't review it
either).  Frankly, you may want akpm or Linus to do reviews of even
userspace code when appropriate, and others too like Al Viro.  They
have a freakingly good eye at detecting crap code.


> suspend2 received no such review, and still people claim it is
> reliable.

Plain numbers.  Just count the "suspend2 works for me which swsusp
doesn't".  I doubt it's purely luck, even if simply moving code around
can change behaviours.


> "I wish they'd kill suspend2 project, it already did enough
> damage." (Half joking here, but suspend2 split user/development
> community, and that's not good).

Yes, that's annoying.  But be careful, you seem to be automatically
rejecting everything Nigel at that point, or at least that's what it
looks like.

You do what you want, obivously, but I suspect your reviews of the
suspend2 code would be way more interesting if you accepted it's not
uswsusp.  Right now, they look more religious/political than really
technical.

Can you try doing a review where you temporarily accept suspend2's
kernel/userspace separation in the background, and review the code as
is?  That way you'll even have a chance to find out where the
differences in reliability are coming from.

  OG.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/