Hi. On Tuesday 27 June 2006 16:00, Herbert Xu wrote: > Nigel Cunningham <[email protected]> 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.
Attachment:
pgpVjL0guqz0W.pgp
Description: PGP signature
- Follow-Ups:
- Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- From: Herbert Xu <[email protected]>
- Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- References:
- Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- From: Herbert Xu <[email protected]>
- Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- Prev by Date: Re: [Suspend2][ 07/13] [Suspend2] Page_alloc paranoia.
- Next by Date: Re: GFS2 and DLM
- Previous by thread: Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- Next by thread: Re: [Suspend2][ 0/2] Cryptoapi deflate fix.
- Index(es):
 |