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: <cd818e0c-4c8c-421e-ae39-c7cea79eeb7e@app.fastmail.com>
In-Reply-To: <CAHJ8P3KU4NBr9ftqp1J_QWxGUjusoUZo6JjefPN-4YazD4mrUQ@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>
Date:   Fri, 11 Nov 2022 10:57:21 +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 Fri, Nov 11, 2022, at 10:40, Zhiguo Niu wrote:
> Arnd Bergmann <arnd@arndb.de> =E4=BA=8E2022=E5=B9=B411=E6=9C=8810=E6=97=
=A5=E5=91=A8=E5=9B=9B 21:45=E5=86=99=E9=81=93=EF=BC=9A
>   I thinks the gcc complier build warning :
> ----------------------------------------------------------------
>     In file included from fs/f2fs/segment.c:24:
>>> fs/f2fs/gc.h:73:1: warning: alignment 1 of 'struct victim_entry' is=20
>
>>> less than 8 [-Wpacked-not-aligned]
>
>       73 | } __packed;
>
>          | ^
>
>  ---------------------------------------------------------------
>
> It is because struct rb_node has the attribute=20
> "__attribute__((aligned(sizeof(long)", it is 8 bytes in 64bits platfor=
m.
>
> struct rb_node {
> unsigned long  __rb_parent_color;
> struct rb_node *rb_right;
> struct rb_node *rb_left;
> } __attribute__((aligned(sizeof(long))));
>
> so I just try to put __packed on union of struct victim_entry and i=20
> also keep consistent with struct rb_entry.

No, that attribute has no effect on any architecture other
than m68k, which defaults to 16-bit alignment for 32-bit
members. I'm fairly sure the alignment attribute on
rb_node is entirely unrelated to the problems you are
seeing in f2fs that come from having a structure with
stricter (4 byte or 8 byte) alignment requirements embedded
in a structure with relaxed (single-byte) alignment:

> struct rb_entry {
> struct rb_node rb_node; /* rb node located in rb-tree */
> union {
> struct {
> unsigned int ofs; /* start offset of the entry */
> unsigned int len; /* length of the entry */
> };
> unsigned long long key; /* 64-bits key */
> } __packed;
> };

This tells the compiler that the anonymous union is=20
entirely unconstrained, but the anonymous struct inside
it has the default alignment, which is the contradition
that gcc correctly warns about.

Since the only thing you need here is to lower the
alignment constraint from 8 bytes to 4 bytes, the easiest
way is to have the __packed annotation on the 'key'
member. This avoids all warnings, as long you do not
take the address of the 'key' member and pass it to
a function that expects an aligned 'u64' pointer.

     Arnd