Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-1115-g8b801eadce-fm-20221102.001-g8b801ead
Mime-Version: 1.0
Message-Id: <d6cf3090-ac0c-4627-b3cb-268fd213056f@app.fastmail.com>
In-Reply-To: <CAHJ8P3JHVu=Qy0ft9Sorq2Zxvu7whFV72OU+NVMhvK8SK6+6MA@mail.gmail.com>
References: <1667889638-9106-1-git-send-email-zhiguo.niu@unisoc.com>
 <202211101659.j9z9jLHv-lkp@intel.com>
 <88fce16b-5092-4246-8bbf-23f2c03224f3@app.fastmail.com>
 <CAHJ8P3+g7dr3PBZFQCD5HQLZ2b0WHe=b6Jt7ha1o7mqZJ7_-BQ@mail.gmail.com>
 <97c529f2-c5a2-4ed6-89eb-c79f020e9d0c@app.fastmail.com>
 <CAHJ8P3KU4NBr9ftqp1J_QWxGUjusoUZo6JjefPN-4YazD4mrUQ@mail.gmail.com>
 <cd818e0c-4c8c-421e-ae39-c7cea79eeb7e@app.fastmail.com>
 <CAHJ8P3JHVu=Qy0ft9Sorq2Zxvu7whFV72OU+NVMhvK8SK6+6MA@mail.gmail.com>
Date:   Mon, 14 Nov 2022 11:17:48 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Zhiguo Niu" <niuzhiguo84@gmail.com>
Cc:     "kernel test robot" <lkp@intel.com>,
        "zhiguo.niu" <zhiguo.niu@unisoc.com>, jaegeuk@kernel.org,
        chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
        oe-kbuild-all@lists.linux.dev
Subject: Re: [PATCH V2] f2fs: fix atgc bug on issue in 32bits platform
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Nov 14, 2022, at 10:23, Zhiguo Niu wrote:
> Arnd Bergmann <arnd@arndb.de> =E4=BA=8E2022=E5=B9=B411=E6=9C=8811=E6=97=
=A5=E5=91=A8=E4=BA=94 17:57=E5=86=99=E9=81=93=EF=BC=9A

> so I just modify struct victim_entry as your suggestion:
> diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
> index 19b956c2d697..e2f25b8fd865 100644
> --- a/fs/f2fs/gc.h
> +++ b/fs/f2fs/gc.h
> @@ -56,16 +56,16 @@ struct gc_inode_list {
>  };
>=20
>  struct victim_info {
> - unsigned long long mtime; /* mtime of section */
> - unsigned int segno; /* section No. */
> + unsigned long long mtime __packed; /* mtime of section */
> + unsigned int segno; /* section No. */
>  };
>=20
>  struct victim_entry {
>   struct rb_node rb_node; /* rb node located in rb-tree */
>   union {
>   struct {
> - unsigned long long mtime; /* mtime of section */
> - unsigned int segno; /* segment No. */
> + unsigned long long mtime __packed; /* mtime of section */
> + unsigned int segno; /* segment No. */
>   };
>   struct victim_info vi; /* victim info */
>   };

Right, this should work. I'm still unsure where you need
a union inside of victim_entry rather than just having the
'victim_info' portion in there by itself, but that is not
a matter of correctness.

> There is no problem with functional verification in both 64bit and=20
> 32bit platforms,=20
> sorry I don't have the environment to verify is  build warnig reported=20
> by the kernel test robot still there.

I had a bit trouble reproducing this as well. It looks like this
only happens when -Wunaligned-access is enabled for a config, but
that requires two things:

- building with CC=3Dclang for a target architecture that does
  not support unaligned access natively, such as ARMv5.
  ARMv7 is interesting because it disables the warning, though
  it only supports unaligned load/store on 32-bit and 16-bit
  words but not 64-bit words using the ldrd/strd instructions.

- Even on architectures with no unaligned load/store, the
  warning is disabled by default unless you use "make W=3D1" to
  enable extra warnings.

Alternatively, you can enable the warning manually by passing
"CFLAGS_MODULE=3D-Wunaligned-access" to make, which should trigger
the warning on any 32-bit architecture.

      Arnd