All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Randy Dunlap <randy.dunlap@oracle.com>
Cc: lkml <linux-kernel@vger.kernel.org>
Subject: Re: Help needed to fix section mismatch warnings
Date: Fri, 11 Jan 2008 19:59:58 +0100	[thread overview]
Message-ID: <20080111185958.GI28624@uranus.ravnborg.org> (raw)
In-Reply-To: <20080110111918.42e4fa0b.randy.dunlap@oracle.com>

On Thu, Jan 10, 2008 at 11:19:18AM -0800, Randy Dunlap wrote:
> On Wed, 9 Jan 2008 22:25:42 -0800 Randy Dunlap wrote:
> 
> > On Sun, 6 Jan 2008 15:07:28 +0100 Sam Ravnborg wrote:
> > 
> > 
> > > This is the current list of warnings
> > 
> > Sam,
> > 
> > Several of these are due to driver variable names not matching
> > the whitelisted names in modpost.  I have patches for the ones
> > that I have identified so far.  And I have patches for a few of
> > the others that are true section mismatch problems (total of 8
> > patches ready for now).
> > 
> > The whitelisted names will always be a (small) problem.
> > Can __init_refok be used in these cases.. or some other new
> > attribute, instead of forever adding to the whitelist or
> > modifying variable names?
> 
> Sam (or anyone :),
> 
> I guess that I'm a little confused.  Instead of changing
> variable names to match the modpost whitelist, I tested adding
> __init_refok or __initdata_refok to these (driver) structs that
> generated the modpost warnings.
> 
> Example:  drivers/char/tpm/tpm_infineon.c
> 
> --- linux-2.6.24-rc7-git1.orig/drivers/char/tpm/tpm_infineon.c
> +++ linux-2.6.24-rc7-git1/drivers/char/tpm/tpm_infineon.c
> @@ -611,7 +611,7 @@ static __devexit void tpm_inf_pnp_remove
>  	}
>  }
>  
> -static struct pnp_driver tpm_inf_pnp = {
> +static struct pnp_driver __init_refok tpm_inf_pnp = {
>  	.name = "tpm_inf_pnp",
>  	.driver = {
>  		.owner = THIS_MODULE,
> 
> This has a build warning with my toolchain:
> 
>   CC      drivers/char/tpm/tpm_infineon.o
> linux-2.6.24-rc7-git1/drivers/char/tpm/tpm_infineon.c:614: warning: 'noinline' attribute ignored
> {standard input}: Assembler messages:
> {standard input}:2315: Warning: setting incorrect section attributes for .text.init.refok
> 
> but otherwise no section mismatch warning.
> 
> OTOH, using __initdata_refok has no build warning and no section
> mismatch warning... but it (__initdata_refok) doesn't make sense
> to me.  Should it (make sense/be used)?

What we try to do is to tell modpost that from this
particular spot in the code it is not a bug when a
__init/__exit function is called.
The whitelisting uses the name of the variable to determine
is this is a bug and not.
But traditionally we have in the kernel used annotation for this.
The __initdata_refok  wer invented as a tag that could be used
to tell modpost that this variable may call an __init function.

What happens is that the variable is placed in a section named:
.data.init.refok and when modpost encounter a variable in this
section then it does not warn.

Likewise __init_refok teach gcc to locate the function in a
section named .text.init.refok and modpost uses this to 
know that __init referneces are not a bug when they happens
in this section. In addition __init_refok contains noinline
to avoid agressive gcc inlining which gcc does across different
sections.

> Is there a __refok that should be used here, instead of having
> to modify variable names?

So far I have favoured consistent variable namings but
I could be persuaded to use annotation as the preferred method.
If for no other reason then because the annotation is a nice
way to document that in this spot the reference is OK.
But better named annothation is then needed...

	Sam

  reply	other threads:[~2008-01-11 19:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-06 14:07 Help needed to fix section mismatch warnings Sam Ravnborg
2008-01-10  6:25 ` Randy Dunlap
2008-01-10 19:19   ` Randy Dunlap
2008-01-11 18:59     ` Sam Ravnborg [this message]
2008-01-10 19:38   ` Sam Ravnborg
2008-01-19 19:52 ` Randy Dunlap
2008-01-19 21:22   ` Sam Ravnborg
2008-01-19 21:45     ` Sam Ravnborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080111185958.GI28624@uranus.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=randy.dunlap@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.