[PATCH 0/8][next] Avoid a couple hundred -Wflex-array-member-not-at-end warnings

[PATCH 0/8][next] Avoid a couple hundred -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

This patch series aims to fix a couple hundred -Wflex-array-member-not-at-end
warnings by creating a new tagged struct `struct bio_hdr` within flexible
structure `struct bio`.

This new tagged struct will be used to fix problematic declarations
of middle-flex-arrays in composite structs, like these[1][2][3], for
instance.

[1] https://git.kernel.org/linus/a7e8997ae18c42d3
[2] https://git.kernel.org/linus/c1ddb29709e675ea
[3] https://git.kernel.org/linus/57be3d3562ca4aa6

Gustavo A. R. Silva (8):
  block: blk_types.h: Use struct_group_tagged() in flex struct bio
  md/raid5-ppl: Avoid -Wflex-array-member-not-at-end warning
  xfs: Avoid -Wflex-array-member-not-at-end warnings
  erofs: Avoid -Wflex-array-member-not-at-end warnings
  btrfs: Avoid -Wflex-array-member-not-at-end warnings
  nvme: target: Avoid -Wflex-array-member-not-at-end warnings
  md/raid5: Avoid -Wflex-array-member-not-at-end warnings
  bcache: Avoid -Wflex-array-member-not-at-end warnings

 drivers/md/bcache/bcache.h     |  4 +-
 drivers/md/bcache/journal.c    | 10 ++--
 drivers/md/bcache/journal.h    |  4 +-
 drivers/md/bcache/super.c      |  8 ++--
 drivers/md/raid5-ppl.c         |  8 ++--
 drivers/md/raid5.c             | 10 ++--
 drivers/md/raid5.h             |  2 +-
 drivers/nvme/target/nvmet.h    |  4 +-
 drivers/nvme/target/passthru.c |  2 +-
 drivers/nvme/target/zns.c      |  2 +-
 fs/btrfs/disk-io.c             |  4 +-
 fs/btrfs/volumes.h             |  2 +-
 fs/erofs/fileio.c              | 25 ++++++----
 fs/erofs/fscache.c             | 13 +++---
 fs/xfs/xfs_log.c               | 15 +++---
 fs/xfs/xfs_log_priv.h          |  2 +-
 include/linux/blk_types.h      | 84 ++++++++++++++++++----------------
 17 files changed, 107 insertions(+), 92 deletions(-)

-- 
2.43.0

[PATCH 3/8][next] xfs: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

Change the type of the middle struct members currently causing trouble
from `struct bio` to `struct bio_hdr`.

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure `struct bio`, through which we can access the
flexible-array member in it, if necessary.

With these changes fix 27 of the following warnings:

fs/xfs/xfs_log_priv.h:208:33: 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/xfs/xfs_log.c      | 15 +++++++++------
 fs/xfs/xfs_log_priv.h |  2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f8851ff835de..7e8b71f64a46 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1245,7 +1245,7 @@ xlog_ioend_work(
 	}
 
 	xlog_state_done_syncing(iclog);
-	bio_uninit(&iclog->ic_bio);
+	bio_uninit(container_of(&iclog->ic_bio, struct bio, __hdr));
 
 	/*
 	 * Drop the lock to signal that we are done. Nothing references the
@@ -1663,7 +1663,8 @@ xlog_write_iclog(
 	 * writeback throttle from throttling log writes behind background
 	 * metadata writeback and causing priority inversions.
 	 */
