Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Apr 2023 16:01:47 -0700
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
Message-ID: <ZDnbW1qYmBLycefL@google.com>
References: <CAFDdnB1Rq3vNe_qt_0u+inzOuL4vrGhgbOoQZKBwfBktni=Npw@mail.gmail.com>
 <YpLLTkje/QUYPP9z@mit.edu>
 <YpNk4bQlRKmgDw8E@mit.edu>
 <CAFDdnB0a3mfcoY7rg5N4dO13qMeSsV+PkA2YHeerEOFRv8484Q@mail.gmail.com>
 <YpQixl+ljcC1VHNU@mit.edu>
 <CAFDdnB1WxrqddeLJwjsqqgoij1q_QGa=SBs-i=j31W2QbksJ=Q@mail.gmail.com>
 <YpVeeKRH1bycP7P1@mit.edu>
 <YpVxYchs1wScNRDw@mit.edu>
 <CAFDdnB1KJVSXXzXKOc+T+g1Qewr11AS4f9tFJqSMLvfpiX-5Lg@mail.gmail.com>
 <YpjNf8WGfYh31F+2@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YpjNf8WGfYh31F+2@mit.edu>
Precedence: bulk

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);
        }
 }