Received-SPF: pass (google.com: domain of linux-kernel+bounces-86240-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Michael Kelley <mhklinux@outlook.com>
To: Will Deacon <will@kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, Petr Tesarik
	<petr.tesarik1@huawei-partners.com>
CC: "kernel-team@android.com" <kernel-team@android.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>, Christoph Hellwig
	<hch@lst.de>, Marek Szyprowski <m.szyprowski@samsung.com>, Robin Murphy
	<robin.murphy@arm.com>, Petr Tesarik <petr.tesarik1@huawei-partners.com>,
	Dexuan Cui <decui@microsoft.com>, Nicolin Chen <nicolinc@nvidia.com>
Subject: RE: [PATCH v5 6/6] swiotlb: Remove pointless stride adjustment for
 allocations >= PAGE_SIZE
Thread-Topic: [PATCH v5 6/6] swiotlb: Remove pointless stride adjustment for
 allocations >= PAGE_SIZE
Thread-Index: AQHaakuep9s0mIf3eE+om+sQlmyZtbEg06/QgAAR1sA=
Date: Thu, 29 Feb 2024 07:36:05 +0000
Message-ID:
 <SN6PR02MB41577D09E97B1D9645369D58D45F2@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240228133930.15400-1-will@kernel.org>
 <20240228133930.15400-7-will@kernel.org>
 <SN6PR02MB4157A62353559DA8DB8BC4ADD45F2@SN6PR02MB4157.namprd02.prod.outlook.com>
In-Reply-To:
 <SN6PR02MB4157A62353559DA8DB8BC4ADD45F2@SN6PR02MB4157.namprd02.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?Wk5AEgXsaiOIzdsBB6bjKxrKdulyQOFErOCnkwIVy1fO8m+qLW9nm5d/+q49?=
 =?us-ascii?Q?hgsGMigGP3Xh54+kNVbItUQo1yORp7cEZJX7ci40nmEEA4e2kIs71z8NIKPZ?=
 =?us-ascii?Q?9oD7dZVK6dRi8T2T0aJ1UgqOZl2Zxbave6yU6At5DkHePgsDqrBuNJK5sC3F?=
 =?us-ascii?Q?nrnDgFOH8qkDYrt7CDYa1tuTf6l4Np0iyolO2SQT+v2QPxZSujWTOv16KAhK?=
 =?us-ascii?Q?CnB8Y0ryhzAzXNBb/FHx1cNJQ4LWsi60G1aqXks6RPI5Y6Zj3WSYNAcHXrtE?=
 =?us-ascii?Q?rtA8GAMYaDtzLZBiJwdWfD++ZDJHnmB6RP/JRDxjQfH14i5JuUCZH7Q5AwhN?=
 =?us-ascii?Q?28SJ2XuwELMXv8usHKGlNEQotbyycKf3/0I5d2zSu0Er7T9HbL2m3jsywY3R?=
 =?us-ascii?Q?bn5pLgr6e5cLbTyQaiUluWL9zhUNByLXHmIeiNjfWaZ1ANFZU6WAX3/mOEHM?=
 =?us-ascii?Q?lxyKHc2831obYJYRG7YjHo8U8CsG+zA9suAeeGDLSkHKinEiqCPHF35QPiP0?=
 =?us-ascii?Q?j+FRXQvB9kqPe/p3lEB/dZtXu3wLDnTGaeSx4wFWEYQRGc5O8AKhYaVHThEm?=
 =?us-ascii?Q?YT/b6FbYlx8G/oFWL5VDdmrG1DulS2sCMIHcTPj+JDdg4l+WXr1RELEtDVpa?=
 =?us-ascii?Q?NkPDEtaJ+9dnY6QcjS30Hg1FX2iD8/1pgXqgtHsC6i4Ab5oWcy6iOKG0a9TZ?=
 =?us-ascii?Q?99s4XijrIVooq4OAolm85/u4ngqCzZN1oME4AsUDf0neArVXwuEtrGXcDa0H?=
 =?us-ascii?Q?qEcPXvsR7Vmvw9HHFGfqKzvpEBe/L/IBuBhvBeySCsjOPBza1V5oJdG+8xRB?=
 =?us-ascii?Q?kMk/SecBmtohoNFRThEz2WYIxCUGyu+VwUjGXcpKWQpof4gfJXKTYXQSXxvw?=
 =?us-ascii?Q?NPK1r99ux0J+5MfQRKAeZ69nJFPo6sAy1AB/3d/NH/Bq+v5L7lnA+jRBxZf2?=
 =?us-ascii?Q?ljqWTYfKXsEzagbjmZR07/GCFCqqvXN1mxdaS297FTDhrTfPdF7LnxiH4ZyE?=
 =?us-ascii?Q?LZM802id2iujW+3vQ7wUdjxWWjAsBNkBrgEskbzjWjHNDu+0jni8ezu0+BOx?=
 =?us-ascii?Q?RTwSw1c6OLrp8l1o6hkyV43PboMW2vM38Ki+zKIIHFcmErIupOCq6I5ULjoG?=
 =?us-ascii?Q?YMjerIXTcdEzrUWAHMC/ixMjtbK6ghB1LbiMWM+m6i35drhsonznvz7zMV/H?=
 =?us-ascii?Q?dAkzJCOgPqF3odFnIPpBz05sXWf4eMRTrJWMKg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: c51196c4-7580-4fa7-4db3-08dc38f91670
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Feb 2024 07:36:05.3677
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR02MB8022

> From: Will Deacon <will@kernel.org> Sent: Wednesday, February 28, 2024 5:=
40 AM
> >
> > For swiotlb allocations >=3D PAGE_SIZE, the slab search historically
> > adjusted the stride to avoid checking unaligned slots. However, this is
> > no longer needed now that the code around it has evolved and the
> > stride is calculated from the required alignment.
> >
> > Either 'alloc_align_mask' is used to specify the allocation alignment o=
r
> > the DMA 'min_align_mask' is used to align the allocation with 'orig_add=
r'.
> > At least one of these masks is always non-zero.
>=20
> I think the patch is correct, but this justification is not.  alloc_align=
_mask
> and the DMA min_align_mask are often both zero.  While the NVMe
> PCI driver sets min_align_mask, SCSI disk drivers do not (except for the
> Hyper-V synthetic SCSI driver).   When both are zero, presumably
> there are no alignment requirements, so a stride of 1 is appropriate.
>=20

I did additional checking into the history of page alignment for alloc
sizes of a page or greater. The swiotlb code probably made sense
prior to commit 0eee5ae10256. This commit has this change:

        slot_base =3D area_index * mem->area_nslabs;
-       index =3D wrap_area_index(mem, ALIGN(area->index, stride));
+       index =3D area->index;

        for (slots_checked =3D 0; slots_checked < mem->area_nslabs; ) {
                slot_index =3D slot_base + index;

In the old code, once the PAGE_SIZE was factored into the stride, the
stride was used to align the starting index, so that the first slot checked
would be page aligned. But commit 0eee5ae10256 drops that use
of the stride and puts the page alignment in iotlb_align_mask, for
reasons explained in the commit message. But in Will's Patch 1
when the page alignment is no longer incorporated into
iotlb_align_mask (for good reason), page aligning the stride then
doesn't do anything useful, resulting in this patch.

If there *is* a requirement for page alignment of page-size-or-greater
requests, even when alloc_align_mask and min_align_mask are zero,
we need to think about how to do that correctly, as that requirement
is no longer met after Patch 1 of this series.

Michael