| Message ID | 1455048108-5045-3-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At 20:01 +0000 on 09 Feb (1455048102), Andrew Cooper wrote: > Clang notices more unused functions than GCC. > > * sh_next_page() is only used at GUEST_PAGING_LEVELS=2, so remove it from the > other guest level translation units > * rcu_batch_after() is completely unused. > * Various of the COMPAT() generated functions are used only for their > BUILD_BUG_ON() properties. Annotate them all as __maybe_used. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Shadow stuff Acked-by: Tim Deegan <tim@xen.org>
>>> On 09.02.16 at 21:01, <andrew.cooper3@citrix.com> wrote: > --- a/xen/common/rcupdate.c > +++ b/xen/common/rcupdate.c > @@ -141,12 +141,6 @@ static inline int rcu_batch_before(long a, long b) > return (a - b) < 0; > } > > -/* Is batch a after batch b ? */ > -static inline int rcu_batch_after(long a, long b) > -{ > - return (a - b) > 0; > -} To me it is the nature of inline functions that they may or may not be used, regardless of whether they live in a header file (where I would have supposed Clang won't warn about, but the change below makes me assume I'm wrong) or in a source file. > --- a/xen/include/xen/compat.h > +++ b/xen/include/xen/compat.h > @@ -134,14 +134,16 @@ > #define CHECK_NAME_(k, n, tag) __check ## tag ## k ## _ ## n > > #define CHECK_TYPE(name) \ > -static inline int CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ > - compat_ ## name ## _t *c) \ > +static inline int __maybe_unused \ > +CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ > + compat_ ## name ## _t *c) \ > { \ > return x == c; \ > } > #define CHECK_TYPE_(k, n) \ > -static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ > - k compat_ ## n *c) \ > +static inline int __maybe_unused \ > +CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ > + k compat_ ## n *c) \ > { \ > return x == c; \ > } > @@ -154,14 +156,14 @@ static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ > sizeof(k compat_ ## n)) * 2] > > #define CHECK_FIELD_COMMON(name, t, f) \ > -static inline int name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ > +static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ > { \ > BUILD_BUG_ON(offsetof(xen_ ## t ## _t, f) != \ > offsetof(compat_ ## t ## _t, f)); \ > return &x->f == &c->f; \ > } > #define CHECK_FIELD_COMMON_(k, name, n, f) \ > -static inline int name(k xen_ ## n *x, k compat_ ## n *c) \ > +static inline int __maybe_unused name(k xen_ ## n *x, k compat_ ## n *c) \ > { \ > BUILD_BUG_ON(offsetof(k xen_ ## n, f) != \ > offsetof(k compat_ ## n, f)); \ So if these are all noticed to be unused, why would others in other header files not be? I think there's at the very least some aspect missing in the description, explaining what makes these stand out from the others. Jan
On 10/02/16 13:06, Jan Beulich wrote: >>>> On 09.02.16 at 21:01, <andrew.cooper3@citrix.com> wrote: >> --- a/xen/common/rcupdate.c >> +++ b/xen/common/rcupdate.c >> @@ -141,12 +141,6 @@ static inline int rcu_batch_before(long a, long b) >> return (a - b) < 0; >> } >> >> -/* Is batch a after batch b ? */ >> -static inline int rcu_batch_after(long a, long b) >> -{ >> - return (a - b) > 0; >> -} > To me it is the nature of inline functions that they may or may not be > used, regardless of whether they live in a header file (where I would > have supposed Clang won't warn about, but the change below makes > me assume I'm wrong) or in a source file. > >> --- a/xen/include/xen/compat.h >> +++ b/xen/include/xen/compat.h >> @@ -134,14 +134,16 @@ >> #define CHECK_NAME_(k, n, tag) __check ## tag ## k ## _ ## n >> >> #define CHECK_TYPE(name) \ >> -static inline int CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ >> - compat_ ## name ## _t *c) \ >> +static inline int __maybe_unused \ >> +CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ >> + compat_ ## name ## _t *c) \ >> { \ >> return x == c; \ >> } >> #define CHECK_TYPE_(k, n) \ >> -static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ >> - k compat_ ## n *c) \ >> +static inline int __maybe_unused \ >> +CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ >> + k compat_ ## n *c) \ >> { \ >> return x == c; \ >> } >> @@ -154,14 +156,14 @@ static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ >> sizeof(k compat_ ## n)) * 2] >> >> #define CHECK_FIELD_COMMON(name, t, f) \ >> -static inline int name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ >> +static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ >> { \ >> BUILD_BUG_ON(offsetof(xen_ ## t ## _t, f) != \ >> offsetof(compat_ ## t ## _t, f)); \ >> return &x->f == &c->f; \ >> } >> #define CHECK_FIELD_COMMON_(k, name, n, f) \ >> -static inline int name(k xen_ ## n *x, k compat_ ## n *c) \ >> +static inline int __maybe_unused name(k xen_ ## n *x, k compat_ ## n *c) \ >> { \ >> BUILD_BUG_ON(offsetof(k xen_ ## n, f) != \ >> offsetof(k compat_ ## n, f)); \ > So if these are all noticed to be unused, why would others in other > header files not be? Because they are instantiated in translation units, with no callers, by code like common/trace.c: #ifdef CONFIG_COMPAT #include <compat/trace.h> #define xen_t_buf t_buf CHECK_t_buf; #undef xen_t_buf #else #define compat_t_rec t_rec #endif This was the first example which blew up. ~Andrew
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index 4d0b317..aaf8db7 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -436,6 +436,7 @@ sh_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p, intpte_t *old, * space.) */ +#if GUEST_PAGING_LEVELS == 2 /* From one page of a multi-page shadow, find the next one */ static inline mfn_t sh_next_page(mfn_t smfn) { @@ -454,6 +455,7 @@ static inline mfn_t sh_next_page(mfn_t smfn) ASSERT(!next->u.sh.head); return page_to_mfn(next); } +#endif static inline u32 guest_index(void *ptr) diff --git a/xen/common/rcupdate.c b/xen/common/rcupdate.c index f13b87b..8cc5a82 100644 --- a/xen/common/rcupdate.c +++ b/xen/common/rcupdate.c @@ -141,12 +141,6 @@ static inline int rcu_batch_before(long a, long b) return (a - b) < 0; } -/* Is batch a after batch b ? */ -static inline int rcu_batch_after(long a, long b) -{ - return (a - b) > 0; -} - static void force_quiescent_state(struct rcu_data *rdp, struct rcu_ctrlblk *rcp) { diff --git a/xen/include/xen/compat.h b/xen/include/xen/compat.h index e5c23e2..3f4cef6 100644 --- a/xen/include/xen/compat.h +++ b/xen/include/xen/compat.h @@ -134,14 +134,16 @@ #define CHECK_NAME_(k, n, tag) __check ## tag ## k ## _ ## n #define CHECK_TYPE(name) \ -static inline int CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ - compat_ ## name ## _t *c) \ +static inline int __maybe_unused \ +CHECK_NAME(name, T)(xen_ ## name ## _t *x, \ + compat_ ## name ## _t *c) \ { \ return x == c; \ } #define CHECK_TYPE_(k, n) \ -static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ - k compat_ ## n *c) \ +static inline int __maybe_unused \ +CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ + k compat_ ## n *c) \ { \ return x == c; \ } @@ -154,14 +156,14 @@ static inline int CHECK_NAME_(k, n, T)(k xen_ ## n *x, \ sizeof(k compat_ ## n)) * 2] #define CHECK_FIELD_COMMON(name, t, f) \ -static inline int name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ +static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## t ## _t *c) \ { \ BUILD_BUG_ON(offsetof(xen_ ## t ## _t, f) != \ offsetof(compat_ ## t ## _t, f)); \ return &x->f == &c->f; \ } #define CHECK_FIELD_COMMON_(k, name, n, f) \ -static inline int name(k xen_ ## n *x, k compat_ ## n *c) \ +static inline int __maybe_unused name(k xen_ ## n *x, k compat_ ## n *c) \ { \ BUILD_BUG_ON(offsetof(k xen_ ## n, f) != \ offsetof(k compat_ ## n, f)); \
Clang notices more unused functions than GCC. * sh_next_page() is only used at GUEST_PAGING_LEVELS=2, so remove it from the other guest level translation units * rcu_batch_after() is completely unused. * Various of the COMPAT() generated functions are used only for their BUILD_BUG_ON() properties. Annotate them all as __maybe_used. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Tim Deegan <tim@xen.org> CC: Ian Campbell <Ian.Campbell@citrix.com> --- xen/arch/x86/mm/shadow/multi.c | 2 ++ xen/common/rcupdate.c | 6 ------ xen/include/xen/compat.h | 14 ++++++++------ 3 files changed, 10 insertions(+), 12 deletions(-)