* [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT
@ 2015-10-16 15:33 Robin Murphy
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Robin Murphy @ 2015-10-16 15:33 UTC (permalink / raw)
To: linux-arm-kernel
The imminent renaming of __GFP_WAIT in the mm tree conflicts with its
use in the new IOMMU DMA ops; introduce a temporary local version of
its replacement to smooth over the transition.
This patch should be reverted at 4.4-rc1.
CC: Mel Gorman <mgorman@techsingularity.net>
CC: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
Sudeep points out that there are pending changes in -next touching arm64
which I hadn't spotted, which end up breaking the build when merged with
my changes in the IOMMU tree. Catalin, would you mind acking these fixes
so that Joerg can carry them? We should be able to send the revert through
arm64 once the dust has settled.
Thanks,
Robin.
---
arch/arm64/mm/dma-mapping.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 6320361..66444df 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -546,6 +546,10 @@ static void flush_page(struct device *dev, const void *virt, phys_addr_t phys)
__dma_flush_range(virt, virt + PAGE_SIZE);
}
+#ifdef __GFP_WAIT
+#define gfpflags_allow_blocking(gfp) ((gfp) & __GFP_WAIT)
+#endif
+
static void *__iommu_alloc_attrs(struct device *dev, size_t size,
dma_addr_t *handle, gfp_t gfp,
struct dma_attrs *attrs)
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
2015-10-16 15:33 [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Robin Murphy
@ 2015-10-16 15:33 ` Robin Murphy
2015-10-16 16:20 ` Catalin Marinas
2015-10-16 20:59 ` Andrew Morton
2015-10-16 16:20 ` [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Catalin Marinas
2015-10-28 0:53 ` Joerg Roedel
2 siblings, 2 replies; 9+ messages in thread
From: Robin Murphy @ 2015-10-16 15:33 UTC (permalink / raw)
To: linux-arm-kernel
__GFP_WAIT is going away to live its life under a new identity; convert
__iommu_alloc_attrs() to the new helper function instead.
CC: Mel Gorman <mgorman@techsingularity.net>
CC: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
arch/arm64/mm/dma-mapping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 66444df..40cd4af 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
*/
gfp |= __GFP_ZERO;
- if (gfp & __GFP_WAIT) {
+ if (gfpflags_allow_blocking(gfp)) {
struct page **pages;
pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
@ 2015-10-16 16:20 ` Catalin Marinas
2015-10-16 20:59 ` Andrew Morton
1 sibling, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2015-10-16 16:20 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 16, 2015 at 04:33:42PM +0100, Robin Murphy wrote:
> __GFP_WAIT is going away to live its life under a new identity; convert
> __iommu_alloc_attrs() to the new helper function instead.
>
> CC: Mel Gorman <mgorman@techsingularity.net>
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
2015-10-16 16:20 ` Catalin Marinas
@ 2015-10-16 20:59 ` Andrew Morton
2015-10-19 12:43 ` Robin Murphy
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2015-10-16 20:59 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Oct 2015 16:33:42 +0100 Robin Murphy <robin.murphy@arm.com> wrote:
> __GFP_WAIT is going away to live its life under a new identity; convert
> __iommu_alloc_attrs() to the new helper function instead.
>
> ...
>
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
> */
> gfp |= __GFP_ZERO;
>
> - if (gfp & __GFP_WAIT) {
> + if (gfpflags_allow_blocking(gfp)) {
> struct page **pages;
> pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
Seems unnecessarily elaborate. What's wrong with
--- a/arch/arm64/mm/dma-mapping.c~mm-page_alloc-rename-__gfp_wait-to-__gfp_reclaim-arm-fix
+++ a/arch/arm64/mm/dma-mapping.c
@@ -562,7 +562,7 @@ static void *__iommu_alloc_attrs(struct
*/
gfp |= __GFP_ZERO;
- if (gfp & __GFP_WAIT) {
+ if (gfp & __GFP_RECLAIM) {
struct page **pages;
pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
2015-10-16 20:59 ` Andrew Morton
@ 2015-10-19 12:43 ` Robin Murphy
2015-10-19 13:26 ` Mel Gorman
0 siblings, 1 reply; 9+ messages in thread
From: Robin Murphy @ 2015-10-19 12:43 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andrew,
On 16/10/15 21:59, Andrew Morton wrote:
> On Fri, 16 Oct 2015 16:33:42 +0100 Robin Murphy <robin.murphy@arm.com> wrote:
>
>> __GFP_WAIT is going away to live its life under a new identity; convert
>> __iommu_alloc_attrs() to the new helper function instead.
>>
>> ...
>>
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>> */
>> gfp |= __GFP_ZERO;
>>
>> - if (gfp & __GFP_WAIT) {
>> + if (gfpflags_allow_blocking(gfp)) {
>> struct page **pages;
>> pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
>
> Seems unnecessarily elaborate. What's wrong with
>
> --- a/arch/arm64/mm/dma-mapping.c~mm-page_alloc-rename-__gfp_wait-to-__gfp_reclaim-arm-fix
> +++ a/arch/arm64/mm/dma-mapping.c
> @@ -562,7 +562,7 @@ static void *__iommu_alloc_attrs(struct
> */
> gfp |= __GFP_ZERO;
>
> - if (gfp & __GFP_WAIT) {
> + if (gfp & __GFP_RECLAIM) {
> struct page **pages;
> pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
>
>
> ?
Well, in that case the charge of "unnecessarily elaborate" should have
been directed at the original patch, and the 53 other locations where
(flags & __GFP_WAIT) was changed as per the commit message:
"Callers that are checking if they are non-blocking should use the
helper gfpflags_allow_blocking() where possible."
More importantly, it's also now apparently inconsistent with all the
other dma_alloc_coherent() implementations, which thanks to the helper
function are testing against __GFP_DIRECT_RECLAIM instead. As I have no
clear understanding of what the difference between the two flags is, and
how they relate to whether it's safe to call map_vm_area() or not (which
is what principally matters here), I'm very uncomfortable with a change
introducing that inconsistency.
If instead of my two patches you'd prefer to carry a fix through -mm and
coordinate with Joerg and Linus to ensure everything gets merged in the
right order, that's fine by me, but either way the change needs to
guarantee the same behaviour as all the other instances in arch/arm and
arch/arm64 which return a remapped buffer (and I have to say personally
I much prefer having the rather inscrutable flag logic hidden behind a
clearly-named helper function).
Thanks,
Robin.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
2015-10-19 12:43 ` Robin Murphy
@ 2015-10-19 13:26 ` Mel Gorman
0 siblings, 0 replies; 9+ messages in thread
From: Mel Gorman @ 2015-10-19 13:26 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 19, 2015 at 01:43:13PM +0100, Robin Murphy wrote:
> Hi Andrew,
>
> On 16/10/15 21:59, Andrew Morton wrote:
> >On Fri, 16 Oct 2015 16:33:42 +0100 Robin Murphy <robin.murphy@arm.com> wrote:
> >
> >>__GFP_WAIT is going away to live its life under a new identity; convert
> >>__iommu_alloc_attrs() to the new helper function instead.
> >>
> >>...
> >>
> >>--- a/arch/arm64/mm/dma-mapping.c
> >>+++ b/arch/arm64/mm/dma-mapping.c
> >>@@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
> >> */
> >> gfp |= __GFP_ZERO;
> >>
> >>- if (gfp & __GFP_WAIT) {
> >>+ if (gfpflags_allow_blocking(gfp)) {
> >> struct page **pages;
> >> pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >Seems unnecessarily elaborate. What's wrong with
> >
> >--- a/arch/arm64/mm/dma-mapping.c~mm-page_alloc-rename-__gfp_wait-to-__gfp_reclaim-arm-fix
> >+++ a/arch/arm64/mm/dma-mapping.c
> >@@ -562,7 +562,7 @@ static void *__iommu_alloc_attrs(struct
> > */
> > gfp |= __GFP_ZERO;
> >
> >- if (gfp & __GFP_WAIT) {
> >+ if (gfp & __GFP_RECLAIM) {
> > struct page **pages;
> > pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >
> >?
>
> Well, in that case the charge of "unnecessarily elaborate" should have been
> directed at the original patch, and the 53 other locations where (flags &
> __GFP_WAIT) was changed as per the commit message:
>
> "Callers that are checking if they are non-blocking should use the
> helper gfpflags_allow_blocking() where possible."
>
The use of gfpflags_allows_blocking() like you originally had is actually
preferred by me. __GFP_RECLAIM can return true when the caller only allows
kswapd to wake which has nothing to do with blocking (currently). The
helper was added to avoid this type of confusion.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT
2015-10-16 15:33 [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Robin Murphy
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
@ 2015-10-16 16:20 ` Catalin Marinas
2015-10-28 0:53 ` Joerg Roedel
2 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2015-10-16 16:20 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 16, 2015 at 04:33:41PM +0100, Robin Murphy wrote:
> The imminent renaming of __GFP_WAIT in the mm tree conflicts with its
> use in the new IOMMU DMA ops; introduce a temporary local version of
> its replacement to smooth over the transition.
>
> This patch should be reverted at 4.4-rc1.
>
> CC: Mel Gorman <mgorman@techsingularity.net>
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT
2015-10-16 15:33 [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Robin Murphy
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
2015-10-16 16:20 ` [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Catalin Marinas
@ 2015-10-28 0:53 ` Joerg Roedel
2015-10-28 11:01 ` Robin Murphy
2 siblings, 1 reply; 9+ messages in thread
From: Joerg Roedel @ 2015-10-28 0:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi Robin,
On Fri, Oct 16, 2015 at 04:33:41PM +0100, Robin Murphy wrote:
> The imminent renaming of __GFP_WAIT in the mm tree conflicts with its
> use in the new IOMMU DMA ops; introduce a temporary local version of
> its replacement to smooth over the transition.
>
> This patch should be reverted at 4.4-rc1.
I lost track here, are these two patches required in the iommu tree for
the next merge window?
Joerg
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT
2015-10-28 0:53 ` Joerg Roedel
@ 2015-10-28 11:01 ` Robin Murphy
0 siblings, 0 replies; 9+ messages in thread
From: Robin Murphy @ 2015-10-28 11:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Joerg,
On 28/10/15 00:53, Joerg Roedel wrote:
> Hi Robin,
>
> On Fri, Oct 16, 2015 at 04:33:41PM +0100, Robin Murphy wrote:
>> The imminent renaming of __GFP_WAIT in the mm tree conflicts with its
>> use in the new IOMMU DMA ops; introduce a temporary local version of
>> its replacement to smooth over the transition.
>>
>> This patch should be reverted at 4.4-rc1.
>
> I lost track here, are these two patches required in the iommu tree for
> the next merge window?
Andrew has the equivalent of patch 2 as a fixup on top of Mel's series,
which Stephen is currently carrying as a merge resolution in -next. I'm
assuming this will go into the final merge as well, so as long as Linus
takes the IOMMU tree first we should be OK as-is.
Robin.
>
>
> Joerg
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-28 11:01 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-16 15:33 [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Robin Murphy
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
2015-10-16 16:20 ` Catalin Marinas
2015-10-16 20:59 ` Andrew Morton
2015-10-19 12:43 ` Robin Murphy
2015-10-19 13:26 ` Mel Gorman
2015-10-16 16:20 ` [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Catalin Marinas
2015-10-28 0:53 ` Joerg Roedel
2015-10-28 11:01 ` Robin Murphy
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).