[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/5] gzip: move huffman code table tracking into gzip state


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Fri, 12 Apr 2024 07:47:01 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1712922426; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=UmGwatd2RFkFR+bnz4CcwqllOhXf8pmquBuOMevZ2G4=; b=eii8b0ugCHVyzQjhQEPhJCvKL6SQEUwPIEPip6wshV2/JmK5lhIz8GtyHYhlQNRjwodzKr+kHoa83KzHQGpCzWGFFOv1UxRqnp2/QIKdcga0hNK/zE/1otbYQYQj2w32eldYgossu7hOhHHHT3qFkyzr8CrD0Cn2yP8Nc6wrrok=
  • Arc-seal: i=1; a=rsa-sha256; t=1712922426; cv=none; d=zohomail.com; s=zohoarc; b=k9CeVTvRq4A2si4GfoamrtlNdSqs9oSCTE+fo7V1snIFqUPup7qGJ4tB51iQognXmoibms8fVPA8DPlV2jYUPDlvktwe+3iXOHEbU9SuBCIXijCI1T70w6/Y0agsOxwP6M9AVOD91CX2HXpXz7nwVKk5XDl7DkHorPwN+n56qBI=
  • Autocrypt: addr=dpsmith@xxxxxxxxxxxxxxxxxxxx; keydata= xsJuBFYrueARCACPWL3r2bCSI6TrkIE/aRzj4ksFYPzLkJbWLZGBRlv7HQLvs6i/K4y/b4fs JDq5eL4e9BdfdnZm/b+K+Gweyc0Px2poDWwKVTFFRgxKWq9R7McwNnvuZ4nyXJBVn7PTEn/Z G7D08iZg94ZsnUdeXfgYdJrqmdiWA6iX9u84ARHUtb0K4r5WpLUMcQ8PVmnv1vVrs/3Wy/Rb foxebZNWxgUiSx+d02e3Ad0aEIur1SYXXv71mqKwyi/40CBSHq2jk9eF6zmEhaoFi5+MMMgX X0i+fcBkvmT0N88W4yCtHhHQds+RDbTPLGm8NBVJb7R5zbJmuQX7ADBVuNYIU8hx3dF3AQCm 601w0oZJ0jGOV1vXQgHqZYJGHg5wuImhzhZJCRESIwf+PJxik7TJOgBicko1hUVOxJBZxoe0 x+/SO6tn+s8wKlR1Yxy8gYN9ZRqV2I83JsWZbBXMG1kLzV0SAfk/wq0PAppA1VzrQ3JqXg7T MZ3tFgxvxkYqUP11tO2vrgys+InkZAfjBVMjqXWHokyQPpihUaW0a8mr40w9Qui6DoJj7+Gg DtDWDZ7Zcn2hoyrypuht88rUuh1JuGYD434Q6qwQjUDlY+4lgrUxKdMD8R7JJWt38MNlTWvy rMVscvZUNc7gxcmnFUn41NPSKqzp4DDRbmf37Iz/fL7i01y7IGFTXaYaF3nEACyIUTr/xxi+ MD1FVtEtJncZNkRn7WBcVFGKMAf+NEeaeQdGYQ6mGgk++i/vJZxkrC/a9ZXme7BhWRP485U5 sXpFoGjdpMn4VlC7TFk2qsnJi3yF0pXCKVRy1ukEls8o+4PF2JiKrtkCrWCimB6jxGPIG3lk 3SuKVS/din3RHz+7Sr1lXWFcGYDENmPd/jTwr1A1FiHrSj+u21hnJEHi8eTa9029F1KRfocp ig+k0zUEKmFPDabpanI323O5Tahsy7hwf2WOQwTDLvQ+eqQu40wbb6NocmCNFjtRhNZWGKJS b5GrGDGu/No5U6w73adighEuNcCSNBsLyUe48CE0uTO7eAL6Vd+2k28ezi6XY4Y0mgASJslb NwW54LzSSM0uRGFuaWVsIFAuIFNtaXRoIDxkcHNtaXRoQGFwZXJ0dXNzb2x1dGlvbnMuY29t PsJ6BBMRCAAiBQJWK7ngAhsjBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRBTc6WbYpR8 KrQ9AP94+xjtFfJ8gj5c7PVx06Zv9rcmFUqQspZ5wSEkvxOuQQEAg6qEsPYegI7iByLVzNEg 7B7fUG7pqWIfMqFwFghYhQzOwU0EViu54BAIAL6MXXNlrJ5tRUf+KMBtVz1LJQZRt/uxWrCb T06nZjnbp2UcceuYNbISOVHGXTzu38r55YzpkEA8eURQf+5hjtvlrOiHxvpD+Z6WcpV6rrMB kcAKWiZTQihW2HoGgVB3gwG9dCh+n0X5OzliAMiGK2a5iqnIZi3o0SeW6aME94bSkTkuj6/7 OmH9KAzK8UnlhfkoMg3tXW8L6/5CGn2VyrjbB/rcrbIR4mCQ+yCUlocuOjFCJhBd10AG1IcX OXUa/ux+/OAV9S5mkr5Fh3kQxYCTcTRt8RY7+of9RGBk10txi94dXiU2SjPbassvagvu/hEi twNHms8rpkSJIeeq0/cAAwUH/jV3tXpaYubwcL2tkk5ggL9Do+/Yo2WPzXmbp8vDiJPCvSJW rz2NrYkd/RoX+42DGqjfu8Y04F9XehN1zZAFmCDUqBMa4tEJ7kOT1FKJTqzNVcgeKNBGcT7q 27+wsqbAerM4A0X/F/ctjYcKwNtXck1Bmd/T8kiw2IgyeOC+cjyTOSwKJr2gCwZXGi5g+2V8 NhJ8n72ISPnOh5KCMoAJXmCF+SYaJ6hIIFARmnuessCIGw4ylCRIU/TiXK94soilx5aCqb1z ke943EIUts9CmFAHt8cNPYOPRd20pPu4VFNBuT4fv9Ys0iv0XGCEP+sos7/pgJ3gV3pCOric p15jV4PCYQQYEQgACQUCViu54AIbDAAKCRBTc6WbYpR8Khu7AP9NJrBUn94C/3PeNbtQlEGZ NV46Mx5HF0P27lH3sFpNrwD/dVdZ5PCnHQYBZ287ZxVfVr4Zuxjo5yJbRjT93Hl0vMY=
  • Cc: Jason Andryuk <jason.andryuk@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • Delivery-date: Fri, 12 Apr 2024 11:47:19 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 4/11/24 15:49, Andrew Cooper wrote:
