* Re: [PATCH] iommu/iova: Improve 32-bit free space estimate [not found] <033815732d83ca73b13c11485ac39336f15c3b40.1646318408.git.robin.murphy@arm.com> @ 2022-03-03 23:36 ` Miles Chen 2022-03-04 9:41 ` Joerg Roedel 0 siblings, 1 reply; 4+ messages in thread From: Miles Chen @ 2022-03-03 23:36 UTC (permalink / raw) To: robin.murphy Cc: iommu, joro, linux-kernel, miles.chen, will, wsd_upstream, yf.wang, stable Hi Robin, > For various reasons based on the allocator behaviour and typical > use-cases at the time, when the max32_alloc_size optimisation was > introduced it seemed reasonable to couple the reset of the tracked > size to the update of cached32_node upon freeing a relevant IOVA. > However, since subsequent optimisations focused on helping genuine > 32-bit devices make best use of even more limited address spaces, it > is now a lot more likely for cached32_node to be anywhere in a "full" > 32-bit address space, and as such more likely for space to become > available from IOVAs below that node being freed. > > At this point, the short-cut in __cached_rbnode_delete_update() really > doesn't hold up any more, and we need to fix the logic to reliably > provide the expected behaviour. We still want cached32_node to only move > upwards, but we should reset the allocation size if *any* 32-bit space > has become available. > > Reported-by: Yunfei Wang <yf.wang@mediatek.com> > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Would you mind adding: Cc: <stable@vger.kernel.org> to this path? I checked and I think the patch can be applied to 5.4 and later. thanks, Miles ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iommu/iova: Improve 32-bit free space estimate 2022-03-03 23:36 ` [PATCH] iommu/iova: Improve 32-bit free space estimate Miles Chen @ 2022-03-04 9:41 ` Joerg Roedel 2022-03-04 11:32 ` Robin Murphy 0 siblings, 1 reply; 4+ messages in thread From: Joerg Roedel @ 2022-03-04 9:41 UTC (permalink / raw) To: Miles Chen Cc: robin.murphy, iommu, linux-kernel, will, wsd_upstream, yf.wang, stable On Fri, Mar 04, 2022 at 07:36:46AM +0800, Miles Chen wrote: > Hi Robin, > > > For various reasons based on the allocator behaviour and typical > > use-cases at the time, when the max32_alloc_size optimisation was > > introduced it seemed reasonable to couple the reset of the tracked > > size to the update of cached32_node upon freeing a relevant IOVA. > > However, since subsequent optimisations focused on helping genuine > > 32-bit devices make best use of even more limited address spaces, it > > is now a lot more likely for cached32_node to be anywhere in a "full" > > 32-bit address space, and as such more likely for space to become > > available from IOVAs below that node being freed. > > > > At this point, the short-cut in __cached_rbnode_delete_update() really > > doesn't hold up any more, and we need to fix the logic to reliably > > provide the expected behaviour. We still want cached32_node to only move > > upwards, but we should reset the allocation size if *any* 32-bit space > > has become available. > > > > Reported-by: Yunfei Wang <yf.wang@mediatek.com> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > > Would you mind adding: > > Cc: <stable@vger.kernel.org> Applied without stable tag for now. If needed, please consider re-sending it for stable when this patch is merged upstream. Regards, Joerg ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iommu/iova: Improve 32-bit free space estimate 2022-03-04 9:41 ` Joerg Roedel @ 2022-03-04 11:32 ` Robin Murphy 2022-03-05 0:03 ` Miles Chen 0 siblings, 1 reply; 4+ messages in thread From: Robin Murphy @ 2022-03-04 11:32 UTC (permalink / raw) To: Joerg Roedel, Miles Chen Cc: iommu, linux-kernel, will, wsd_upstream, yf.wang, stable On 2022-03-04 09:41, Joerg Roedel wrote: > On Fri, Mar 04, 2022 at 07:36:46AM +0800, Miles Chen wrote: >> Hi Robin, >> >>> For various reasons based on the allocator behaviour and typical >>> use-cases at the time, when the max32_alloc_size optimisation was >>> introduced it seemed reasonable to couple the reset of the tracked >>> size to the update of cached32_node upon freeing a relevant IOVA. >>> However, since subsequent optimisations focused on helping genuine >>> 32-bit devices make best use of even more limited address spaces, it >>> is now a lot more likely for cached32_node to be anywhere in a "full" >>> 32-bit address space, and as such more likely for space to become >>> available from IOVAs below that node being freed. >>> >>> At this point, the short-cut in __cached_rbnode_delete_update() really >>> doesn't hold up any more, and we need to fix the logic to reliably >>> provide the expected behaviour. We still want cached32_node to only move >>> upwards, but we should reset the allocation size if *any* 32-bit space >>> has become available. >>> >>> Reported-by: Yunfei Wang <yf.wang@mediatek.com> >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> >> Would you mind adding: >> >> Cc: <stable@vger.kernel.org> > > Applied without stable tag for now. If needed, please consider > re-sending it for stable when this patch is merged upstream. Yeah, having figured out the history, I ended up with the opinion that it was a missed corner-case optimisation opportunity, rather than an actual error with respect to intent or implementation, so I intentionally left that out. Plus figuring out an exact Fixes tag might be tricky - as above I reckon it probably only started to become significant somwehere around 5.11 or so. All of these various levels of retry mechanisms are only a best-effort thing, and ultimately if you're making large allocations from a small space there are always going to be *some* circumstances that still manage to defeat them. Over time, we've made them try harder, but that fact that we haven't yet made them try hard enough to work well for a particular use-case does not constitute a bug. However as Joerg says, anyone's welcome to make a case to Greg to backport a mainline commit if it's a low-risk change with significant benefit to real-world stable kernel users. Thanks all! Robin. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iommu/iova: Improve 32-bit free space estimate 2022-03-04 11:32 ` Robin Murphy @ 2022-03-05 0:03 ` Miles Chen 0 siblings, 0 replies; 4+ messages in thread From: Miles Chen @ 2022-03-05 0:03 UTC (permalink / raw) To: robin.murphy Cc: iommu, joro, linux-kernel, miles.chen, stable, will, wsd_upstream, yf.wang Hi Joerg, Robin, > Applied without stable tag for now. If needed, please consider > re-sending it for stable when this patch is merged upstream. > Yeah, having figured out the history, I ended up with the opinion that > it was a missed corner-case optimisation opportunity, rather than an > actual error with respect to intent or implementation, so I > intentionally left that out. Plus figuring out an exact Fixes tag might > be tricky - as above I reckon it probably only started to become > significant somwehere around 5.11 or so. > > All of these various levels of retry mechanisms are only a best-effort > thing, and ultimately if you're making large allocations from a small > space there are always going to be *some* circumstances that still > manage to defeat them. Over time, we've made them try harder, but that > fact that we haven't yet made them try hard enough to work well for a > particular use-case does not constitute a bug. However as Joerg says, > anyone's welcome to make a case to Greg to backport a mainline commit if > it's a low-risk change with significant benefit to real-world stable > kernel users. Got it, thank you. We will try to push to the android LTS trees we need. Thanks, Miles > > Thanks all! > > Robin. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-05 0:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <033815732d83ca73b13c11485ac39336f15c3b40.1646318408.git.robin.murphy@arm.com> 2022-03-03 23:36 ` [PATCH] iommu/iova: Improve 32-bit free space estimate Miles Chen 2022-03-04 9:41 ` Joerg Roedel 2022-03-04 11:32 ` Robin Murphy 2022-03-05 0:03 ` Miles Chen
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).