* [bug report] Linux-2.6.12-rc2
@ 2025-05-28 8:10 Dan Carpenter
2025-05-30 5:46 ` Arnd Bergmann
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-05-28 8:10 UTC (permalink / raw)
To: linux-alpha; +Cc: Arnd Bergmann
Hello Alpha Maintainers,
Commit 1da177e4c3f4 ("Linux-2.6.12-rc2") from Apr 16, 2005
(linux-next), leads to the following Smatch static checker warning:
arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn: statement has no effect 'csrs->POx_ERR_SUM.csr'
arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn: statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
arch/alpha/kernel/err_marvel.c
797 static struct ev7_pal_io_subpacket *
798 marvel_find_io7_with_error(struct ev7_lf_subpackets *lf_subpackets)
799 {
800 struct ev7_pal_io_subpacket *io = lf_subpackets->io;
801 struct io7 *io7;
802 int i;
803
804 /*
805 * Caller must provide the packet to fill
806 */
807 if (!io)
808 return NULL;
809
810 /*
811 * Fill the subpacket with the console's standard fill pattern
812 */
813 memset(io, 0x55, sizeof(*io));
814
815 for (io7 = NULL; NULL != (io7 = marvel_next_io7(io7)); ) {
816 unsigned long err_sum = 0;
817
818 err_sum |= io7->csrs->PO7_ERROR_SUM.csr;
819 for (i = 0; i < IO7_NUM_PORTS; i++) {
820 if (!io7->ports[i].enabled)
821 continue;
822 err_sum |= io7->ports[i].csrs->POx_ERR_SUM.csr;
823 }
824
825 /*
826 * Is there at least one error?
827 */
828 if (err_sum & (1UL << 63))
829 break;
830 }
831
832 /*
833 * Did we find an IO7 with an error?
834 */
835 if (!io7)
836 return NULL;
837
838 /*
839 * We have an IO7 with an error.
840 *
841 * Fill in the IO subpacket.
842 */
843 io->io_asic_rev = io7->csrs->IO_ASIC_REV.csr;
844 io->io_sys_rev = io7->csrs->IO_SYS_REV.csr;
845 io->io7_uph = io7->csrs->IO7_UPH.csr;
846 io->hpi_ctl = io7->csrs->HPI_CTL.csr;
847 io->crd_ctl = io7->csrs->CRD_CTL.csr;
848 io->hei_ctl = io7->csrs->HEI_CTL.csr;
849 io->po7_error_sum = io7->csrs->PO7_ERROR_SUM.csr;
850 io->po7_uncrr_sym = io7->csrs->PO7_UNCRR_SYM.csr;
851 io->po7_crrct_sym = io7->csrs->PO7_CRRCT_SYM.csr;
852 io->po7_ugbge_sym = io7->csrs->PO7_UGBGE_SYM.csr;
853 io->po7_err_pkt0 = io7->csrs->PO7_ERR_PKT[0].csr;
854 io->po7_err_pkt1 = io7->csrs->PO7_ERR_PKT[1].csr;
855
856 for (i = 0; i < IO7_NUM_PORTS; i++) {
857 io7_ioport_csrs *csrs = io7->ports[i].csrs;
858
859 if (!io7->ports[i].enabled)
860 continue;
861
862 io->ports[i].pox_err_sum = csrs->POx_ERR_SUM.csr;
863 io->ports[i].pox_tlb_err = csrs->POx_TLB_ERR.csr;
864 io->ports[i].pox_spl_cmplt = csrs->POx_SPL_COMPLT.csr;
865 io->ports[i].pox_trans_sum = csrs->POx_TRANS_SUM.csr;
866 io->ports[i].pox_first_err = csrs->POx_FIRST_ERR.csr;
867 io->ports[i].pox_mult_err = csrs->POx_MULT_ERR.csr;
868 io->ports[i].pox_dm_source = csrs->POx_DM_SOURCE.csr;
869 io->ports[i].pox_dm_dest = csrs->POx_DM_DEST.csr;
870 io->ports[i].pox_dm_size = csrs->POx_DM_SIZE.csr;
871 io->ports[i].pox_dm_ctrl = csrs->POx_DM_CTRL.csr;
872
873 /*
874 * Ack this port's errors, if any. POx_ERR_SUM must be last.
875 *
876 * Most of the error registers get cleared and unlocked when
877 * the associated bits in POx_ERR_SUM are cleared (by writing
878 * 1). POx_TLB_ERR is an exception and must be explicitly
879 * cleared.
880 */
881 csrs->POx_TLB_ERR.csr = io->ports[i].pox_tlb_err;
882 csrs->POx_ERR_SUM.csr = io->ports[i].pox_err_sum;
883 mb();
--> 884 csrs->POx_ERR_SUM.csr;
^^^^^^^^^^^^^^^^^^^^^^
I occasionally try to run Smatch against code that I can't actually
compile and it found this code from before the git era. These days
we build with -Wall and so this kind of code doesn't normally compile
on x86 or Arm. We would get a warning like:
test.c:18:9: warning: statement with no effect [-Wunused-value]
18 | x;
| ^
So it makes me wonder if this code can actually build?
885 }
886
887 /*
888 * Ack any port 7 error(s).
889 */
890 io7->csrs->PO7_ERROR_SUM.csr = io->po7_error_sum;
91 mb();
--> 892 io7->csrs->PO7_ERROR_SUM.csr;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
893
894 /*
895 * Correct the io7_pid.
896 */
897 lf_subpackets->io_pid = io7->pe;
898
899 return io;
900 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] Linux-2.6.12-rc2
2025-05-28 8:10 [bug report] Linux-2.6.12-rc2 Dan Carpenter
@ 2025-05-30 5:46 ` Arnd Bergmann
2025-06-02 7:33 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2025-05-30 5:46 UTC (permalink / raw)
To: Dan Carpenter, linux-alpha
On Wed, May 28, 2025, at 10:10, Dan Carpenter wrote:
>
> arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn:
> statement has no effect 'csrs->POx_ERR_SUM.csr'
> arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn:
> statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
>
> I occasionally try to run Smatch against code that I can't actually
> compile and it found this code from before the git era. These days
> we build with -Wall and so this kind of code doesn't normally compile
> on x86 or Arm. We would get a warning like:
>
> test.c:18:9: warning: statement with no effect [-Wunused-value]
> 18 | x;
> | ^
>
> So it makes me wonder if this code can actually build?
>
FWIW, I tried building the file with gcc-15 and don't see a
warning for that construct, even with the -Wextra.
After digging around some more, I found that this is the
definition of the structure, and that the 'volatile' in there
causes gcc and clang to not consider the statement to be
free of side-effects. I assume it will actually cause a
load from an MMIO register here:
typedef struct {
volatile unsigned long csr __attribute__((aligned(64)));
} io7_csr;
Arnd
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] Linux-2.6.12-rc2
2025-05-30 5:46 ` Arnd Bergmann
@ 2025-06-02 7:33 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-06-02 7:33 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-alpha
On Fri, May 30, 2025 at 07:46:20AM +0200, Arnd Bergmann wrote:
> On Wed, May 28, 2025, at 10:10, Dan Carpenter wrote:
> >
> > arch/alpha/kernel/err_marvel.c:884 marvel_find_io7_with_error() warn:
> > statement has no effect 'csrs->POx_ERR_SUM.csr'
> > arch/alpha/kernel/err_marvel.c:892 marvel_find_io7_with_error() warn:
> > statement has no effect 'io7->csrs->PO7_ERROR_SUM.csr'
> >
>
> > I occasionally try to run Smatch against code that I can't actually
> > compile and it found this code from before the git era. These days
> > we build with -Wall and so this kind of code doesn't normally compile
> > on x86 or Arm. We would get a warning like:
> >
> > test.c:18:9: warning: statement with no effect [-Wunused-value]
> > 18 | x;
> > | ^
> >
> > So it makes me wonder if this code can actually build?
> >
>
> FWIW, I tried building the file with gcc-15 and don't see a
> warning for that construct, even with the -Wextra.
>
> After digging around some more, I found that this is the
> definition of the structure, and that the 'volatile' in there
> causes gcc and clang to not consider the statement to be
> free of side-effects. I assume it will actually cause a
> load from an MMIO register here:
>
> typedef struct {
> volatile unsigned long csr __attribute__((aligned(64)));
> } io7_csr;
Huh. Thanks, Arnd. I hadn't even considered that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-02 7:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28 8:10 [bug report] Linux-2.6.12-rc2 Dan Carpenter
2025-05-30 5:46 ` Arnd Bergmann
2025-06-02 7:33 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).