On 11/04/2024 4:25 pm, Daniel P. Smith wrote:
Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
---
  xen/common/gzip/gunzip.c  |  2 ++
  xen/common/gzip/inflate.c | 26 ++++++++++++--------------
  2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index a1b516b925c9..79a641263597 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -31,6 +31,8 @@ struct gzip_data {
unsigned long crc_32_tab[256];
      unsigned long crc;
+
+    unsigned hufts;        /* track memory usage */
  };
#define OF(args) args
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 6c8c7452a31f..53ee1d8ce1e3 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -140,7 +140,7 @@ struct huft {
  };
/* Function prototypes */
-static int huft_build OF((unsigned *, unsigned, unsigned,
+static int huft_build OF((struct gzip_data *, unsigned *, unsigned, unsigned,
                            const ush *, const ush *, struct huft **, int *));
  static int huft_free OF((struct huft *));
  static int inflate_codes OF((struct gzip_data *, struct huft *, struct huft 
*, int, int));
@@ -311,8 +311,6 @@ static const int dbits = 6;          /* bits in base 
distance lookup table */
  #define BMAX 16         /* maximum bit length of any code (16 for explode) */
  #define N_MAX 288       /* maximum number of codes in any set */
-static unsigned __initdata hufts; /* track memory usage */
-
  /*
   * Given a list of code lengths and a maximum table size, make a set of
   * tables to decode that set of codes.  Return zero on success, one if
@@ -329,8 +327,8 @@ static unsigned __initdata hufts;      /* track memory 
usage */
   * @param m Maximum lookup bits, returns actual
   */
  static int __init huft_build(
-    unsigned *b, unsigned n, unsigned s, const ush *d, const ush *e,
-    struct huft **t, int *m)
+    struct gzip_data *gd, unsigned *b, unsigned n, unsigned s, const ush *d,
+    const ush *e, struct huft **t, int *m)
  {
      unsigned a;                   /* counter for codes of length k */
      unsigned f;                   /* i repeats in table every f entries */
@@ -492,7 +490,7 @@ static int __init huft_build(
                      goto out;
                  }
                  DEBG1("4 ");
-                hufts += z + 1;         /* track memory usage */
+                gd->hufts += z + 1;         /* track memory usage */
                  *t = q + 1;             /* link to list for huft_free() */
                  *(t = &(q->v.t)) = (struct huft *)NULL;
                  u[h] = ++q;             /* table starts after link */
@@ -787,7 +785,7 @@ static int noinline __init inflate_fixed(struct gzip_data 
*gd)
      for (; i < 288; i++)          /* make a complete, but wrong code set */
          l[i] = 8;
      bl = 7;
-    if ((i = huft_build(l, 288, 257, cplens, cplext, &tl, &bl)) != 0) {
+    if ((i = huft_build(gd, l, 288, 257, cplens, cplext, &tl, &bl)) != 0) {
          free(l);
          return i;
      }
@@ -796,7 +794,7 @@ static int noinline __init inflate_fixed(struct gzip_data 
*gd)
      for (i = 0; i < 30; i++)      /* make an incomplete code set */
          l[i] = 5;
      bd = 5;
-    if ((i = huft_build(l, 30, 0, cpdist, cpdext, &td, &bd)) > 1)
+    if ((i = huft_build(gd, l, 30, 0, cpdist, cpdext, &td, &bd)) > 1)
      {
          huft_free(tl);
          free(l);
@@ -894,7 +892,7 @@ static int noinline __init inflate_dynamic(struct gzip_data 
*gd)
/* build decoding table for trees--single level, 7 bit lookup */
      bl = 7;
-    if ((i = huft_build(ll, 19, 19, NULL, NULL, &tl, &bl)) != 0)
+    if ((i = huft_build(gd, ll, 19, 19, NULL, NULL, &tl, &bl)) != 0)
      {
          if (i == 1)
              huft_free(tl);
@@ -971,7 +969,7 @@ static int noinline __init inflate_dynamic(struct gzip_data 
*gd)
/* build the decoding tables for literal/length and distance codes */
      bl = lbits;
-    if ((i = huft_build(ll, nl, 257, cplens, cplext, &tl, &bl)) != 0)
+    if ((i = huft_build(gd, ll, nl, 257, cplens, cplext, &tl, &bl)) != 0)
      {
          DEBG("dyn5b ");
          if (i == 1) {
@@ -983,7 +981,7 @@ static int noinline __init inflate_dynamic(struct gzip_data 
*gd)
      }
      DEBG("dyn5c ");
      bd = dbits;
-    if ((i = huft_build(ll + nl, nd, 0, cpdist, cpdext, &td, &bd)) != 0)
+    if ((i = huft_build(gd, ll + nl, nd, 0, cpdist, cpdext, &td, &bd)) != 0)
      {
          DEBG("dyn5d ");
          if (i == 1) {
@@ -1090,15 +1088,15 @@ static int __init inflate(struct gzip_data *gd)
      /* decompress until the last block */
      h = 0;
      do {
-        hufts = 0;
+        gd->hufts = 0;
  #ifdef ARCH_HAS_DECOMP_WDOG
          arch_decomp_wdog();
  #endif
          r = inflate_block(gd, &e);
          if (r)
              return r;
-        if (hufts > h)
-            h = hufts;
+        if (gd->hufts > h)
+            h = gd->hufts;
      } while (!e);
/* Undo too much lookahead. The next read will be byte aligned so we


AFAICT, hothing in inflate() reads h.  So hufts is a write-only variable?

Good question, let me study it to make sure.

Can't we just delete it, rather than plumb it through into the state
block?  It would certainly shrink this patch somewhat.

Yes, if I can't find any reads of gd->hufts, directly or indirectly intermediate variables like h above, then yes, I can try dropping it and testing to see if anything breaks.

v/r,
dps




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.