2006-12-01 11:48:10

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] parisc: "extern inline" -> "static inline" (fwd)

"extern inline" generates a warning with -Wmissing-prototypes and I'm
currently working on getting the kernel cleaned up for adding this to
the CFLAGS since it will help us to avoid a nasty class of runtime
errors.

If there are places that really need a forced inline, __always_inline
would be the correct solution.

Signed-off-by: Adrian Bunk <[email protected]>
Acked-by: Kyle McMartin <[email protected]>

---

This patch was already sent on:
- 21 Nov 2006
- 19 Aug 2006

arch/parisc/lib/memcpy.c | 4 +--
include/asm-parisc/io.h | 2 -
include/asm-parisc/pci.h | 2 -
include/asm-parisc/pgtable.h | 40 ++++++++++++++++-----------------
include/asm-parisc/prefetch.h | 4 +--
include/asm-parisc/semaphore.h | 10 ++++----
include/asm-parisc/tlbflush.h | 3 --
7 files changed, 32 insertions(+), 33 deletions(-)

--- linux-2.6.14-rc5-mm1-full/arch/parisc/lib/memcpy.c.old 2005-10-30 01:58:43.000000000 +0200
+++ linux-2.6.14-rc5-mm1-full/arch/parisc/lib/memcpy.c 2005-10-30 01:59:11.000000000 +0200
@@ -158,12 +158,12 @@
#define stw(_s,_t,_o,_a,_e) def_store_insn(stw,"r",_s,_t,_o,_a,_e)

#ifdef CONFIG_PREFETCH
-extern inline void prefetch_src(const void *addr)
+static inline void prefetch_src(const void *addr)
{
__asm__("ldw 0(" s_space ",%0), %%r0" : : "r" (addr));
}

-extern inline void prefetch_dst(const void *addr)
+static inline void prefetch_dst(const void *addr)
{
__asm__("ldd 0(" d_space ",%0), %%r0" : : "r" (addr));
}
--- linux-2.6.14-rc5-mm1-full/include/asm-parisc/pci.h.old 2005-10-30 01:59:57.000000000 +0200
+++ linux-2.6.14-rc5-mm1-full/include/asm-parisc/pci.h 2005-10-30 02:00:01.000000000 +0200
@@ -193,7 +193,7 @@
extern void pcibios_register_hba(struct pci_hba_data *);
extern void pcibios_set_master(struct pci_dev *);
#else
-extern inline void pcibios_register_hba(struct pci_hba_data *x)
+static inline void pcibios_register_hba(struct pci_hba_data *x)
{
}
#endif
--- linux-2.6.14-rc5-mm1-full/include/asm-parisc/pgtable.h.old 2005-10-30 02:00:14.000000000 +0200
+++ linux-2.6.14-rc5-mm1-full/include/asm-parisc/pgtable.h 2005-10-30 02:00:18.000000000 +0200
@@ -316,31 +316,31 @@
* setup: the pgd is never bad, and a pmd always exists (as it's folded
* into the pgd entry)
*/
-extern inline int pgd_none(pgd_t pgd) { return 0; }
-extern inline int pgd_bad(pgd_t pgd) { return 0; }
-extern inline int pgd_present(pgd_t pgd) { return 1; }
-extern inline void pgd_clear(pgd_t * pgdp) { }
+static inline int pgd_none(pgd_t pgd) { return 0; }
+static inline int pgd_bad(pgd_t pgd) { return 0; }
+static inline int pgd_present(pgd_t pgd) { return 1; }
+static inline void pgd_clear(pgd_t * pgdp) { }
#endif

