From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Daniel P. Smith" <dpsmith@apertussolutions.com>,
xen-devel@lists.xenproject.org
Cc: Jason Andryuk <jason.andryuk@amd.com>,
George Dunlap <george.dunlap@citrix.com>,
Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH v3 0/8] Clean up of gzip decompressor
Date: Thu, 9 May 2024 16:36:21 +0100 [thread overview]
Message-ID: <5bf22886-08a1-459d-939e-008f2c1836ea@citrix.com> (raw)
In-Reply-To: <20240424163422.23276-1-dpsmith@apertussolutions.com>
On 24/04/2024 5:34 pm, Daniel P. Smith wrote:
> An issue ran into by hyperlaunch was the need to use the gzip decompressor
> multiple times. The current implementation fails when reused due to tainting of
> decompressor state from a previous usage. This series seeks to colocate the
> gzip unit files under a single directory similar to the other decompression
> algorithms. To enable the refactoring of the state tracking, the code is then
> cleaned up in line with Xen coding style.
>
> Changes in v3:
> - patch "xen/gzip: Drop huffman code table tracking" was merged
> - patch "xen/gzip: Remove custom memory allocator" was merged
> - patch "xen/gzip: Drop unused define checks" was merged
> - move of the decompressor state into struct was broken up to ease review
> - expanded macros that were either only used once or obsfucated the logic
> - adjusted variable types as needed to be more appropriate for their usage
>
> Changes in v2:
> - patch "xen/gzip: Colocate gunzip code files" was merged
> - dropped ifdef chunks that are never enabled
> - addressed formatting changes missed in v1
> - replaced custom memory allocator with xalloc_bytes()
> - renamed gzip_data to gzip_state
> - moved gzip_state to being dynamically allocated
> - changed crc table to the explicit size of uint32_t
> - instead of moving huffman tracking into state, dropped altogether
>
>
> Daniel P. Smith (8):
> gzip: clean up comments and fix code alignment
> gzip: refactor and expand macros
> gzip: refactor the gunzip window into common state
> gzip: move window pointer into gunzip state
> gzip: move input buffer handling into gunzip state
> gzip: move output buffer into gunzip state
> gzip: move bitbuffer into gunzip state
> gzip: move crc state into gunzip state
In hindsight my suggestion that lead to "refactor and expand macros"
wasn't great.
We want to keep the underrun labels for a future when the error handling
isn't panic(). After that, expanding flush_window() is better folded
into "refactor the gunzip window into common state" to reduce the churn.
As that was my fault, and unpicking it is reasonably hard, and we're on
a tight deadline for 4.19 now, I've gone ahead and unpicked this.
Also I've rebased over Jan's patch addressing the memory leaks reported
by Coverity, as the two sets of fixes collide, along with various other
minor notes.
I've conferred with Daniel who is happy with the aformentioned changes.
This area of code still has a lot of work needing doing on it, but
removing the use of global state is a concrete improvement in the status
quo.
~Andrew
prev parent reply other threads:[~2024-05-09 15:36 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-24 16:34 [PATCH v3 0/8] Clean up of gzip decompressor Daniel P. Smith
2024-04-24 16:34 ` [PATCH v3 1/8] gzip: clean up comments and fix code alignment Daniel P. Smith
2024-04-25 18:31 ` Andrew Cooper
2024-04-24 16:34 ` [PATCH v3 2/8] gzip: refactor and expand macros Daniel P. Smith
2024-04-29 14:07 ` Jan Beulich
2024-04-24 16:34 ` [PATCH v3 3/8] gzip: refactor the gunzip window into common state Daniel P. Smith
2024-04-24 21:34 ` Daniel P. Smith
2024-04-24 16:34 ` [PATCH v3 4/8] gzip: move window pointer into gunzip state Daniel P. Smith
2024-04-29 14:08 ` Jan Beulich
2024-04-24 16:34 ` [PATCH v3 5/8] gzip: move input buffer handling " Daniel P. Smith
2024-04-29 15:04 ` Jan Beulich
2024-04-24 16:34 ` [PATCH v3 6/8] gzip: move output buffer " Daniel P. Smith
2024-04-29 15:07 ` Jan Beulich
2024-04-24 16:34 ` [PATCH v3 7/8] gzip: move bitbuffer " Daniel P. Smith
2024-04-25 19:23 ` Andrew Cooper
2024-04-26 5:55 ` Jan Beulich
2024-04-26 5:57 ` Jan Beulich
2024-04-26 12:36 ` Andrew Cooper
2024-04-24 16:34 ` [PATCH v3 8/8] gzip: move crc state " Daniel P. Smith
2024-04-25 19:19 ` Andrew Cooper
2024-04-29 15:17 ` Jan Beulich
2024-04-24 16:48 ` [PATCH v3 0/8] Clean up of gzip decompressor Daniel P. Smith
2024-05-09 15:36 ` Andrew Cooper [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5bf22886-08a1-459d-939e-008f2c1836ea@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=dpsmith@apertussolutions.com \
--cc=george.dunlap@citrix.com \
--cc=jason.andryuk@amd.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).