Re: [git pull] device mapper fixes for 6.8-rc6

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Mike Snitzer <snitzer@kernel.org>, Jens Axboe <axboe@kernel.dk>,
	Christoph Hellwig <hch@lst.de>,  Ming Lei <ming.lei@redhat.com>
Cc: dm-devel@lists.linux.dev, linux-block@vger.kernel.org,
	 Alasdair G Kergon <agk@redhat.com>,
	Benjamin Marzinski <bmarzins@redhat.com>,
	 Mikulas Patocka <mpatocka@redhat.com>
Subject: Re: [git pull] device mapper fixes for 6.8-rc6
Date: Fri, 23 Feb 2024 09:42:21 -0800	[thread overview]
Message-ID: <CAHk-=whmiQC_F1s1bWmOhM8csz_zxL32B=sPGgaz1kiTK_T2iA@mail.gmail.com> (raw)
In-Reply-To: <ZdjTMZRwZ_9GjCmc@redhat.com>

On Fri, 23 Feb 2024 at 09:17, Mike Snitzer <snitzer@kernel.org> wrote:
>
> - Fix DM crypt and verity targets to align their respective bvec_iter
>   struct members to avoid the need for byte level access (due to
>   __packed attribute) that is costly on some arches (like RISC).

Ugh. This is due to commit 19416123ab3e ("block: define 'struct
bvec_iter' as packed"), and the point of *that* commit was that it
doesn't hurt to mark it packed.

That was clearly not true.

And honestly, "__packed" really is wrong here.  Nobody ever wanted it
to be completely unaligned.

I think we might be better off marking it as being 4-byte aligned.
That would mean that instead of __packed, it is done as

   __packed __aligned(4)

because "__aligned" on its own only increases alignment (so without
the __packed it would stay 8-byte aligned).

Then the only part of that structure that might be unaligned is
"sector_t", and that would only matter on 64-bit architectures.

 And very few architectures are both 64-bit _and_ so broken as to not
do unaligned loads well. And even if such broken architectures exist,
at least they can do the 8-byte load as two 4-byte ones rather than
doing it as byte loads..

Anyway, I've pulled this, but I really think this should have been
fixed in bvec.h instead.

Jens/Christoph/whoever feels they own bvec.h?

                Linus