From: Arnd Bergmann <arnd@kernel.org>
To: Don.Brace@microchip.com
Cc: Sergei Trofimovich <slyfox@gentoo.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-ia64@vger.kernel.org, storagedev@microchip.com,
linux-scsi <linux-scsi@vger.kernel.org>,
jszczype@redhat.com, Scott.Benesh@microchip.com,
Scott.Teel@microchip.com, thenzl@redhat.com,
"Martin K. Petersen" <martin.petersen@oracle.com>,
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Subject: Re: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)
Date: Tue, 16 Mar 2021 19:28:22 +0100 [thread overview]
Message-ID: <CAK8P3a3JYmhbN3TXB2cWGfXGKgsUa9Hg=ZvWckTaL_31OMgbtQ@mail.gmail.com> (raw)
In-Reply-To: <SN6PR11MB2848561E3D85A8F55EB86977E16B9@SN6PR11MB2848.namprd11.prod.outlook.com>
On Tue, Mar 16, 2021 at 6:12 PM <Don.Brace@microchip.com> wrote:
> drivers/scsi/hpsa_cmd.h | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h index d126bb877250..617bdae9a7de 100644
> --- a/drivers/scsi/hpsa_cmd.h
> +++ b/drivers/scsi/hpsa_cmd.h
> @@ -20,6 +20,9 @@
> #ifndef HPSA_CMD_H
> #define HPSA_CMD_H
>
> +#include <linux/build_bug.h> /* static_assert */ #include
> +<linux/stddef.h> /* offsetof */
> +
> /* general boundary defintions */
> #define SENSEINFOBYTES 32 /* may vary between hbas */
> #define SG_ENTRIES_IN_CMD 32 /* Max SG entries excluding chain blocks */
> @@ -448,11 +451,20 @@ struct CommandList {
> */
> struct hpsa_scsi_dev_t *phys_disk;
>
> - bool retry_pending;
> + int retry_pending;
> struct hpsa_scsi_dev_t *device;
> atomic_t refcount; /* Must be last to avoid memset in hpsa_cmd_init() */ } __aligned(COMMANDLIST_ALIGNMENT);
>
> +/*
> + * Make sure our embedded atomic variable is aligned. Otherwise we
> +break atomic
> + * operations on architectures that don't support unaligned atomics like IA64.
> + *
> + * Ideally this header should be cleaned up to only mark individual
> +structs as
> + * packed.
> + */
> +static_assert(offsetof(struct CommandList, refcount) %
> +__alignof__(atomic_t) == 0);
> +
Actually that still feels wrong: the annotation of the struct is to pack
every member, which causes the access to be done in byte units
on architectures that do not have hardware unaligned load/store
instructions, at least for things like atomic_read() that does not
go through a cmpxchg() or ll/sc cycle.
This change may fix itanium, but it's still not correct. Other
architectures would have already been broken before the recent
change, but that's not a reason against fixing them now.
I'd recommend marking the entire structure as having default
alignment, by adding the appropriate pragmas before and after it.
Arnd
next prev parent reply other threads:[~2021-03-16 18:29 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210222230519.73f3e239@sf>
2021-02-22 23:34 ` 5.11 regression: "ia64: add support for TIF_NOTIFY_SIGNAL" breaks ia64 boot Jens Axboe
2021-02-22 23:55 ` John Paul Adrian Glaubitz
[not found] ` <20210223083507.43b5a6dd@sf>
2021-02-23 9:13 ` John Paul Adrian Glaubitz
2021-02-23 12:36 ` John Paul Adrian Glaubitz
[not found] ` <20210223192743.0198d4a9@sf>
[not found] ` <20210302222630.5056f243@sf>
2021-03-02 22:31 ` John Paul Adrian Glaubitz
2021-03-03 0:22 ` [bisected] 5.12-rc1 hpsa regression: "scsi: hpsa: Correct dev cmds outstanding for retried cmds" breaks hpsa P600 Sergei Trofimovich
2021-03-03 8:55 ` Sergei Trofimovich
2021-03-03 17:33 ` Don.Brace
[not found] ` <20210303220401.501449e5@sf>
2021-03-04 17:00 ` Don.Brace
2021-03-05 13:26 ` Tomas Henzl
2021-03-12 22:27 ` [PATCH] hpsa: fix boot on ia64 (atomic_t alignment) Sergei Trofimovich
2021-03-16 16:30 ` Don.Brace
2021-03-16 18:28 ` Arnd Bergmann [this message]
2021-03-17 2:25 ` Martin K. Petersen
2021-03-17 13:19 ` David Laight
2021-03-17 19:06 ` Don.Brace
2021-03-17 17:28 ` John Paul Adrian Glaubitz
2021-03-27 10:24 ` Sergei Trofimovich
2021-03-24 7:08 ` John Paul Adrian Glaubitz
2021-03-24 18:37 ` Don.Brace
2021-03-29 11:25 ` John Paul Adrian Glaubitz
2021-03-29 14:22 ` Arnd Bergmann
2021-03-30 3:02 ` Martin K. Petersen
2021-03-30 7:19 ` [PATCH v2 1/3] hpsa: use __packed on individual structs, not header-wide Sergei Trofimovich
2021-03-30 7:19 ` [PATCH v2 2/3] hpsa: fix boot on ia64 (atomic_t alignment) Sergei Trofimovich
2021-03-30 7:19 ` [PATCH v2 3/3] hpsa: add an assert to prevent from __packed reintroduction Sergei Trofimovich
2021-03-30 7:34 ` Arnd Bergmann
2021-04-02 14:40 ` Elliott, Robert (Servers)
2021-04-03 14:51 ` Sergei Trofimovich
2021-03-30 7:30 ` [PATCH v2 1/3] hpsa: use __packed on individual structs, not header-wide Arnd Bergmann
2021-03-30 7:43 ` Arnd Bergmann
2021-04-02 3:54 ` Martin K. Petersen
2021-04-15 18:41 ` Don.Brace
2021-03-05 9:22 ` [bisected] 5.12-rc1 hpsa regression: "scsi: hpsa: Correct dev cmds outstanding for retried cmds" breaks hpsa P600 Geert Uytterhoeven
2021-03-05 13:31 ` Arnd Bergmann
2021-03-05 20:45 ` Don.Brace
2021-03-03 15:42 ` Don.Brace
2021-03-17 17:42 ` 5.11 regression: "ia64: add support for TIF_NOTIFY_SIGNAL" breaks ia64 boot John Paul Adrian Glaubitz
2021-03-17 17:53 ` John Paul Adrian Glaubitz
[not found] ` <20210222235359.75d1a912@sf>
2021-02-23 0:34 ` Jens Axboe
2021-02-23 0:41 ` Jens Axboe
2021-02-23 0:43 ` Jens Axboe
[not found] ` <20210223080830.23bccdbf@sf>
2021-03-02 22:07 ` Sergei Trofimovich
2021-03-02 22:31 ` Jens Axboe
[not found] ` <20210302232716.353ed49b@sf>
2021-03-03 0:34 ` Jens Axboe
2021-03-03 3:51 ` Jens Axboe
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='CAK8P3a3JYmhbN3TXB2cWGfXGKgsUa9Hg=ZvWckTaL_31OMgbtQ@mail.gmail.com' \
--to=arnd@kernel.org \
--cc=Don.Brace@microchip.com \
--cc=Scott.Benesh@microchip.com \
--cc=Scott.Teel@microchip.com \
--cc=glaubitz@physik.fu-berlin.de \
--cc=jszczype@redhat.com \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=slyfox@gentoo.org \
--cc=storagedev@microchip.com \
--cc=thenzl@redhat.com \
/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 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).