/*
* The following only work if pte_present() is true.
* Undefined behaviour if not..
*/
-extern inline int pte_read(pte_t pte) { return pte_val(pte) & _PAGE_READ; }
-extern inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; }
-extern inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
-extern inline int pte_write(pte_t pte) { return pte_val(pte) & _PAGE_WRITE; }
-extern inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE; }
-extern inline int pte_user(pte_t pte) { return pte_val(pte) & _PAGE_USER; }
-
-extern inline pte_t pte_rdprotect(pte_t pte) { pte_val(pte) &= ~_PAGE_READ; return pte; }
-extern inline pte_t pte_mkclean(pte_t pte) { pte_val(pte) &= ~_PAGE_DIRTY; return pte; }
-extern inline pte_t pte_mkold(pte_t pte) { pte_val(pte) &= ~_PAGE_ACCESSED; return pte; }
-extern inline pte_t pte_wrprotect(pte_t pte) { pte_val(pte) &= ~_PAGE_WRITE; return pte; }
-extern inline pte_t pte_mkread(pte_t pte) { pte_val(pte) |= _PAGE_READ; return pte; }
-extern inline pte_t pte_mkdirty(pte_t pte) { pte_val(pte) |= _PAGE_DIRTY; return pte; }
-extern inline pte_t pte_mkyoung(pte_t pte) { pte_val(pte) |= _PAGE_ACCESSED; return pte; }
-extern inline pte_t pte_mkwrite(pte_t pte) { pte_val(pte) |= _PAGE_WRITE; return pte; }
+static inline int pte_read(pte_t pte) { return pte_val(pte) & _PAGE_READ; }
+static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; }
+static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
+static inline int pte_write(pte_t pte) { return pte_val(pte) & _PAGE_WRITE; }
+static inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_user(pte_t pte) { return pte_val(pte) & _PAGE_USER; }
+
+static inline pte_t pte_rdprotect(pte_t pte) { pte_val(pte) &= ~_PAGE_READ; return pte; }
+static inline pte_t pte_mkclean(pte_t pte) { pte_val(pte) &= ~_PAGE_DIRTY; return pte; }
+static inline pte_t pte_mkold(pte_t pte) { pte_val(pte) &= ~_PAGE_ACCESSED; return pte; }
+static inline pte_t pte_wrprotect(pte_t pte) { pte_val(pte) &= ~_PAGE_WRITE; return pte; }
+static inline pte_t pte_mkread(pte_t pte) { pte_val(pte) |= _PAGE_READ; return pte; }
+static inline pte_t pte_mkdirty(pte_t pte) { pte_val(pte) |= _PAGE_DIRTY; return pte; }
+static inline pte_t pte_mkyoung(pte_t pte) { pte_val(pte) |= _PAGE_ACCESSED; return pte; }
+static inline pte_t pte_mkwrite(pte_t pte) { pte_val(pte) |= _PAGE_WRITE; return pte; }

/*
* Conversion functions: convert a page and protection to a page entry,
@@ -368,7 +368,7 @@
#define mk_pte_phys(physpage, pgprot) \
({ pte_t __pte; pte_val(__pte) = physpage + pgprot_val(pgprot); __pte; })

-extern inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
+static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{ pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot); return pte; }

/* Permanent address of a page. On parisc we don't have highmem. */
--- linux-2.6.14-rc5-mm1-full/include/asm-parisc/semaphore.h.old 2005-10-30 02:00:45.000000000 +0200
+++ linux-2.6.14-rc5-mm1-full/include/asm-parisc/semaphore.h 2005-10-30 02:00:51.000000000 +0200
@@ -58,7 +58,7 @@
#define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)

-extern inline void sema_init (struct semaphore *sem, int val)
+static inline void sema_init (struct semaphore *sem, int val)
{
*sem = (struct semaphore)__SEMAPHORE_INITIALIZER((*sem),val);
}
@@ -86,7 +86,7 @@
* interrupts while we're messing with the semaphore. Sorry.
*/

-extern __inline__ void down(struct semaphore * sem)
+static inline void down(struct semaphore * sem)
{
might_sleep();
spin_lock_irq(&sem->sentry);
@@ -98,7 +98,7 @@
spin_unlock_irq(&sem->sentry);
}

