| Message ID | 928b5df7-fada-cf2f-6f6a-257a84547c3@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | swiotlb-xen: provide the "max_mapping_size" method | expand |
On Mon, Nov 06, 2023 at 03:59:40PM +0100, Mikulas Patocka wrote: > There's a bug that when using the XEN hypervisor with dm-crypt on NVMe, > the kernel deadlocks [1]. > > The deadlocks are caused by inability to map a large bio vector - > dma_map_sgtable always returns an error, this gets propagated to the block > layer as BLK_STS_RESOURCE and the block layer retries the request > indefinitely. > > XEN uses the swiotlb framework to map discontiguous pages into contiguous > runs that are submitted to the PCIe device. The swiotlb framework has a > limitation on the length of a mapping - this needs to be announced with > the max_mapping_size method to make sure that the hardware drivers do not > create larger mappings. > > Without max_mapping_size, the NVMe block driver would create large > mappings that overrun the maximum mapping size. > > [1] https://lore.kernel.org/stable/ZTNH0qtmint%2FzLJZ@mail-itl/ This should be a "Link:" tag. > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Reported-by: Marek Marczykowski-G'orecki <marmarek@invisiblethingslab.com> > Tested-by: Marek Marczykowski-G'orecki <marmarek@invisiblethingslab.com> > Suggested-by: Keith Busch <kbusch@kernel.org> I was about to send the same thing. I did a little more than suggest this: it's is the very patch I wrote for testing, minus the redundant nvme bits! But since you already have a commit message for it... Acked-by: Keith Busch <kbusch@kernel.org> > Suggested-by: Christoph Hellwig <hch@lst.de> > Cc: stable@vger.kernel.org > > --- > drivers/xen/swiotlb-xen.c | 1 + > 1 file changed, 1 insertion(+) > > Index: linux-stable/drivers/xen/swiotlb-xen.c > =================================================================== > --- linux-stable.orig/drivers/xen/swiotlb-xen.c 2023-11-03 17:57:18.000000000 +0100 > +++ linux-stable/drivers/xen/swiotlb-xen.c 2023-11-06 15:30:59.000000000 +0100 > @@ -405,4 +405,5 @@ const struct dma_map_ops xen_swiotlb_dma > .get_sgtable = dma_common_get_sgtable, > .alloc_pages = dma_common_alloc_pages, > .free_pages = dma_common_free_pages, > + .max_mapping_size = swiotlb_max_mapping_size, > };
On Mon, Nov 06 2023 at 10:16P -0500, Keith Busch <kbusch@kernel.org> wrote: > On Mon, Nov 06, 2023 at 03:59:40PM +0100, Mikulas Patocka wrote: > > There's a bug that when using the XEN hypervisor with dm-crypt on NVMe, > > the kernel deadlocks [1]. > > > > The deadlocks are caused by inability to map a large bio vector - > > dma_map_sgtable always returns an error, this gets propagated to the block > > layer as BLK_STS_RESOURCE and the block layer retries the request > > indefinitely. > > > > XEN uses the swiotlb framework to map discontiguous pages into contiguous > > runs that are submitted to the PCIe device. The swiotlb framework has a > > limitation on the length of a mapping - this needs to be announced with > > the max_mapping_size method to make sure that the hardware drivers do not > > create larger mappings. > > > > Without max_mapping_size, the NVMe block driver would create large > > mappings that overrun the maximum mapping size. > > > > [1] https://lore.kernel.org/stable/ZTNH0qtmint%2FzLJZ@mail-itl/ > > This should be a "Link:" tag. > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > Reported-by: Marek Marczykowski-G'orecki <marmarek@invisiblethingslab.com> > > Tested-by: Marek Marczykowski-G'orecki <marmarek@invisiblethingslab.com> > > Suggested-by: Keith Busch <kbusch@kernel.org> > > I was about to send the same thing. I did a little more than suggest > this: it's is the very patch I wrote for testing, minus the redundant > nvme bits! But since you already have a commit message for it... > > Acked-by: Keith Busch <kbusch@kernel.org> No, this patch should be attributed to you Keith. Mikulas, I like that you ran with getting a fix prepared but please update the patch so Keith is the author and use Link: as suggested for the v2. Note: you'll still use your Signed-off-by since you had a role in getting this patch together (but please move yours to the end of the header). Mike > > > Suggested-by: Christoph Hellwig <hch@lst.de> > > Cc: stable@vger.kernel.org > > > > --- > > drivers/xen/swiotlb-xen.c | 1 + > > 1 file changed, 1 insertion(+) > > > > Index: linux-stable/drivers/xen/swiotlb-xen.c > > =================================================================== > > --- linux-stable.orig/drivers/xen/swiotlb-xen.c 2023-11-03 17:57:18.000000000 +0100 > > +++ linux-stable/drivers/xen/swiotlb-xen.c 2023-11-06 15:30:59.000000000 +0100 > > @@ -405,4 +405,5 @@ const struct dma_map_ops xen_swiotlb_dma > > .get_sgtable = dma_common_get_sgtable, > > .alloc_pages = dma_common_alloc_pages, > > .free_pages = dma_common_free_pages, > > + .max_mapping_size = swiotlb_max_mapping_size, > > }; >
Index: linux-stable/drivers/xen/swiotlb-xen.c =================================================================== --- linux-stable.orig/drivers/xen/swiotlb-xen.c 2023-11-03 17:57:18.000000000 +0100 +++ linux-stable/drivers/xen/swiotlb-xen.c 2023-11-06 15:30:59.000000000 +0100 @@ -405,4 +405,5 @@ const struct dma_map_ops xen_swiotlb_dma .get_sgtable = dma_common_get_sgtable, .alloc_pages = dma_common_alloc_pages, .free_pages = dma_common_free_pages, + .max_mapping_size = swiotlb_max_mapping_size, };