Hi Jiri,
On Thu, May 30, 2024 at 08:41:18AM +0200, Jiri Slaby wrote:
> On 29. 05. 24, 23:42, Nathan Chancellor wrote:
> > drivers/nvme/target/fc.c:151:2: error: 'counted_by' should not be applied to an array with element of unknown size because 'struct nvmet_fc_fcp_iod' is a struct type with a flexible array member.
>
> The same as for mxser_port:
>
> struct nvmet_fc_fcp_iod {
> struct nvmefc_tgt_fcp_req *fcpreq;
>
> struct nvme_fc_cmd_iu cmdiubuf;
> struct nvme_fc_ersp_iu rspiubuf;
> dma_addr_t rspdma;
> struct scatterlist *next_sg;
> struct scatterlist *data_sg;
> int data_sg_cnt;
> u32 offset;
> enum nvmet_fcp_datadir io_dir;
> bool active;
> bool abort;
> bool aborted;
> bool writedataactive;
> spinlock_t flock;
>
> struct nvmet_req req;
> struct work_struct defer_work;
>
> struct nvmet_fc_tgtport *tgtport;
> struct nvmet_fc_tgt_queue *queue;
>
> struct list_head fcp_list; /* tgtport->fcp_list
> */
> };
>
> The error appears to be invalid.
>
> > This will be an error in a future compiler version [-Werror,-Wbounds-safety-counted-by-elt-type-unknown-size]
> > 151 | struct nvmet_fc_fcp_iod fod[] __counted_by(sqsize);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
My apologies, I should have done the work to fully uncover the flexible
array member within 'struct nvmet_fc_fcp_iod' from the beginning and put
it in the commit message. I did not think of using pahole to make my
life easier until just now and I knew from the other examples that I had
and clang's code that it was not incorrect. Sure enough, it comes from
'struct bio' within 'struct nvmet_req'.
struct nvmet_fc_fcp_iod {
...
struct nvmet_req req;
...
};
struct nvmet_req {
...
struct bio_vec inline_bvec[NVMET_MAX_INLINE_BIOVEC];
union {
struct {
struct bio inline_bio;
} b;
struct {
bool mpool_alloc;
struct kiocb iocb;
struct bio_vec *bvec;
struct work_struct work;
} f;
struct {
struct bio inline_bio;
struct request *rq;
struct work_struct work;
bool use_workqueue;
} p;
#ifdef CONFIG_BLK_DEV_ZONED
struct {
struct bio inline_bio;
struct work_struct zmgmt_work;
} z;
#endif /* CONFIG_BLK_DEV_ZONED */
};
int sg_cnt;
...
};
struct bio {
...
struct bio_set *bi_pool;
/*
* We can inline a number of vecs at the end of the bio, to avoid
* double allocations for a small number of bio_vecs. This member
* MUST obviously be kept at the very end of the bio.
*/
struct bio_vec bi_inline_vecs[];
};
It sounds like it is already on Gustavo's radar to look into for
-Wflexible-array-member-not-at-end, so he said he would take a look. It
may not be a quick fix though (I'll let him comment on it further if he
is so inclined). It will be needed in stable because the patch that
added __counted_by to this structure is there, so considering this patch
for that sake may still be worthwhile, then it could be reverted with
Gustavo's changes.
I would really like to avoid leaving the build with tip of tree Clang
broken for a long period of time, as we qualify it against the kernel
continously so that any fixes needed on the kernel side are merged and
ready by the time the toolchain is actually releases (such as this one).
I am fine with waiting some time to see how this plays out but I don't
want it to be forgotten about.
Cheers,
Nathan