-extern __inline__ int down_interruptible(struct semaphore * sem)
+static inline int down_interruptible(struct semaphore * sem)
{
int ret = 0;
might_sleep();
@@ -116,7 +116,7 @@
* down_trylock returns 0 on success, 1 if we failed to get the lock.
* May not sleep, but must preserve irq state
*/
-extern __inline__ int down_trylock(struct semaphore * sem)
+static inline int down_trylock(struct semaphore * sem)
{
int flags, count;

@@ -132,7 +132,7 @@
* Note! This is subtle. We jump to wake people up only if
* the semaphore was negative (== somebody was waiting on it).
*/
-extern __inline__ void up(struct semaphore * sem)
+static inline void up(struct semaphore * sem)
{
int flags;
spin_lock_irqsave(&sem->sentry, flags);
--- linux-2.6.14-rc5-mm1-full/include/asm-parisc/tlbflush.h.old 2005-10-30 02:01:00.000000000 +0200
+++ linux-2.6.14-rc5-mm1-full/include/asm-parisc/tlbflush.h 2005-10-30 02:01:06.000000000 +0200
@@ -42,7 +42,7 @@
#endif
}

-extern __inline__ void flush_tlb_pgtables(struct mm_struct *mm, unsigned long start, unsigned long end)
+static inline void flush_tlb_pgtables(struct mm_struct *mm, unsigned long start, unsigned long end)
{
}



2006-12-01 16:43:57

by Grant Grundler

[permalink] [raw]
Subject: Re: [2.6 patch] parisc: "extern inline" -> "static inline" (fwd)

On Fri, Dec 01, 2006 at 12:48:11PM +0100, Adrian Bunk wrote:
> "extern inline" generates a warning with -Wmissing-prototypes and I'm
> currently working on getting the kernel cleaned up for adding this to
> the CFLAGS since it will help us to avoid a nasty class of runtime
> errors.


John David Anglin is the hppa/parisc gcc maintainer and has
commented on inline variants last year:
http://lists.parisc-linux.org/pipermail/parisc-linux/2005-October/027587.html

This makes me think -Wmissing-prototypes is reporting the wrong warning.
ie there is a prototype but no function and no label.
Can you check with gcc folks to see if this is a gcc bug?

The parisc point intentionally switched to "extern inline" at one
point and unless what jda wrote is now incorrect, I'm not inclined
to change it.

> If there are places that really need a forced inline, __always_inline
> would be the correct solution.

Yes, all the functions marked "extern inline" are expected to get
essentially the same treatment as "always_inline".

thanks,
grant

2006-12-01 16:54:24

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] parisc: "extern inline" -> "static inline" (fwd)

On Fri, Dec 01, 2006 at 09:43:54AM -0700, Grant Grundler wrote:
> On Fri, Dec 01, 2006 at 12:48:11PM +0100, Adrian Bunk wrote:
> > "extern inline" generates a warning with -Wmissing-prototypes and I'm
> > currently working on getting the kernel cleaned up for adding this to
> > the CFLAGS since it will help us to avoid a nasty class of runtime
> > errors.
>
>
> John David Anglin is the hppa/parisc gcc maintainer and has
> commented on inline variants last year:
> http://lists.parisc-linux.org/pipermail/parisc-linux/2005-October/027587.html
>
> This makes me think -Wmissing-prototypes is reporting the wrong warning.
> ie there is a prototype but no function and no label.
> Can you check with gcc folks to see if this is a gcc bug?
>
> The parisc point intentionally switched to "extern inline" at one
> point and unless what jda wrote is now incorrect, I'm not inclined
> to change it.

If you read John David Anglin's email, you'll note that if you take the
address you need this function provided somewhere.

Which of the functions my patch changes also have a global function
provided within the kernel?

If none, "extern inline" didn't make any sense.

> > If there are places that really need a forced inline, __always_inline
> > would be the correct solution.
>
> Yes, all the functions marked "extern inline" are expected to get
> essentially the same treatment as "always_inline".

Currently, "inline" is defined to be always_inline, and
__always_inline is for cases that produce non-compiling or non-working
(opposed to only suboptimal) code.

> thanks,
> grant

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2006-12-01 17:36:50

by Grant Grundler

[permalink] [raw]
Subject: Re: [2.6 patch] parisc: "extern inline" -> "static inline" (fwd)

On Fri, Dec 01, 2006 at 05:54:27PM +0100, Adrian Bunk wrote:
> If you read John David Anglin's email, you'll note that if you take the
> address you need this function provided somewhere.

Let me turn that around.
Which of the "extern inline" functions are we taking the address?
The parisc kernel wouldn't (shouldn't) link if that were true.

> Which of the functions my patch changes also have a global function
> provided within the kernel?
>
> If none, "extern inline" didn't make any sense.

I expect none.

...
> Currently, "inline" is defined to be always_inline, and
> __always_inline is for cases that produce non-compiling or non-working
> (opposed to only suboptimal) code.

Ok. Sounds like "extern inline" is the same as __always_inline.

Has gcc community confirmed "gcc -Wmissing-prototypes" behavior
is really correct with respect to "extern inline"?

If so, I'm ok with changing "extern inline" to __always_inline.

thanks,
grant

2006-12-01 17:39:49

by John David Anglin

[permalink] [raw]
Subject: Re: [parisc-linux] Re: [2.6 patch] parisc: "extern inline" -> "static

> The parisc point intentionally switched to "extern inline" at one
> point and unless what jda wrote is now incorrect, I'm not inclined
> to change it.

The handling of "extern inline" is changing in GCC 4.3 to the C99 specified
behavior. The attribute gnu_inline on an inline declaration results in the
old GNU C89 inline behavior even in the ISO C99 mode.

The C99 behavior allows the function being inlined to be externally
called. As a result, conflicts will occur if the function is defined in
a header included by multiple objects. So, code that assumed the previous
GNU treatment unfortunately needs to change.

The main difference between "static inline" and "extern inline" in the
old GNU treatment was that with "extern inline" the function was never
compiled on its own, even if its address was explicitly referenced.
With "static inline", the function would be compiled on its own in some
circumstances. So, this is mostly a code size issue.

More details are present in extend.texi in the GCC head. This has also
been discussed on the gcc list a few times.

Dave
--
J. David Anglin [email protected]
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)