[PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings

Gustavo A. R. Silva posted 1 patch 7 months, 1 week ago
fs/notify/fanotify/fanotify.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
Posted by Gustavo A. R. Silva 7 months, 1 week ago
-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

Modify FANOTIFY_INLINE_FH() macro, which defines a struct containing a
flexible-array member in the middle (struct fanotify_fh::buf), to use
struct_size_t() to pre-allocate space for both struct fanotify_fh and
its flexible-array member. Replace the struct with a union and relocate
the flexible structure (struct fanotify_fh) to the end.

See the memory layout of struct fanotify_fid_event before and after
changes below.

pahole -C fanotify_fid_event fs/notify/fanotify/fanotify.o

BEFORE:
struct fanotify_fid_event {
        struct fanotify_event      fae;                  /*     0    48 */
        __kernel_fsid_t            fsid;                 /*    48     8 */
        struct {
                struct fanotify_fh object_fh;            /*    56     4 */
                unsigned char      _inline_fh_buf[12];   /*    60    12 */
        };                                               /*    56    16 */

        /* size: 72, cachelines: 2, members: 3 */
        /* last cacheline: 8 bytes */
};

AFTER:
struct fanotify_fid_event {
        struct fanotify_event      fae;                  /*     0    48 */
        __kernel_fsid_t            fsid;                 /*    48     8 */
        union {
                unsigned char      _inline_fh_buf[16];   /*    56    16 */
                struct fanotify_fh object_fh __attribute__((__aligned__(1))); /*    56     4 */
        } __attribute__((__aligned__(1)));               /*    56    16 */

        /* size: 72, cachelines: 2, members: 3 */
        /* forced alignments: 1 */
        /* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)));

So, with these changes, fix the following warnings:

fs/notify/fanotify/fanotify.h:317:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
fs/notify/fanotify/fanotify.h:289:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 fs/notify/fanotify/fanotify.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index b44e70e44be6..91c26b1c1d32 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -275,12 +275,12 @@ static inline void fanotify_init_event(struct fanotify_event *event,
 	event->pid = NULL;
 }
 
-#define FANOTIFY_INLINE_FH(name, size)					\
-struct {								\
-	struct fanotify_fh name;					\
-	/* Space for object_fh.buf[] - access with fanotify_fh_buf() */	\
-	unsigned char _inline_fh_buf[size];				\
-}
+#define FANOTIFY_INLINE_FH(name, size)						      \
+union {										      \
+	/* Space for object_fh and object_fh.buf[] - access with fanotify_fh_buf() */ \
+	unsigned char _inline_fh_buf[struct_size_t(struct fanotify_fh, buf, size)];   \
+	struct fanotify_fh name;						      \
+} __packed
 
 struct fanotify_fid_event {
 	struct fanotify_event fae;
-- 
2.43.0
Re: [PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
Posted by kernel test robot 7 months, 1 week ago
Hi Gustavo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jack-fs/fsnotify]
[also build test WARNING on linus/master v6.15-rc5 next-20250507]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gustavo-A-R-Silva/fanotify-Avoid-a-couple-of-Wflex-array-member-not-at-end-warnings/20250507-074110
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link:    https://lore.kernel.org/r/aBqdlxlBtb9s7ydc%40kspp
patch subject: [PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20250508/202505081249.CUDWsu7Z-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081249.CUDWsu7Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505081249.CUDWsu7Z-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from fs/notify/fdinfo.c:17:
>> fs/notify/fanotify/fanotify.h:280:16: warning: alignment 1 of 'union <anonymous>' is less than 4 [-Wpacked-not-aligned]
     280 |         struct fanotify_fh name;                                                      \
         |                ^~~~~~~~~~~
   fs/notify/fanotify/fanotify.h:287:9: note: in expansion of macro 'FANOTIFY_INLINE_FH'
     287 |         FANOTIFY_INLINE_FH(object_fh, FANOTIFY_INLINE_FH_LEN);
         |         ^~~~~~~~~~~~~~~~~~
>> fs/notify/fanotify/fanotify.h:280:16: warning: alignment 1 of 'union <anonymous>' is less than 4 [-Wpacked-not-aligned]
     280 |         struct fanotify_fh name;                                                      \
         |                ^~~~~~~~~~~
   fs/notify/fanotify/fanotify.h:315:9: note: in expansion of macro 'FANOTIFY_INLINE_FH'
     315 |         FANOTIFY_INLINE_FH(object_fh, MAX_HANDLE_SZ);
         |         ^~~~~~~~~~~~~~~~~~


vim +280 fs/notify/fanotify/fanotify.h

b8a6c3a2f0ae4d Amir Goldstein          2020-07-08  275  
2c5069433a3adc Gabriel Krisman Bertazi 2021-10-25  276  #define FANOTIFY_INLINE_FH(name, size)						      \
e3725b8a2ecdf6 Gustavo A. R. Silva     2025-05-06  277  union {										      \
e3725b8a2ecdf6 Gustavo A. R. Silva     2025-05-06  278  	/* Space for object_fh and object_fh.buf[] - access with fanotify_fh_buf() */ \
e3725b8a2ecdf6 Gustavo A. R. Silva     2025-05-06  279  	unsigned char _inline_fh_buf[struct_size_t(struct fanotify_fh, buf, size)];   \
1758cd2e95d31b Alexey Dobriyan         2023-10-10 @280  	struct fanotify_fh name;						      \
e3725b8a2ecdf6 Gustavo A. R. Silva     2025-05-06  281  } __packed
2c5069433a3adc Gabriel Krisman Bertazi 2021-10-25  282  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
Posted by Amir Goldstein 7 months, 1 week ago
On Wed, May 7, 2025 at 1:39 AM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
>
> Modify FANOTIFY_INLINE_FH() macro, which defines a struct containing a
> flexible-array member in the middle (struct fanotify_fh::buf), to use
> struct_size_t() to pre-allocate space for both struct fanotify_fh and
> its flexible-array member. Replace the struct with a union and relocate
> the flexible structure (struct fanotify_fh) to the end.
>
> See the memory layout of struct fanotify_fid_event before and after
> changes below.
>
> pahole -C fanotify_fid_event fs/notify/fanotify/fanotify.o
>
> BEFORE:
> struct fanotify_fid_event {
>         struct fanotify_event      fae;                  /*     0    48 */
>         __kernel_fsid_t            fsid;                 /*    48     8 */
>         struct {
>                 struct fanotify_fh object_fh;            /*    56     4 */
>                 unsigned char      _inline_fh_buf[12];   /*    60    12 */
>         };                                               /*    56    16 */
>
>         /* size: 72, cachelines: 2, members: 3 */
>         /* last cacheline: 8 bytes */
> };
>
> AFTER:
> struct fanotify_fid_event {
>         struct fanotify_event      fae;                  /*     0    48 */
>         __kernel_fsid_t            fsid;                 /*    48     8 */
>         union {
>                 unsigned char      _inline_fh_buf[16];   /*    56    16 */
>                 struct fanotify_fh object_fh __attribute__((__aligned__(1))); /*    56     4 */

I'm not that familiar with pahole, but I find it surprising to see this member
aligned(1), when struct fanotify_fh is defined as __aligned(4).

>         } __attribute__((__aligned__(1)));               /*    56    16 */
>
>         /* size: 72, cachelines: 2, members: 3 */
>         /* forced alignments: 1 */
>         /* last cacheline: 8 bytes */
> } __attribute__((__aligned__(8)));
>
> So, with these changes, fix the following warnings:
>
> fs/notify/fanotify/fanotify.h:317:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/notify/fanotify/fanotify.h:289:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  fs/notify/fanotify/fanotify.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index b44e70e44be6..91c26b1c1d32 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -275,12 +275,12 @@ static inline void fanotify_init_event(struct fanotify_event *event,
>         event->pid = NULL;
>  }
>
> -#define FANOTIFY_INLINE_FH(name, size)                                 \
> -struct {                                                               \
> -       struct fanotify_fh name;                                        \
> -       /* Space for object_fh.buf[] - access with fanotify_fh_buf() */ \
> -       unsigned char _inline_fh_buf[size];                             \
> -}
> +#define FANOTIFY_INLINE_FH(name, size)                                               \
> +union {                                                                                      \
> +       /* Space for object_fh and object_fh.buf[] - access with fanotify_fh_buf() */ \
> +       unsigned char _inline_fh_buf[struct_size_t(struct fanotify_fh, buf, size)];   \

The name _inline_fh_buf is confusing in this setting
better use bytes[] as in DEFINE_FLEX() or maybe even consider
a generic helper DEFINE_FLEX_MEMBER() to use instead of
FANOTIFY_INLINE_FH(), because this is not fanotify specific,
except maybe for alignment (see below).

> +       struct fanotify_fh name;                                                      \
> +} __packed

Why added __packed?

The fact that struct fanotify_fh is 4 bytes aligned could end up with less
bytes reserved for the inline buffer if the union is not also 4 bytes aligned.

So maybe something like this:

#define FANOTIFY_INLINE_FH(name, size) \
    DEFINE_FLEX_MEMBER(struct fanotify_fh, name, size) __aligned(4)

Thanks,
Amir.
Re: [PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
Posted by Jan Kara 7 months, 1 week ago
On Wed 07-05-25 07:56:21, Amir Goldstein wrote:
> On Wed, May 7, 2025 at 1:39 AM Gustavo A. R. Silva
> <gustavoars@kernel.org> wrote:
> >
> > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> > getting ready to enable it, globally.
> >
> > Modify FANOTIFY_INLINE_FH() macro, which defines a struct containing a
> > flexible-array member in the middle (struct fanotify_fh::buf), to use
> > struct_size_t() to pre-allocate space for both struct fanotify_fh and
> > its flexible-array member. Replace the struct with a union and relocate
> > the flexible structure (struct fanotify_fh) to the end.
> >
> > See the memory layout of struct fanotify_fid_event before and after
> > changes below.
> >
> > pahole -C fanotify_fid_event fs/notify/fanotify/fanotify.o
> >
> > BEFORE:
> > struct fanotify_fid_event {
> >         struct fanotify_event      fae;                  /*     0    48 */
> >         __kernel_fsid_t            fsid;                 /*    48     8 */
> >         struct {
> >                 struct fanotify_fh object_fh;            /*    56     4 */
> >                 unsigned char      _inline_fh_buf[12];   /*    60    12 */
> >         };                                               /*    56    16 */
> >
> >         /* size: 72, cachelines: 2, members: 3 */
> >         /* last cacheline: 8 bytes */
> > };
> >
> > AFTER:
> > struct fanotify_fid_event {
> >         struct fanotify_event      fae;                  /*     0    48 */
> >         __kernel_fsid_t            fsid;                 /*    48     8 */
> >         union {
> >                 unsigned char      _inline_fh_buf[16];   /*    56    16 */
> >                 struct fanotify_fh object_fh __attribute__((__aligned__(1))); /*    56     4 */
> 
> I'm not that familiar with pahole, but I find it surprising to see this member
> aligned(1), when struct fanotify_fh is defined as __aligned(4).

Yeah.

> >         } __attribute__((__aligned__(1)));               /*    56    16 */
> >
> >         /* size: 72, cachelines: 2, members: 3 */
> >         /* forced alignments: 1 */
> >         /* last cacheline: 8 bytes */
> > } __attribute__((__aligned__(8)));
> >
> > So, with these changes, fix the following warnings:
> >
> > fs/notify/fanotify/fanotify.h:317:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > fs/notify/fanotify/fanotify.h:289:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> >
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> >  fs/notify/fanotify/fanotify.h | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> > index b44e70e44be6..91c26b1c1d32 100644
> > --- a/fs/notify/fanotify/fanotify.h
> > +++ b/fs/notify/fanotify/fanotify.h
> > @@ -275,12 +275,12 @@ static inline void fanotify_init_event(struct fanotify_event *event,
> >         event->pid = NULL;
> >  }
> >
> > -#define FANOTIFY_INLINE_FH(name, size)                                 \
> > -struct {                                                               \
> > -       struct fanotify_fh name;                                        \
> > -       /* Space for object_fh.buf[] - access with fanotify_fh_buf() */ \
> > -       unsigned char _inline_fh_buf[size];                             \
> > -}
> > +#define FANOTIFY_INLINE_FH(name, size)                                               \
> > +union {                                                                                      \
> > +       /* Space for object_fh and object_fh.buf[] - access with fanotify_fh_buf() */ \
> > +       unsigned char _inline_fh_buf[struct_size_t(struct fanotify_fh, buf, size)];   \
> 
> The name _inline_fh_buf is confusing in this setting
> better use bytes[] as in DEFINE_FLEX() or maybe even consider
> a generic helper DEFINE_FLEX_MEMBER() to use instead of
> FANOTIFY_INLINE_FH(), because this is not fanotify specific,
> except maybe for alignment (see below).

Yes, I guess a generic helper for this would be nice but if fanotify is the
only place that plays these tricks, we can keep it specific for now. I
agree naming the "space-buffer" field "bytes" would be less confusing.

> 
> > +       struct fanotify_fh name;                                                      \
> > +} __packed
> 
> Why added __packed?
> 
> The fact that struct fanotify_fh is 4 bytes aligned could end up with less
> bytes reserved for the inline buffer if the union is not also 4 bytes aligned.
> 
> So maybe something like this:
> 
> #define FANOTIFY_INLINE_FH(name, size) \
>     DEFINE_FLEX_MEMBER(struct fanotify_fh, name, size) __aligned(4)

I guess you need to provide the "member" information to
DEFINE_FLEX_MEMBER() somewhere as well.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH][next] fanotify: Avoid a couple of -Wflex-array-member-not-at-end warnings
Posted by Gustavo A. R. Silva 7 months, 1 week ago

On 07/05/25 05:08, Jan Kara wrote:
> On Wed 07-05-25 07:56:21, Amir Goldstein wrote:
>> On Wed, May 7, 2025 at 1:39 AM Gustavo A. R. Silva
>> <gustavoars@kernel.org> wrote:
>>>
>>> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>>> getting ready to enable it, globally.
>>>
>>> Modify FANOTIFY_INLINE_FH() macro, which defines a struct containing a
>>> flexible-array member in the middle (struct fanotify_fh::buf), to use
>>> struct_size_t() to pre-allocate space for both struct fanotify_fh and
>>> its flexible-array member. Replace the struct with a union and relocate
>>> the flexible structure (struct fanotify_fh) to the end.
>>>
>>> See the memory layout of struct fanotify_fid_event before and after
>>> changes below.
>>>
>>> pahole -C fanotify_fid_event fs/notify/fanotify/fanotify.o
>>>
>>> BEFORE:
>>> struct fanotify_fid_event {
>>>          struct fanotify_event      fae;                  /*     0    48 */
>>>          __kernel_fsid_t            fsid;                 /*    48     8 */
>>>          struct {
>>>                  struct fanotify_fh object_fh;            /*    56     4 */
>>>                  unsigned char      _inline_fh_buf[12];   /*    60    12 */
>>>          };                                               /*    56    16 */
>>>
>>>          /* size: 72, cachelines: 2, members: 3 */
>>>          /* last cacheline: 8 bytes */
>>> };
>>>
>>> AFTER:
>>> struct fanotify_fid_event {
>>>          struct fanotify_event      fae;                  /*     0    48 */
>>>          __kernel_fsid_t            fsid;                 /*    48     8 */
>>>          union {
>>>                  unsigned char      _inline_fh_buf[16];   /*    56    16 */
>>>                  struct fanotify_fh object_fh __attribute__((__aligned__(1))); /*    56     4 */
>>
>> I'm not that familiar with pahole, but I find it surprising to see this member
>> aligned(1), when struct fanotify_fh is defined as __aligned(4).
> 
> Yeah.

Yep, gotcha.

> 
>>>          } __attribute__((__aligned__(1)));               /*    56    16 */
>>>
>>>          /* size: 72, cachelines: 2, members: 3 */
>>>          /* forced alignments: 1 */
>>>          /* last cacheline: 8 bytes */
>>> } __attribute__((__aligned__(8)));
>>>
>>> So, with these changes, fix the following warnings:
>>>
>>> fs/notify/fanotify/fanotify.h:317:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>> fs/notify/fanotify/fanotify.h:289:28: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>>
>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>> ---
>>>   fs/notify/fanotify/fanotify.h | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
>>> index b44e70e44be6..91c26b1c1d32 100644
>>> --- a/fs/notify/fanotify/fanotify.h
>>> +++ b/fs/notify/fanotify/fanotify.h
>>> @@ -275,12 +275,12 @@ static inline void fanotify_init_event(struct fanotify_event *event,
>>>          event->pid = NULL;
>>>   }
>>>
>>> -#define FANOTIFY_INLINE_FH(name, size)                                 \
>>> -struct {                                                               \
>>> -       struct fanotify_fh name;                                        \
>>> -       /* Space for object_fh.buf[] - access with fanotify_fh_buf() */ \
>>> -       unsigned char _inline_fh_buf[size];                             \
>>> -}
>>> +#define FANOTIFY_INLINE_FH(name, size)                                               \
>>> +union {                                                                                      \
>>> +       /* Space for object_fh and object_fh.buf[] - access with fanotify_fh_buf() */ \
>>> +       unsigned char _inline_fh_buf[struct_size_t(struct fanotify_fh, buf, size)];   \
>>
>> The name _inline_fh_buf is confusing in this setting
>> better use bytes[] as in DEFINE_FLEX() or maybe even consider
>> a generic helper DEFINE_FLEX_MEMBER() to use instead of
>> FANOTIFY_INLINE_FH(), because this is not fanotify specific,
>> except maybe for alignment (see below).
> 
> Yes, I guess a generic helper for this would be nice but if fanotify is the
> only place that plays these tricks, we can keep it specific for now. I
> agree naming the "space-buffer" field "bytes" would be less confusing.

I can send v2 with this change.

> 
>>
>>> +       struct fanotify_fh name;                                                      \
>>> +} __packed
>>
>> Why added __packed?
>>
>> The fact that struct fanotify_fh is 4 bytes aligned could end up with less
>> bytes reserved for the inline buffer if the union is not also 4 bytes aligned.
>>
>> So maybe something like this:
>>
>> #define FANOTIFY_INLINE_FH(name, size) \
>>      DEFINE_FLEX_MEMBER(struct fanotify_fh, name, size) __aligned(4)
> 
> I guess you need to provide the "member" information to
> DEFINE_FLEX_MEMBER() somewhere as well.

Yeah, I can write something like that as well.

I'll come back with v2, shortly.

Thanks for the feedback, folks! :)
--
Gustavo