grub-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] btrfs: disable zstd support for i386-pc


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH] btrfs: disable zstd support for i386-pc
Date: Wed, 6 Nov 2019 21:08:41 -0800



On Wed, 6 Nov 2019, 20:55 Michael Chang, <address@hidden> wrote:
On Wed, Nov 06, 2019 at 11:15:04AM -0800, Vladimir 'phcoder' Serbinenko wrote:
> Please don't do it this way. The right solution is to move it to separate
> module and include zstd module when needed.

I fully agree to work out a proper solution, but before that I think it
is necessary to stop it from spread out going forward, as the running
system upgrading to new version could be affected and the consequence is
fatal - an unbootable system, qualified to be show stopper imho.
We don't do a release right now, so I don't think it's justified. Also increase in size can easily come from a compiler. If an increase in size can make system unbootable on upgrade, I'd rather be fixing this than just pepper over it.

> Not everybody uses btrfs
> embedded area. I recommend not to use it. Using mbr gap or BBP is the
> recommended way.

The problem here is existing installation would fail to update to new
grub version if it is using btrfs bootloader area. For new installation
certainly we could recommend them to do so. or btrfs zstd support will
be disabled if using a btrfs bootloader area.


Thanks,
Michael

>
> On Tue, 5 Nov 2019, 01:25 Michael Chang, <address@hidden> wrote:
>
> > The zstd support in btrfs has dependenciy to zstd module and core.img
> > grows its size significantly to 75KB on my system. The resulted image
> > cannot be installed into btrfs bootloader area in the size of 64KB and
> > eventually fails with following message.
> >
> > /usr/sbin/grub-install: warning: your core.img is unusually large.  It
> > won't fit in the embedding area.
> > /usr/sbin/grub-install: error: filesystem `btrfs' doesn't support
> > blocklists.
> >
> > The patch disabled the zstd support of btrfs in pc-bios platform to
> > avoid the regression. The resulting size is 56KB, albeit a bit too close
> > to the 64KB but works. This is simple workaround until a proper fix
> > landed upstream.
> >
> > Signed-off-by: Michael Chang <address@hidden>
> > ---
> >  grub-core/fs/btrfs.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> > index 48bd3d04a..8f98892d3 100644
> > --- a/grub-core/fs/btrfs.c
> > +++ b/grub-core/fs/btrfs.c
> > @@ -17,6 +17,7 @@
> >   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> >   */
> >
> > +#ifndef GRUB_MACHINE_PCBIOS
> >  /*
> >   * Tell zstd to expose functions that aren't part of the stable API, which
> >   * aren't safe to use when linking against a dynamic library. We vendor
> > in a
> > @@ -24,6 +25,7 @@
> >   * functions to provide our own allocator, which uses grub_malloc(), to
> > zstd.
> >   */
> >  #define ZSTD_STATIC_LINKING_ONLY
> > +#endif
> >
> >  #include <grub/err.h>
> >  #include <grub/file.h>
> > @@ -35,7 +37,9 @@
> >  #include <grub/lib/crc.h>
> >  #include <grub/deflate.h>
> >  #include <minilzo.h>
> > +#ifndef GRUB_MACHINE_PCBIOS
> >  #include <zstd.h>
> > +#endif
> >  #include <grub/i18n.h>
> >  #include <grub/btrfs.h>
> >  #include <grub/crypto.h>
> > @@ -56,8 +60,10 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >  #define GRUB_BTRFS_LZO_BLOCK_MAX_CSIZE (GRUB_BTRFS_LZO_BLOCK_SIZE + \
> >                                      (GRUB_BTRFS_LZO_BLOCK_SIZE / 16) + 64
> > + 3)
> >
> > +#ifndef GRUB_MACHINE_PCBIOS
> >  #define ZSTD_BTRFS_MAX_WINDOWLOG 17
> >  #define ZSTD_BTRFS_MAX_INPUT     (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
> > +#endif
> >
> >  typedef grub_uint8_t grub_btrfs_checksum_t[0x20];
> >  typedef grub_uint16_t grub_btrfs_uuid_t[8];
> > @@ -229,7 +235,9 @@ struct grub_btrfs_extent_data
> >  #define GRUB_BTRFS_COMPRESSION_NONE 0
> >  #define GRUB_BTRFS_COMPRESSION_ZLIB 1
> >  #define GRUB_BTRFS_COMPRESSION_LZO  2
> > +#ifndef GRUB_MACHINE_PCBIOS
> >  #define GRUB_BTRFS_COMPRESSION_ZSTD 3
> > +#endif
> >
> >  #define GRUB_BTRFS_OBJECT_ID_CHUNK 0x100
> >
> > @@ -1229,6 +1237,7 @@ grub_btrfs_read_inode (struct grub_btrfs_data *data,
> >    return grub_btrfs_read_logical (data, elemaddr, inode, sizeof (*inode),
> > 0);
> >  }
> >
> > +#ifndef GRUB_MACHINE_PCBIOS
> >  static void *grub_zstd_malloc (void *state __attribute__((unused)),
> > size_t size)
> >  {
> >    return grub_malloc (size);
> > @@ -1318,6 +1327,7 @@ err:
> >
> >    return ret;
> >  }
> > +#endif
> >
> >  static grub_ssize_t
> >  grub_btrfs_lzo_decompress(char *ibuf, grub_size_t isize, grub_off_t off,
> > @@ -1494,8 +1504,12 @@ grub_btrfs_extent_read (struct grub_btrfs_data
> > *data,
> >
> >        if (data->extent->compression != GRUB_BTRFS_COMPRESSION_NONE
> >           && data->extent->compression != GRUB_BTRFS_COMPRESSION_ZLIB
> > +#ifndef GRUB_MACHINE_PCBIOS
> >           && data->extent->compression != GRUB_BTRFS_COMPRESSION_LZO
> >           && data->extent->compression != GRUB_BTRFS_COMPRESSION_ZSTD)
> > +#else
> > +         && data->extent->compression != GRUB_BTRFS_COMPRESSION_LZO)
> > +#endif
> >         {
> >           grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> >                       "compression type 0x%x not supported",
> > @@ -1535,6 +1549,7 @@ grub_btrfs_extent_read (struct grub_btrfs_data *data,
> >                   != (grub_ssize_t) csize)
> >                 return -1;
> >             }
> > +#ifndef GRUB_MACHINE_PCBIOS
> >           else if (data->extent->compression ==
> > GRUB_BTRFS_COMPRESSION_ZSTD)
> >             {
> >               if (grub_btrfs_zstd_decompress (data->extent->inl,
> > data->extsize -
> > @@ -1544,6 +1559,7 @@ grub_btrfs_extent_read (struct grub_btrfs_data *data,
> >                   != (grub_ssize_t) csize)
> >                 return -1;
> >             }
> > +#endif
> >           else
> >             grub_memcpy (buf, data->extent->inl + extoff, csize);
> >           break;
> > @@ -1581,10 +1597,12 @@ grub_btrfs_extent_read (struct grub_btrfs_data
> > *data,
> >                 ret = grub_btrfs_lzo_decompress (tmp, zsize, extoff
> >                                     + grub_le_to_cpu64
> > (data->extent->offset),
> >                                     buf, csize);
> > +#ifndef GRUB_MACHINE_PCBIOS
> >               else if (data->extent->compression ==
> > GRUB_BTRFS_COMPRESSION_ZSTD)
> >                 ret = grub_btrfs_zstd_decompress (tmp, zsize, extoff
> >                                     + grub_le_to_cpu64
> > (data->extent->offset),
> >                                     buf, csize);
> > +#endif
> >               else
> >                 ret = -1;
> >
> > --
> > 2.16.4
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >

> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel


_______________________________________________
Grub-devel mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/grub-devel

reply via email to

[Prev in Thread] Current Thread [Next in Thread]