lkml.org 
[lkml]   [2006]   [Jun]   [27]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
FromNigel Cunningham <>
SubjectRe: [Suspend2][ 0/2] Cryptoapi deflate fix.
DateTue, 27 Jun 2006 17:02:46 +1000
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-2008