All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ben Dooks" <ben.dooks@codethink.co.uk>
Cc: yukuai@fygo.io, linux-raid@vger.kernel.org
Subject: Re: [PATCH] raid6: fix riscv symbol undeclared warnigns
Date: Mon, 22 Jun 2026 14:02:57 +0000	[thread overview]
Message-ID: <20260622140257.CF4781F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260622135207.480540-1-ben.dooks@codethink.co.uk>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Including `pq_arch.h` in `rvv.c` misuses an initialization fragment as a header, creating a brittle include dependency and violating architectural boundaries.
--

commit 0d7480a95df3baf577ec13b2b5aec7d36d11ee3f
Author: Ben Dooks <ben.dooks@codethink.co.uk>

raid6: fix riscv symbol undeclared warnigns

This commit attempts to resolve sparse warnings about undeclared raid6_rvvx
symbols in the RISC-V RAID6 implementation by including pq_arch.h in rvv.c.

> raid6: fix riscv symbol undeclared warnigns

This isn't a bug, but there's a typo (warnigns) in the subject line.

> diff --git a/lib/raid/raid6/riscv/rvv.c b/lib/raid/raid6/riscv/rvv.c
> index 75c9dafedb284..4ac50606f3dc1 100644
> --- a/lib/raid/raid6/riscv/rvv.c
> +++ b/lib/raid/raid6/riscv/rvv.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "rvv.h"
> +#include "pq_arch.h"

[Severity: Low]
Is it appropriate to include pq_arch.h here to fix the sparse warnings?

The pq_arch.h file appears to be designed as an initialization fragment meant
to be conditionally included by the generic algos.c, since it contains the
arch_raid6_init() static inline function. Including it here inappropriately
injects unused initialization functions into the backend implementation.

Does this also create a brittle include order dependency? The pq_arch.h file
uses functions like raid6_algo_add_default and raid6_algo_add but does not
include algos.h where they are declared. The build only succeeds because rvv.c
happens to include rvv.h (which includes algos.h) immediately before pq_arch.h.
Any reordering of these includes could break the build.

Would it be safer to place the extern declarations for these symbols in a
shared internal header like rvv.h?

>  
>  #ifdef __riscv_vector
>  #error "This code must be built without compiler support for vector"

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622135207.480540-1-ben.dooks@codethink.co.uk?part=1

  reply	other threads:[~2026-06-22 14:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-22 13:52 [PATCH] raid6: fix riscv symbol undeclared warnigns Ben Dooks
2026-06-22 13:52 ` Ben Dooks
2026-06-22 14:02 ` sashiko-bot [this message]
2026-06-22 18:23 ` Nam Cao
2026-06-22 18:23   ` Nam Cao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260622140257.CF4781F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ben.dooks@codethink.co.uk \
    --cc=linux-raid@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yukuai@fygo.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.