lkml.org 
[lkml]   [2006]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [Suspend2][ 0/2] Cryptoapi deflate fix.
Date
Hi.

On Tuesday 27 June 2006 16:00, Herbert Xu wrote:
> Nigel Cunningham <nigel@suspend2.net> wrote:
> > Sorry for the bad choice of heading. I have posted this before and
> > emailed it direct to Herbert, but he (rightly) doesn't see the need while
> > suspend2 isn't merged.
>
> Hmm, I don't recall ever telling you that.

Ok. Sorry for my wonky memory then :)

> I searched through my mail archive here is what I said to you last year
> on these two patches. As far as I can see I don't have a reply from you
> on either subject. So if you could reply to my questions below then I
> can consider your patches again.
>
> 1. Deflate fix:
> > > Hi Nigel, I need a bit more information about this patch.
> > > Do you have a specific input stream that requires a fix like
> > > this?
> >
> > Yes, Suspend2 if the user selects deflate as their compressor. The
> > output data will be PAGE_SIZE chunks, but deflate sometimes thinks it
> > has an extra byte to give us back.
>
> Are you saying that you're feeding it PAGE_SIZE chunks and it's
> giving you back something bigger than a page? That is expected
> since the nature of compression in general is that not everything
> is compressible. Data streams which are not compressible will
> end up bigger than what you start with.
>
> What would be a bug is if you feed it something that's
> deflateBound^-1(PAGE_SIZE) bytes long and it spits back
> something that's longer than a page.

Yes, I'm always feeding it PAGE_SIZE chunks and compressing each page
separately. It's a long time since I looked at or thought about this, so I'll
spend some more time getting it fresh in my head if you like.

> > I agree that it's ugly and don't recall using it when I had gzip support
> > in an earlier version of Suspend2. Are you thinking there might be a
> > better way? If so, I can dig out the old (non crypto api) code.
>
> Well if you could give me a snippet of code that actually uses this
> stuff to compress pages then I might have a better idea of what it's
> trying to do.
>
> 2. LZF:
> > +static int lzf_compress_init(void *context)
> > +{
> > + struct lzf_ctx *ctx = (struct lzf_ctx *)context;
> > +
> > + /* Get LZF ready to go */
> > + ctx->hbuf = vmalloc_32((1 << hlog) * sizeof(char *));
>
> Any reason why vmalloc can't be used?

I just left Marc's original code as it was, so I'm not completely sure, but I
guess it's because we want lowmem.

> > + /* Allocate local buffer */
> > + ctx->local_buffer = (char *)get_zeroed_page(GFP_ATOMIC);
> >
> > + /* Allocate page buffer */
> > + ctx->page_buffer = (char *)get_zeroed_page(GFP_ATOMIC);
>
> Where are these two buffers used anyway?

Page_buffer is the destination buffer passed to the compressor. Local buffer
is used to buffer the output for writing (and vv).

> > + if (!ctx->page_buffer) {
> > + free_page((unsigned long)ctx->local_buffer);
> > + lzf_compress_exit(ctx);
>
> This is a double-free of local_buffer.

Is that from our previous correspondence? I can't find anything like it now.

Regards,

Nigel

> Cheers,

--
See http://www.suspend2.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2006-06-27 09:05    [from the cache]
©2003-2011 Jasper Spaans