arm64: mm: fix __page_to_voff definition

PATCH New Oleksandr Andrushchenko 2 participants 6 messages Jan 17, 2017 arm64

lore

Share & tools

lore

Oleksandr Andrushchenko

author

2017-01-17 15:23 UTC #0 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

From: Oleksandr Andrushchenko <[email protected]>

Fix parameter name for __page_to_voff, to match its definition. There are other callers of page_to_virt which do not declare 'page'.

Fixes 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 arch/arm64/include/asm/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/memory.h +1 -1

index bfe632808d77..90c39a662379 100644
@@ -222,7 +222,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 #define _virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 #else
 #define __virt_to_pgoff(kaddr)	(((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(page)	(((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(kaddr)	(((u64)(kaddr) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
 
 #define page_to_virt(page)	((void *)((__page_to_voff(page)) | PAGE_OFFSET))
 #define virt_to_page(vaddr)	((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))

Signature · 1 line

2.7.4

Mark Rutland

reviewer

2017-01-17 16:15 UTC #1 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

Quoted 8 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>
Fix parameter name for __page_to_voff, to match its definition. There are other callers of page_to_virt which do not declare 'page'.
Fixes 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")
Signed-off-by Oleksandr Andrushchenko <[email protected]>

I take it this is because it messes up the 'struct page' at the end of the macro expansion?

It would be good to explicitly state that, since that's not immediately obvious.

With that:

Acked-by Mark Rutland <[email protected]>

Thanks,
Mark.

Quoted 25 lines

---
 arch/arm64/include/asm/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/memory.h +1 -1

index bfe632808d77..90c39a662379 100644
@@ -222,7 +222,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 #define _virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 #else
 #define __virt_to_pgoff(kaddr)	(((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(page)	(((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(kaddr)	(((u64)(kaddr) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
 
 #define page_to_virt(page)	((void *)((__page_to_voff(page)) | PAGE_OFFSET))
 #define virt_to_page(vaddr)	((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))

Signature · 7 lines

2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Oleksandr Andrushchenko

author

2017-01-17 17:13 UTC #2 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

Quoted 11 lines · Mark Rutland

Quoted 8 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
Fix parameter name for __page_to_voff, to match its definition. There are other callers of page_to_virt which do not declare 'page'.
Fixes 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")
Signed-off-by Oleksandr Andrushchenko <[email protected]>
I take it this is because it messes up the 'struct page' at the end of the macro expansion?

yes, you got it right

Quoted 2 lines

It would be good to explicitly state that, since that's not immediately obvious.

Will do that in patch v1

Quoted 4 lines

With that:
Acked-by Mark Rutland <[email protected]>

Can I put your ack into the v1?

Quoted 3 lines

Thanks,
Mark.

Thanks,
Oleksandr

Quoted 25 lines

---
  arch/arm64/include/asm/memory.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/memory.h +1 -1

index bfe632808d77..90c39a662379 100644
@@ -222,7 +222,7 @@ static inline void *phys_to_virt(phys_addr_t x)
  #define _virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
  #else
  #define __virt_to_pgoff(kaddr)	(((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(page)	(((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(kaddr)	(((u64)(kaddr) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
 
  #define page_to_virt(page)	((void *)((__page_to_voff(page)) | PAGE_OFFSET))
  #define virt_to_page(vaddr)	((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))

Signature · 7 lines

2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Mark Rutland

reviewer

2017-01-17 17:19 UTC #3 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

Quoted 13 lines · Oleksandr Andrushchenko

Quoted 11 lines · Mark Rutland
Quoted 8 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
Fix parameter name for __page_to_voff, to match its definition. There are other callers of page_to_virt which do not declare 'page'.
Fixes 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")
Signed-off-by Oleksandr Andrushchenko <[email protected]>
I take it this is because it messes up the 'struct page' at the end of the macro expansion?
yes, you got it right

Quoted 8 lines

Quoted 2 lines
It would be good to explicitly state that, since that's not immediately obvious.
Will do that in patch v1
Quoted 4 lines
With that:
Acked-by Mark Rutland <[email protected]>
Can I put your ack into the v1?

Yes, with the updated commit message and title, you can add my ack.

W.r.t. wording, I'd recommend:

arm64: mm: avoid name clash in __page_to_voff()

The arm64 __page_to_voff() macro taks a parameter called 'page', and also refers to 'struct page'. Thus, if the value passed in is not called 'page', we'll refer to the wrong struct name (which might not exist).

Rename the parameter so that it doesn't clash.

Thanks,
Mark.

Quoted 30 lines

Quoted 3 lines

Thanks,
Mark.

Thanks,
Oleksandr

Quoted 24 lines

---
 arch/arm64/include/asm/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/memory.h +1 -1

index bfe632808d77..90c39a662379 100644
@@ -222,7 +222,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 #define _virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 #else
 #define __virt_to_pgoff(kaddr)	(((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(page)	(((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(kaddr)	(((u64)(kaddr) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
 #define page_to_virt(page)	((void *)((__page_to_voff(page)) | PAGE_OFFSET))
 #define virt_to_page(vaddr)	((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))

Signature · 7 lines

2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Oleksandr Andrushchenko

author

2017-01-17 18:15 UTC #4 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

Quoted 25 lines · Mark Rutland

Quoted 12 lines · Oleksandr Andrushchenko
Quoted 11 lines · Mark Rutland
Quoted 8 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
Fix parameter name for __page_to_voff, to match its definition. There are other callers of page_to_virt which do not declare 'page'.
Fixes 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")
Signed-off-by Oleksandr Andrushchenko <[email protected]>
I take it this is because it messes up the 'struct page' at the end of the macro expansion?
Quoted 8 lines
yes, you got it right
Quoted 2 lines
It would be good to explicitly state that, since that's not immediately obvious.
Will do that in patch v1
Quoted 3 lines
With that:
Acked-by Mark Rutland <[email protected]>
Can I put your ack into the v1?
Yes, with the updated commit message and title, you can add my ack.
W.r.t. wording, I'd recommend:

Quoted 8 lines

arm64: mm: avoid name clash in __page_to_voff()
The arm64 __page_to_voff() macro taks a parameter called 'page', and also refers to 'struct page'. Thus, if the value passed in is not called 'page', we'll refer to the wrong struct name (which might not exist).
Rename the parameter so that it doesn't clash.

Great, thanks
Do you also want "Fixes: 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")"
line or it is good without it?

Quoted 31 lines

Thanks,
Mark.

Quoted 28 lines

Quoted 2 lines

Thanks,
Mark.

Thanks,
Oleksandr

Quoted 24 lines

---
  arch/arm64/include/asm/memory.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/include/asm/memory.h +1 -1

index bfe632808d77..90c39a662379 100644
@@ -222,7 +222,7 @@ static inline void *phys_to_virt(phys_addr_t x)
  #define _virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
  #else
  #define __virt_to_pgoff(kaddr)	(((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(page)	(((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(kaddr)	(((u64)(kaddr) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
  #define page_to_virt(page)	((void *)((__page_to_voff(page)) | PAGE_OFFSET))
  #define virt_to_page(vaddr)	((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))

Signature · 7 lines

2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Mark Rutland

reviewer

2017-01-17 18:38 UTC #5 lore raw

PATCH 1/1 arm64: mm: fix __page_to_voff definition

Quoted 4 lines · Oleksandr Andrushchenko

Do you also want "Fixes: 3fa72fe9c614 ("arm64: mm: fix __page_to_voff definition")"
From a look at the git history, that Fixes tag looks correct, so yes

please.

Thanks,
Mark.