All of lore.kernel.org
 help / color / mirror / Atom feed
From: William McVicker <willmcvicker@google.com>
To: Theodore Ts'o <tytso@mit.edu>, hch@lst.de, linux-ext4@vger.kernel.org
Cc: "Stephen E. Baker" <baker.stephen.e@gmail.com>,
	adilger.kernel@dilger.ca, hch@lst.de, linux-ext4@vger.kernel.org
Subject: Re: simplify ext4_sb_read_encoding regression
Date: Fri, 14 Apr 2023 16:01:47 -0700	[thread overview]
Message-ID: <ZDnbW1qYmBLycefL@google.com> (raw)
In-Reply-To: <YpjNf8WGfYh31F+2@mit.edu>

On 06/02/2022, Theodore Ts'o wrote:
> On Wed, Jun 01, 2022 at 10:06:04PM -0400, Stephen E. Baker wrote:
> > On Mon, May 30, 2022 at 9:37 PM Theodore Ts'o <tytso@mit.edu> wrote:
> > > > I don't know what to tell you.  I took your config, stripped out all
> > > > of the modules, and enabled CONFIG_HYPERVISOR_GUEST,
> > > > CONFIG_VIRTIO_MENU, and CONFIG_VIRTIO_BLK, and build a 5.16 kernel.
> > >
> > Maybe a silly question, but how do I enable CONFIG_HYPERVISOR_GUEST with
> > this config.
> 
> So let's make things easy.  Attached please find my minimal config.
> This is what I use when I normally build a test kernels, and I get it
> by running "kvm-xfstests install-kconfig".  I use it because it's fast
> to build, since it doesn't build extraneous stuff.  I've also attached
> the "seb-config", which is your configuration with the minimal changes
> needed so it can run under qemu.  The compressed size is twice as big,
> and it takes 2-3 times longer to build.
> 
> I can't reproduce the problem you are seeing using kvm-xfstests using
> either kernel config building on 5.17.
> 
> 					- Ted


Hi,

I believe I figured out what is going on here since I am hitting a similar
issue with CONFIG_UNICODE. If you take a look at the .config from Stephen's
message, you'll see that he sets:

  CONFIG_TRIM_UNUSED_KSYMS=y
  CONFIG_UNUSED_KSYMS_WHITELIST=""

When trimming is enabled, kbuild will strip all exported symbols that are not
listed in the whitelist. As a result, when utf8-core.c calls:

  um->tables = symbol_request(utf8_data_table);

it will fail since `utf8_data_table` doesn't exist in the exported section of
the kernel symbol table. For me on Android, this leads to the userdata
partition failing to mount. To be clear, this happens when CONFIG_UNICODE=y.

One question I have is -- Why are we using symbol_request()/symbol_put() when
`utf8_data_table` is exported? Why can't we directly reference the
`utf8_data_table` symbol?

If we need to use symbol_request() when CONFIG_UNICODE=m, then can we apply the
below patch to fix this when CONFIG_UNICODE=y? I have verified this works for
me with CONFIG_UNICODE=y and CONFIG_TRIM_UNUSED_KSYMS=y.

Thanks,
Will


diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
index 67aaadc3ab072..1631bffa51b2f 100644
--- a/fs/unicode/utf8-core.c
+++ b/fs/unicode/utf8-core.c
@@ -181,9 +181,15 @@ struct unicode_map *utf8_load(unsigned int version)
                return ERR_PTR(-ENOMEM);
        um->version = version;
 
+#if defined(CONFIG_UNICODE_MODULE)
        um->tables = symbol_request(utf8_data_table);
-       if (!um->tables)
+#else
+       um->tables = &utf8_data_table;
+#endif
+       if (!um->tables) {
+               pr_err("%s: WILL: Failed to find utf8_data_table symbol!\n", __func__);
                goto out_free_um;
+       }
 
        if (!utf8version_is_supported(um, version))
                goto out_symbol_put;
@@ -198,7 +204,9 @@ struct unicode_map *utf8_load(unsigned int version)
        return um;
 
 out_symbol_put:
+#if defined(CONFIG_UNICODE_MODULE)
        symbol_put(um->tables);
+#endif
 out_free_um:
        kfree(um);
        return ERR_PTR(-EINVAL);
@@ -208,7 +216,9 @@ EXPORT_SYMBOL(utf8_load);
 void utf8_unload(struct unicode_map *um)
 {
        if (um) {
+#if defined(CONFIG_UNICODE_MODULE)
                symbol_put(utf8_data_table);
+#endif
                kfree(um);
        }
 }


  reply	other threads:[~2023-04-14 23:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAFDdnB1Rq3vNe_qt_0u+inzOuL4vrGhgbOoQZKBwfBktni=Npw@mail.gmail.com>
2022-05-29  1:24 ` simplify ext4_sb_read_encoding regression Theodore Ts'o
2022-05-29 12:19   ` Theodore Ts'o
2022-05-29 22:49     ` Stephen E. Baker
2022-05-30  1:49       ` Theodore Ts'o
2022-05-30 22:20         ` Stephen E. Baker
2022-05-31  0:16           ` Theodore Ts'o
2022-05-31  1:37             ` Theodore Ts'o
2022-06-02  2:06               ` Stephen E. Baker
2022-06-02 14:47                 ` Theodore Ts'o
2023-04-14 23:01                   ` William McVicker [this message]
2023-04-16  5:47                     ` Christoph Hellwig
2023-04-16  5:56                       ` Christoph Hellwig
2023-04-17  3:26                         ` Theodore Ts'o
2023-04-17 16:16                           ` Will McVicker
2022-05-28 22:55 Stephen E. Baker
2022-05-30 14:21 ` Gabriel Krisman Bertazi
2022-05-30 22:27   ` Stephen E. Baker
2022-05-31  0:56     ` Dave Chinner
2022-05-31  2:39       ` Stephen E. Baker
2022-06-02 17:47     ` Gabriel Krisman Bertazi
2022-06-05  0:11       ` Stephen E. Baker
2022-06-05  3:21         ` Stephen E. Baker
2022-06-05  6:23           ` Gabriel Krisman Bertazi
2022-06-05 13:56             ` Stephen E. Baker

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=ZDnbW1qYmBLycefL@google.com \
    --to=willmcvicker@google.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=baker.stephen.e@gmail.com \
    --cc=hch@lst.de \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.