-	bio_init(&iclog->ic_bio, log->l_targ->bt_bdev, iclog->ic_bvec,
+	bio_init(container_of(&iclog->ic_bio, struct bio, __hdr),
+		 log->l_targ->bt_bdev, iclog->ic_bvec,
 		 howmany(count, PAGE_SIZE),
 		 REQ_OP_WRITE | REQ_META | REQ_SYNC | REQ_IDLE);
 	iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart + bno;
@@ -1692,7 +1693,8 @@ xlog_write_iclog(
 
 	iclog->ic_flags &= ~(XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
 
-	if (xlog_map_iclog_data(&iclog->ic_bio, iclog->ic_data, count))
+	if (xlog_map_iclog_data(container_of(&iclog->ic_bio, struct bio, __hdr),
+				iclog->ic_data, count))
 		goto shutdown;
 
 	if (is_vmalloc_addr(iclog->ic_data))
@@ -1705,16 +1707,17 @@ xlog_write_iclog(
 	if (bno + BTOBB(count) > log->l_logBBsize) {
 		struct bio *split;
 
-		split = bio_split(&iclog->ic_bio, log->l_logBBsize - bno,
+		split = bio_split(container_of(&iclog->ic_bio, struct bio, __hdr),
+				  log->l_logBBsize - bno,
 				  GFP_NOIO, &fs_bio_set);
-		bio_chain(split, &iclog->ic_bio);
+		bio_chain(split, container_of(&iclog->ic_bio, struct bio, __hdr));
 		submit_bio(split);
 
 		/* restart at logical offset zero for the remainder */
 		iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart;
 	}
 
-	submit_bio(&iclog->ic_bio);
+	submit_bio(container_of(&iclog->ic_bio, struct bio, __hdr));
 	return;
 shutdown:
 	xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index f3d78869e5e5..32abc48aef24 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -205,7 +205,7 @@ typedef struct xlog_in_core {
 #endif
 	struct semaphore	ic_sema;
 	struct work_struct	ic_end_io_work;
-	struct bio		ic_bio;
+	struct bio_hdr		ic_bio;
 	struct bio_vec		ic_bvec[];
 } xlog_in_core_t;
 
-- 
2.43.0

Re: [PATCH 3/8][next] xfs: Avoid -Wflex-array-member-not-at-end warnings

[Darrick J. Wong]

On Mon, Feb 24, 2025 at 08:27:44PM +1030, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> Change the type of the middle struct members currently causing trouble
> from `struct bio` to `struct bio_hdr`.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure `struct bio`, through which we can access the
> flexible-array member in it, if necessary.
> 
> With these changes fix 27 of the following warnings:
> 
> fs/xfs/xfs_log_priv.h:208:33: 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/xfs/xfs_log.c      | 15 +++++++++------
>  fs/xfs/xfs_log_priv.h |  2 +-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index f8851ff835de..7e8b71f64a46 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1245,7 +1245,7 @@ xlog_ioend_work(
>  	}
>  
>  	xlog_state_done_syncing(iclog);
> -	bio_uninit(&iclog->ic_bio);
> +	bio_uninit(container_of(&iclog->ic_bio, struct bio, __hdr));
>  
>  	/*
>  	 * Drop the lock to signal that we are done. Nothing references the
> @@ -1663,7 +1663,8 @@ xlog_write_iclog(
>  	 * writeback throttle from throttling log writes behind background
>  	 * metadata writeback and causing priority inversions.
>  	 */
> -	bio_init(&iclog->ic_bio, log->l_targ->bt_bdev, iclog->ic_bvec,
> +	bio_init(container_of(&iclog->ic_bio, struct bio, __hdr),
> +		 log->l_targ->bt_bdev, iclog->ic_bvec,
>  		 howmany(count, PAGE_SIZE),
>  		 REQ_OP_WRITE | REQ_META | REQ_SYNC | REQ_IDLE);
>  	iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart + bno;
> @@ -1692,7 +1693,8 @@ xlog_write_iclog(
>  
>  	iclog->ic_flags &= ~(XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
>  
> -	if (xlog_map_iclog_data(&iclog->ic_bio, iclog->ic_data, count))
> +	if (xlog_map_iclog_data(container_of(&iclog->ic_bio, struct bio, __hdr),
> +				iclog->ic_data, count))
>  		goto shutdown;
>  
>  	if (is_vmalloc_addr(iclog->ic_data))
> @@ -1705,16 +1707,17 @@ xlog_write_iclog(
>  	if (bno + BTOBB(count) > log->l_logBBsize) {
>  		struct bio *split;
>  
> -		split = bio_split(&iclog->ic_bio, log->l_logBBsize - bno,
> +		split = bio_split(container_of(&iclog->ic_bio, struct bio, __hdr),
> +				  log->l_logBBsize - bno,
>  				  GFP_NOIO, &fs_bio_set);
> -		bio_chain(split, &iclog->ic_bio);
> +		bio_chain(split, container_of(&iclog->ic_bio, struct bio, __hdr));
>  		submit_bio(split);
>  
>  		/* restart at logical offset zero for the remainder */
>  		iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart;
>  	}
>  
> -	submit_bio(&iclog->ic_bio);
> +	submit_bio(container_of(&iclog->ic_bio, struct bio, __hdr));
>  	return;
>  shutdown:
>  	xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index f3d78869e5e5..32abc48aef24 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -205,7 +205,7 @@ typedef struct xlog_in_core {
>  #endif
>  	struct semaphore	ic_sema;
>  	struct work_struct	ic_end_io_work;
> -	struct bio		ic_bio;
> +	struct bio_hdr		ic_bio;

What struct is this?

$ git grep 'struct bio_hdr' include/
$

(Please always send the core code change patches to the xfs list.)

--D

>  	struct bio_vec		ic_bvec[];
>  } xlog_in_core_t;
>  
> -- 
> 2.43.0
> 
> 

Re: [PATCH 3/8][next] xfs: Avoid -Wflex-array-member-not-at-end warnings

[Dave Chinner]

On Mon, Feb 24, 2025 at 08:27:44PM +1030, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> Change the type of the middle struct members currently causing trouble
> from `struct bio` to `struct bio_hdr`.

What's this bio_hdr thing? You haven't sent the patch to the XFS
list, so we cannot review this for correctness. Please cc the
-entire- patch set to -all- recipients so we can see the whole
change in it's full context.

> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure `struct bio`, through which we can access the
> flexible-array member in it, if necessary.
> 
> With these changes fix 27 of the following warnings:
> 
> fs/xfs/xfs_log_priv.h:208:33: 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/xfs/xfs_log.c      | 15 +++++++++------
>  fs/xfs/xfs_log_priv.h |  2 +-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index f8851ff835de..7e8b71f64a46 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1245,7 +1245,7 @@ xlog_ioend_work(
>  	}
>  
>  	xlog_state_done_syncing(iclog);
> -	bio_uninit(&iclog->ic_bio);
> +	bio_uninit(container_of(&iclog->ic_bio, struct bio, __hdr));

This is a pretty nasty conversion. The code is obviously correct
right now - the bio uses an external bvec table, so has no
associated allocated bvec space and so the bio isn't flexibly
sized.

Making the code more complex for humans to understand and get
right because "the compiler is dumb" is a bad tradeoff.

I also think this is a really nasty way of fixing the problem;
casting the fixed size structure to a flex sized structure that can
overlap other parent structure fields really isn't a good solution.
IMO, it is a recipe for unexpected memory corruption when the bio
isn't set up properly by the caller or there are bugs in the way the
bio is iterated/processed....

>  	return;
>  shutdown:
>  	xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index f3d78869e5e5..32abc48aef24 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -205,7 +205,7 @@ typedef struct xlog_in_core {
>  #endif
>  	struct semaphore	ic_sema;
>  	struct work_struct	ic_end_io_work;
> -	struct bio		ic_bio;
> +	struct bio_hdr		ic_bio;
>  	struct bio_vec          ic_bvec[];

But then there is the bigger question: ic_bvec is a *fixed size*
that is allocated at mount time. The fact it is defined as a
flexible array is the problem here, not the embedding of a struct
bio. i.e. we've used a flex array to optimise away an extra
allocation in a non-performance sensitive path.

Hence if we had done it this way:

	struct bvec		*ic_bvecs;
	struct bio		ic_bio;		// must be last
};

and use another allocation for ic_bvecs in xlog_alloc_log() (similar
to how we do the separate allocation of the *ic_data buffer that is
mapped into ic_bio at IO time), then none of the code that accesses
ic_bio for IO purposes needs to change and the compiler warnings go
away. That's a much cleaner solution that requires no extra
cognitive load on developers to maintain in correct working order.

-Dave.
-- 
Dave Chinner
david@fromorbit.com