* Re: linux-next: Tree for Feb 9
2016-02-09 14:35 ` Mark Rutland
@ 2016-02-09 14:35 ` Mark Rutland
2016-02-09 14:38 ` Catalin Marinas
` (2 subsequent siblings)
3 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 14:35 UTC (permalink / raw)
To: Geert Uytterhoeven, Arnd Bergmann, Catalin Marinas
Cc: Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott
On Tue, Feb 09, 2016 at 02:48:26PM +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 9, 2016 at 1:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Tue, Feb 09, 2016 at 01:04:28PM +0530, Sudip Mukherjee wrote:
> >> On Tue, Feb 09, 2016 at 04:41:04PM +1100, Stephen Rothwell wrote:
> >> > Changes since 20160208:
> >>
> >> tilepro, tilegx, mips defconfig build fails with the error:
> >> ../include/asm-generic/fixmap.h: In function '__set_fixmap_offset':
> >> ../include/asm-generic/fixmap.h:77:2: error: implicit declaration of
> >> function '__set_fixmap' [-Werror=implicit-function-declaration]
> >>
> >> caused by:
> >> commit ac4c0ac73485 ("asm-generic: make __set_fixmap_offset a static inline")
> >>
> >> Reverting the commit fixes the issue.
> >
> > Sorry about this.
> >
> > Is seems any arch without its own __set_fixmap may be adversely
> > affected.
> >
> > I can't easily stub __set_fixmap as it's not implemented as a macro.
>
> But you can add a forward declaration?
Good point. That seems to work (tested on arm64, mips, tilegx).
Arnd, Catalin, happy with the bloew fixup to the original patch?
Mark.
---->8----
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 14:35 ` Mark Rutland
2016-02-09 14:35 ` Mark Rutland
@ 2016-02-09 14:38 ` Catalin Marinas
2016-02-09 15:08 ` Arnd Bergmann
2016-02-09 16:13 ` [PATCH] asm-generic: Fix build when __set_fixmap is absent kbuild test robot
3 siblings, 0 replies; 25+ messages in thread
From: Catalin Marinas @ 2016-02-09 14:38 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Arnd Bergmann, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tue, Feb 09, 2016 at 02:35:54PM +0000, Mark Rutland wrote:
> On Tue, Feb 09, 2016 at 02:48:26PM +0100, Geert Uytterhoeven wrote:
> > On Tue, Feb 9, 2016 at 1:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Tue, Feb 09, 2016 at 01:04:28PM +0530, Sudip Mukherjee wrote:
> > >> On Tue, Feb 09, 2016 at 04:41:04PM +1100, Stephen Rothwell wrote:
> > >> > Changes since 20160208:
> > >>
> > >> tilepro, tilegx, mips defconfig build fails with the error:
> > >> ../include/asm-generic/fixmap.h: In function '__set_fixmap_offset':
> > >> ../include/asm-generic/fixmap.h:77:2: error: implicit declaration of
> > >> function '__set_fixmap' [-Werror=implicit-function-declaration]
> > >>
> > >> caused by:
> > >> commit ac4c0ac73485 ("asm-generic: make __set_fixmap_offset a static inline")
> > >>
> > >> Reverting the commit fixes the issue.
> > >
> > > Sorry about this.
> > >
> > > Is seems any arch without its own __set_fixmap may be adversely
> > > affected.
> > >
> > > I can't easily stub __set_fixmap as it's not implemented as a macro.
> >
> > But you can add a forward declaration?
>
> Good point. That seems to work (tested on arm64, mips, tilegx).
>
> Arnd, Catalin, happy with the bloew fixup to the original patch?
It looks fine to me. But I'll wait for Arnd's ack before pushing it.
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 14:35 ` Mark Rutland
2016-02-09 14:35 ` Mark Rutland
2016-02-09 14:38 ` Catalin Marinas
@ 2016-02-09 15:08 ` Arnd Bergmann
2016-02-09 15:08 ` Arnd Bergmann
2016-02-09 16:01 ` Mark Rutland
2016-02-09 16:13 ` [PATCH] asm-generic: Fix build when __set_fixmap is absent kbuild test robot
3 siblings, 2 replies; 25+ messages in thread
From: Arnd Bergmann @ 2016-02-09 15:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tuesday 09 February 2016 14:35:54 Mark Rutland wrote:
> diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> index f9c27b6..e5255ff 100644
> --- a/include/asm-generic/fixmap.h
> +++ b/include/asm-generic/fixmap.h
> @@ -69,6 +69,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
> #endif
>
> +void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> +
> /* Return a pointer with offset calculated */
> static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> phys_addr_t phys,
>
I think there is a conflicting declaration in arch/x86/include/asm/paravirt.h:
static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
phys_addr_t phys, pgprot_t flags)
{
pv_mmu_ops.set_fixmap(idx, phys, flags);
}
Can you test on x86 with CONFIG_PARAVIRT set?
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 15:08 ` Arnd Bergmann
@ 2016-02-09 15:08 ` Arnd Bergmann
2016-02-09 16:01 ` Mark Rutland
1 sibling, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2016-02-09 15:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tuesday 09 February 2016 14:35:54 Mark Rutland wrote:
> diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> index f9c27b6..e5255ff 100644
> --- a/include/asm-generic/fixmap.h
> +++ b/include/asm-generic/fixmap.h
> @@ -69,6 +69,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
> #endif
>
> +void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> +
> /* Return a pointer with offset calculated */
> static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> phys_addr_t phys,
>
I think there is a conflicting declaration in arch/x86/include/asm/paravirt.h:
static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
phys_addr_t phys, pgprot_t flags)
{
pv_mmu_ops.set_fixmap(idx, phys, flags);
}
Can you test on x86 with CONFIG_PARAVIRT set?
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 15:08 ` Arnd Bergmann
2016-02-09 15:08 ` Arnd Bergmann
@ 2016-02-09 16:01 ` Mark Rutland
2016-02-09 16:01 ` Mark Rutland
2016-02-09 16:08 ` Arnd Bergmann
1 sibling, 2 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 16:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tue, Feb 09, 2016 at 04:08:03PM +0100, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 14:35:54 Mark Rutland wrote:
> > diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> > index f9c27b6..e5255ff 100644
> > --- a/include/asm-generic/fixmap.h
> > +++ b/include/asm-generic/fixmap.h
> > @@ -69,6 +69,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> > __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
> > #endif
> >
> > +void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > +
> > /* Return a pointer with offset calculated */
> > static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> > phys_addr_t phys,
> >
>
>
> I think there is a conflicting declaration in arch/x86/include/asm/paravirt.h:
>
> static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
> phys_addr_t phys, pgprot_t flags)
> {
> pv_mmu_ops.set_fixmap(idx, phys, flags);
> }
>
> Can you test on x86 with CONFIG_PARAVIRT set?
That builds fine for me atop of for-next/pgtable, both 64-bit and
32-bit.
GCC seems to treat enum fixed_addresses the same as unsigned. Only if I
change the type of idx in fixmap.h (e.g. to char) do I get a conflict
against paravirt.h
Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 16:01 ` Mark Rutland
@ 2016-02-09 16:01 ` Mark Rutland
2016-02-09 16:08 ` Arnd Bergmann
1 sibling, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 16:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tue, Feb 09, 2016 at 04:08:03PM +0100, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 14:35:54 Mark Rutland wrote:
> > diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> > index f9c27b6..e5255ff 100644
> > --- a/include/asm-generic/fixmap.h
> > +++ b/include/asm-generic/fixmap.h
> > @@ -69,6 +69,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> > __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
> > #endif
> >
> > +void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > +
> > /* Return a pointer with offset calculated */
> > static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> > phys_addr_t phys,
> >
>
>
> I think there is a conflicting declaration in arch/x86/include/asm/paravirt.h:
>
> static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
> phys_addr_t phys, pgprot_t flags)
> {
> pv_mmu_ops.set_fixmap(idx, phys, flags);
> }
>
> Can you test on x86 with CONFIG_PARAVIRT set?
That builds fine for me atop of for-next/pgtable, both 64-bit and
32-bit.
GCC seems to treat enum fixed_addresses the same as unsigned. Only if I
change the type of idx in fixmap.h (e.g. to char) do I get a conflict
against paravirt.h
Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: linux-next: Tree for Feb 9
2016-02-09 16:01 ` Mark Rutland
2016-02-09 16:01 ` Mark Rutland
@ 2016-02-09 16:08 ` Arnd Bergmann
2016-02-09 16:08 ` Arnd Bergmann
1 sibling, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tuesday 09 February 2016 16:01:18 Mark Rutland wrote:
> That builds fine for me atop of for-next/pgtable, both 64-bit and
> 32-bit.
>
> GCC seems to treat enum fixed_addresses the same as unsigned. Only if I
> change the type of idx in fixmap.h (e.g. to char) do I get a conflict
> against paravirt.h
Interesting.
The patch seems fine then:
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: linux-next: Tree for Feb 9
2016-02-09 16:08 ` Arnd Bergmann
@ 2016-02-09 16:08 ` Arnd Bergmann
0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Catalin Marinas, Sudip Mukherjee,
Stephen Rothwell, Linux-Next, linux-kernel@vger.kernel.org,
Ard Biesheuvel, Jeremy Linton, Linux-Arch, Laura Abbott
On Tuesday 09 February 2016 16:01:18 Mark Rutland wrote:
> That builds fine for me atop of for-next/pgtable, both 64-bit and
> 32-bit.
>
> GCC seems to treat enum fixed_addresses the same as unsigned. Only if I
> change the type of idx in fixmap.h (e.g. to char) do I get a conflict
> against paravirt.h
Interesting.
The patch seems fine then:
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 14:35 ` Mark Rutland
` (2 preceding siblings ...)
2016-02-09 15:08 ` Arnd Bergmann
@ 2016-02-09 16:13 ` kbuild test robot
2016-02-09 16:13 ` kbuild test robot
2016-02-09 16:33 ` Mark Rutland
3 siblings, 2 replies; 25+ messages in thread
From: kbuild test robot @ 2016-02-09 16:13 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild-all, Geert Uytterhoeven, Arnd Bergmann, Catalin Marinas,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott
[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]
Hi Mark,
[auto build test ERROR on next-20160209]
[also build test ERROR on v4.5-rc3]
[cannot apply to v4.5-rc3 v4.5-rc2 v4.5-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Mark-Rutland/asm-generic-Fix-build-when-__set_fixmap-is-absent/20160209-223916
config: um-alldefconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=um
All errors (new ones prefixed by >>):
In file included from arch/um/include/asm/fixmap.h:54:0,
from arch/um/include/asm/pgtable.h:11,
from include/linux/mm.h:67,
from include/linux/ring_buffer.h:5,
from include/linux/trace_events.h:5,
from include/trace/syscall.h:6,
from include/linux/syscalls.h:81,
from init/main.c:18:
>> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
^
In file included from arch/um/include/asm/pgtable.h:11:0,
from include/linux/mm.h:67,
from include/linux/ring_buffer.h:5,
from include/linux/trace_events.h:5,
from include/trace/syscall.h:6,
from include/linux/syscalls.h:81,
from init/main.c:18:
arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
extern void __set_fixmap (enum fixed_addresses idx,
^
vim +/__set_fixmap +72 include/asm-generic/fixmap.h
66
67 #ifndef clear_fixmap
68 #define clear_fixmap(idx) \
69 __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
70 #endif
71
> 72 void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
73
74 /* Return a pointer with offset calculated */
75 static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 4141 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:13 ` [PATCH] asm-generic: Fix build when __set_fixmap is absent kbuild test robot
@ 2016-02-09 16:13 ` kbuild test robot
2016-02-09 16:33 ` Mark Rutland
1 sibling, 0 replies; 25+ messages in thread
From: kbuild test robot @ 2016-02-09 16:13 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild-all, Geert Uytterhoeven, Arnd Bergmann, Catalin Marinas,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott
[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]
Hi Mark,
[auto build test ERROR on next-20160209]
[also build test ERROR on v4.5-rc3]
[cannot apply to v4.5-rc3 v4.5-rc2 v4.5-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Mark-Rutland/asm-generic-Fix-build-when-__set_fixmap-is-absent/20160209-223916
config: um-alldefconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=um
All errors (new ones prefixed by >>):
In file included from arch/um/include/asm/fixmap.h:54:0,
from arch/um/include/asm/pgtable.h:11,
from include/linux/mm.h:67,
from include/linux/ring_buffer.h:5,
from include/linux/trace_events.h:5,
from include/trace/syscall.h:6,
from include/linux/syscalls.h:81,
from init/main.c:18:
>> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
^
In file included from arch/um/include/asm/pgtable.h:11:0,
from include/linux/mm.h:67,
from include/linux/ring_buffer.h:5,
from include/linux/trace_events.h:5,
from include/trace/syscall.h:6,
from include/linux/syscalls.h:81,
from init/main.c:18:
arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
extern void __set_fixmap (enum fixed_addresses idx,
^
vim +/__set_fixmap +72 include/asm-generic/fixmap.h
66
67 #ifndef clear_fixmap
68 #define clear_fixmap(idx) \
69 __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR)
70 #endif
71
> 72 void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
73
74 /* Return a pointer with offset calculated */
75 static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 4141 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:13 ` [PATCH] asm-generic: Fix build when __set_fixmap is absent kbuild test robot
2016-02-09 16:13 ` kbuild test robot
@ 2016-02-09 16:33 ` Mark Rutland
2016-02-09 16:33 ` Mark Rutland
` (2 more replies)
1 sibling, 3 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 16:33 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Geert Uytterhoeven, Arnd Bergmann, Catalin Marinas,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
> >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> ^
> In file included from arch/um/include/asm/pgtable.h:11:0,
> from include/linux/mm.h:67,
> from include/linux/ring_buffer.h:5,
> from include/linux/trace_events.h:5,
> from include/trace/syscall.h:6,
> from include/linux/syscalls.h:81,
> from init/main.c:18:
> arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> extern void __set_fixmap (enum fixed_addresses idx,
> ^
The conflict is the type of 'phys'. In arch/um that's an unsigned long
rather than a phys_addr_t as it is elsewhere.
If I convert that to a phys_addr_t the build goes along happily.
Should we change set_fixmap_offset back to a macro function for now, or
is it simple/correct to change arch/um to use phys_addr_t in
__set_fixmap?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:33 ` Mark Rutland
@ 2016-02-09 16:33 ` Mark Rutland
2016-02-09 16:45 ` Arnd Bergmann
2016-02-09 16:52 ` Catalin Marinas
2 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 16:33 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Geert Uytterhoeven, Arnd Bergmann, Catalin Marinas,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
> >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> ^
> In file included from arch/um/include/asm/pgtable.h:11:0,
> from include/linux/mm.h:67,
> from include/linux/ring_buffer.h:5,
> from include/linux/trace_events.h:5,
> from include/trace/syscall.h:6,
> from include/linux/syscalls.h:81,
> from init/main.c:18:
> arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> extern void __set_fixmap (enum fixed_addresses idx,
> ^
The conflict is the type of 'phys'. In arch/um that's an unsigned long
rather than a phys_addr_t as it is elsewhere.
If I convert that to a phys_addr_t the build goes along happily.
Should we change set_fixmap_offset back to a macro function for now, or
is it simple/correct to change arch/um to use phys_addr_t in
__set_fixmap?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:33 ` Mark Rutland
2016-02-09 16:33 ` Mark Rutland
@ 2016-02-09 16:45 ` Arnd Bergmann
2016-02-09 16:52 ` Catalin Marinas
2 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:45 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven,
Catalin Marinas, Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tuesday 09 February 2016 16:33:34 Mark Rutland wrote:
> > >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> > void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > ^
> > In file included from arch/um/include/asm/pgtable.h:11:0,
> > from include/linux/mm.h:67,
> > from include/linux/ring_buffer.h:5,
> > from include/linux/trace_events.h:5,
> > from include/trace/syscall.h:6,
> > from include/linux/syscalls.h:81,
> > from init/main.c:18:
> > arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> > extern void __set_fixmap (enum fixed_addresses idx,
> > ^
>
> The conflict is the type of 'phys'. In arch/um that's an unsigned long
> rather than a phys_addr_t as it is elsewhere.
>
> If I convert that to a phys_addr_t the build goes along happily.
>
> Should we change set_fixmap_offset back to a macro function for now, or
> is it simple/correct to change arch/um to use phys_addr_t in
> __set_fixmap?
>
I'd vote for using phys_addr_t in arch/um.
Arnd
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:33 ` Mark Rutland
2016-02-09 16:33 ` Mark Rutland
2016-02-09 16:45 ` Arnd Bergmann
@ 2016-02-09 16:52 ` Catalin Marinas
2016-02-09 16:52 ` Catalin Marinas
2016-02-09 17:21 ` Mark Rutland
2 siblings, 2 replies; 25+ messages in thread
From: Catalin Marinas @ 2016-02-09 16:52 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 04:33:34PM +0000, Mark Rutland wrote:
> > >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> > void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > ^
> > In file included from arch/um/include/asm/pgtable.h:11:0,
> > from include/linux/mm.h:67,
> > from include/linux/ring_buffer.h:5,
> > from include/linux/trace_events.h:5,
> > from include/trace/syscall.h:6,
> > from include/linux/syscalls.h:81,
> > from init/main.c:18:
> > arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> > extern void __set_fixmap (enum fixed_addresses idx,
> > ^
>
> The conflict is the type of 'phys'. In arch/um that's an unsigned long
> rather than a phys_addr_t as it is elsewhere.
At a quick grep, we also have:
arch/sh/include/asm/fixmap.h
arch/sh/mm/init.c
arch/sh/mm/nommu.c
> If I convert that to a phys_addr_t the build goes along happily.
>
> Should we change set_fixmap_offset back to a macro function for now, or
> is it simple/correct to change arch/um to use phys_addr_t in
> __set_fixmap?
And sh. I prefer the static inline, though there is more effort needed
to test and get acks ;) (I really don't mind either way).
--
Catalin
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:52 ` Catalin Marinas
@ 2016-02-09 16:52 ` Catalin Marinas
2016-02-09 17:21 ` Mark Rutland
1 sibling, 0 replies; 25+ messages in thread
From: Catalin Marinas @ 2016-02-09 16:52 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 04:33:34PM +0000, Mark Rutland wrote:
> > >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> > void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > ^
> > In file included from arch/um/include/asm/pgtable.h:11:0,
> > from include/linux/mm.h:67,
> > from include/linux/ring_buffer.h:5,
> > from include/linux/trace_events.h:5,
> > from include/trace/syscall.h:6,
> > from include/linux/syscalls.h:81,
> > from init/main.c:18:
> > arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> > extern void __set_fixmap (enum fixed_addresses idx,
> > ^
>
> The conflict is the type of 'phys'. In arch/um that's an unsigned long
> rather than a phys_addr_t as it is elsewhere.
At a quick grep, we also have:
arch/sh/include/asm/fixmap.h
arch/sh/mm/init.c
arch/sh/mm/nommu.c
> If I convert that to a phys_addr_t the build goes along happily.
>
> Should we change set_fixmap_offset back to a macro function for now, or
> is it simple/correct to change arch/um to use phys_addr_t in
> __set_fixmap?
And sh. I prefer the static inline, though there is more effort needed
to test and get acks ;) (I really don't mind either way).
--
Catalin
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 16:52 ` Catalin Marinas
2016-02-09 16:52 ` Catalin Marinas
@ 2016-02-09 17:21 ` Mark Rutland
2016-02-09 17:21 ` Mark Rutland
2016-02-09 17:27 ` Catalin Marinas
1 sibling, 2 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 17:21 UTC (permalink / raw)
To: Catalin Marinas
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 04:52:34PM +0000, Catalin Marinas wrote:
> On Tue, Feb 09, 2016 at 04:33:34PM +0000, Mark Rutland wrote:
> > > >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> > > void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > > ^
> > > In file included from arch/um/include/asm/pgtable.h:11:0,
> > > from include/linux/mm.h:67,
> > > from include/linux/ring_buffer.h:5,
> > > from include/linux/trace_events.h:5,
> > > from include/trace/syscall.h:6,
> > > from include/linux/syscalls.h:81,
> > > from init/main.c:18:
> > > arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> > > extern void __set_fixmap (enum fixed_addresses idx,
> > > ^
> >
> > The conflict is the type of 'phys'. In arch/um that's an unsigned long
> > rather than a phys_addr_t as it is elsewhere.
>
> At a quick grep, we also have:
>
> arch/sh/include/asm/fixmap.h
> arch/sh/mm/init.c
> arch/sh/mm/nommu.c
>
> > If I convert that to a phys_addr_t the build goes along happily.
> >
> > Should we change set_fixmap_offset back to a macro function for now, or
> > is it simple/correct to change arch/um to use phys_addr_t in
> > __set_fixmap?
>
> And sh. I prefer the static inline, though there is more effort needed
> to test and get acks ;) (I really don't mind either way).
I would also prefer to make this a static inline, but it looks like we
need to sort out some cross-architecture cleanup first. I'm happy to
have a go at that.
In the meantime, so as to allow linux-next to build, and to save us from
merge hell, let's follow the usual idiom and hope that underscores will
protect us.
Hopefully this is the last time I ask this today: Arnd, Catalin, are you
happy with the below patch?
Mark.
---->8----
From 6d283603d18071690dc138e4a0591a445a1d1e30 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Tue, 9 Feb 2016 17:08:26 +0000
Subject: [PATCH] asm-generic: make __set_fixmap_offset a macro again
Turning __set_fixmap_offset into a static inline breaks the build for
several architectures. Fixing this properly requires updates to a number
of architectures to make them agree on the prototype of __set_fixmap.
For the timebeing, restore __set_fixmap_offset to its prior state as a
macro function, reverting commit ac4c0ac73485867c ("asm-generic: make
__set_fixmap_offset a static inline"). To avoid the original issue with
namespace clashes, 'addr' is prefixed with a liberal sprinking of
underscores.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
include/asm-generic/fixmap.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
index f9c27b6..827e4d3 100644
--- a/include/asm-generic/fixmap.h
+++ b/include/asm-generic/fixmap.h
@@ -70,13 +70,13 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
#endif
/* Return a pointer with offset calculated */
-static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
- phys_addr_t phys,
- pgprot_t flags)
-{
- __set_fixmap(idx, phys, flags);
- return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1));
-}
+#define __set_fixmap_offset(idx, phys, flags) \
+({ \
+ unsigned long ________addr; \
+ __set_fixmap(idx, phys, flags); \
+ ________addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \
+ ________addr; \
+})
#define set_fixmap_offset(idx, phys) \
__set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL)
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 17:21 ` Mark Rutland
@ 2016-02-09 17:21 ` Mark Rutland
2016-02-09 17:27 ` Catalin Marinas
1 sibling, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2016-02-09 17:21 UTC (permalink / raw)
To: Catalin Marinas
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 04:52:34PM +0000, Catalin Marinas wrote:
> On Tue, Feb 09, 2016 at 04:33:34PM +0000, Mark Rutland wrote:
> > > >> include/asm-generic/fixmap.h:72:6: error: conflicting types for '__set_fixmap'
> > > void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> > > ^
> > > In file included from arch/um/include/asm/pgtable.h:11:0,
> > > from include/linux/mm.h:67,
> > > from include/linux/ring_buffer.h:5,
> > > from include/linux/trace_events.h:5,
> > > from include/trace/syscall.h:6,
> > > from include/linux/syscalls.h:81,
> > > from init/main.c:18:
> > > arch/um/include/asm/fixmap.h:39:13: note: previous declaration of '__set_fixmap' was here
> > > extern void __set_fixmap (enum fixed_addresses idx,
> > > ^
> >
> > The conflict is the type of 'phys'. In arch/um that's an unsigned long
> > rather than a phys_addr_t as it is elsewhere.
>
> At a quick grep, we also have:
>
> arch/sh/include/asm/fixmap.h
> arch/sh/mm/init.c
> arch/sh/mm/nommu.c
>
> > If I convert that to a phys_addr_t the build goes along happily.
> >
> > Should we change set_fixmap_offset back to a macro function for now, or
> > is it simple/correct to change arch/um to use phys_addr_t in
> > __set_fixmap?
>
> And sh. I prefer the static inline, though there is more effort needed
> to test and get acks ;) (I really don't mind either way).
I would also prefer to make this a static inline, but it looks like we
need to sort out some cross-architecture cleanup first. I'm happy to
have a go at that.
In the meantime, so as to allow linux-next to build, and to save us from
merge hell, let's follow the usual idiom and hope that underscores will
protect us.
Hopefully this is the last time I ask this today: Arnd, Catalin, are you
happy with the below patch?
Mark.
---->8----
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 17:21 ` Mark Rutland
2016-02-09 17:21 ` Mark Rutland
@ 2016-02-09 17:27 ` Catalin Marinas
2016-02-09 17:27 ` Catalin Marinas
1 sibling, 1 reply; 25+ messages in thread
From: Catalin Marinas @ 2016-02-09 17:27 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 05:21:26PM +0000, Mark Rutland wrote:
> From 6d283603d18071690dc138e4a0591a445a1d1e30 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Tue, 9 Feb 2016 17:08:26 +0000
> Subject: [PATCH] asm-generic: make __set_fixmap_offset a macro again
>
> Turning __set_fixmap_offset into a static inline breaks the build for
> several architectures. Fixing this properly requires updates to a number
> of architectures to make them agree on the prototype of __set_fixmap.
>
> For the timebeing, restore __set_fixmap_offset to its prior state as a
> macro function, reverting commit ac4c0ac73485867c ("asm-generic: make
> __set_fixmap_offset a static inline"). To avoid the original issue with
> namespace clashes, 'addr' is prefixed with a liberal sprinking of
> underscores.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> include/asm-generic/fixmap.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> index f9c27b6..827e4d3 100644
> --- a/include/asm-generic/fixmap.h
> +++ b/include/asm-generic/fixmap.h
> @@ -70,13 +70,13 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> #endif
>
> /* Return a pointer with offset calculated */
> -static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> - phys_addr_t phys,
> - pgprot_t flags)
> -{
> - __set_fixmap(idx, phys, flags);
> - return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1));
> -}
> +#define __set_fixmap_offset(idx, phys, flags) \
> +({ \
> + unsigned long ________addr; \
> + __set_fixmap(idx, phys, flags); \
> + ________addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \
> + ________addr; \
> +})
>
> #define set_fixmap_offset(idx, phys) \
> __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL)
> --
> 1.9.1
It looks fine to me.
--
Catalin
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH] asm-generic: Fix build when __set_fixmap is absent
2016-02-09 17:27 ` Catalin Marinas
@ 2016-02-09 17:27 ` Catalin Marinas
0 siblings, 0 replies; 25+ messages in thread
From: Catalin Marinas @ 2016-02-09 17:27 UTC (permalink / raw)
To: Mark Rutland
Cc: kbuild test robot, kbuild-all, Geert Uytterhoeven, Arnd Bergmann,
Sudip Mukherjee, Stephen Rothwell, Linux-Next,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jeremy Linton,
Linux-Arch, Laura Abbott, Jeff Dike, Richard Weinberger
On Tue, Feb 09, 2016 at 05:21:26PM +0000, Mark Rutland wrote:
> From 6d283603d18071690dc138e4a0591a445a1d1e30 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Tue, 9 Feb 2016 17:08:26 +0000
> Subject: [PATCH] asm-generic: make __set_fixmap_offset a macro again
>
> Turning __set_fixmap_offset into a static inline breaks the build for
> several architectures. Fixing this properly requires updates to a number
> of architectures to make them agree on the prototype of __set_fixmap.
>
> For the timebeing, restore __set_fixmap_offset to its prior state as a
> macro function, reverting commit ac4c0ac73485867c ("asm-generic: make
> __set_fixmap_offset a static inline"). To avoid the original issue with
> namespace clashes, 'addr' is prefixed with a liberal sprinking of
> underscores.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
> include/asm-generic/fixmap.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> index f9c27b6..827e4d3 100644
> --- a/include/asm-generic/fixmap.h
> +++ b/include/asm-generic/fixmap.h
> @@ -70,13 +70,13 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr)
> #endif
>
> /* Return a pointer with offset calculated */
> -static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx,
> - phys_addr_t phys,
> - pgprot_t flags)
> -{
> - __set_fixmap(idx, phys, flags);
> - return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1));
> -}
> +#define __set_fixmap_offset(idx, phys, flags) \
> +({ \
> + unsigned long ________addr; \
> + __set_fixmap(idx, phys, flags); \
> + ________addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \
> + ________addr; \
> +})
>
> #define set_fixmap_offset(idx, phys) \
> __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL)
> --
> 1.9.1
It looks fine to me.
--
Catalin
^ permalink raw reply [flat|nested] 25+ messages in thread