netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: David Miller <davem@davemloft.net>
Cc: JBeulich@suse.com, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org, xemul@openvz.org,
	ebiederm@xmission.com
Subject: Re: [PATCH] adjust __net_exit
Date: Sun, 18 Mar 2012 00:03:18 +0100	[thread overview]
Message-ID: <20120317230318.GA19460@merkur.ravnborg.org> (raw)
In-Reply-To: <20120316.221813.236130733754848615.davem@davemloft.net>

On Fri, Mar 16, 2012 at 10:18:13PM -0700, David Miller wrote:
> From: "Jan Beulich" <JBeulich@suse.com>
> Date: Thu, 08 Mar 2012 09:37:32 +0000
> 
> > __net_exit, judging by the majority of its uses, was intended to serve
> > as an abstraction to allow calling such annotated functions from both
> > __init and __exit functions. Using the (bogus and unused elsewhere)
> > __exit_refok to implement this is inefficient - any non-modular code
> > really can reside in __init (as non-modular __exit code is never used).
> > 
> > Therefore, adjust __net_exit to resolve to nothing (i.e. normal .text)
> > in modules, and __init in the core kernel.
> > 
> > A few other adjustments are necessary/possible with this done - those
> > were likely just oversights when added originally.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> [ I have been waiting for more than a week for a netns developer
>   to review this patch, I guess I'm too optimistic these days. :-( ]
> 
> The only reason you think __exit_refok is "bogus" is because it's
> semantics got changed by Sam Ravnborg in commit
> 312b1485fb509c9bc32eda28ad29537896658cb8 ("Introduce new section
> reference annotations tags: __ref, __refdata, __refconst")
> 
> Beforehand the __exit_refok was a real .exit section, so it got
> completely discarded AT LINK TIME.  Now it sits together with
> __init_refok which is an unremovable kernel image section, which
> neither gets removed at compile time nor boot time.

Some misunderstanding is going on here.

The *ref* annotation is used to teach modpost that this function (or data)
may reference functions (or data) which is annotated __init*.
And the *ref* annotation never caused the annotated code to be discarded.

This was true before the above mentioned patch - and it is still true.

Before the path ("Introduce new section reference ....") the __exit_refok
annotation moved functions to the section named ".exit.text.refok"
which was explicit part of .text (TEXT_TEXT in vmlinux).

So __exit_refok does exactly what it is intended to do:
It puts the function in a section so modpost does not warn about
references to __init or __exit sections.

As Jan points out there is only a single user left - so
this would be a good time to kill it. It is even documented
in init.h that this is a backward compatibility define.

The intention with Jan's patch is to move functions annotated
__net_exit to a discardable section in the core kernel.
Then at least __exit should be used - there is no logic
using __init for exit code.

I suggest to:
1) fix the patch to use __exit
2) fix up the bogus commit message

Then it should be OK - iff the assumption hold that the functions
can be discarded in the core kernel.
I have grepped a little and saw no uses where this did not hold true.
So based on this I assume the assumption is OK.

	Sam

  reply	other threads:[~2012-03-17 23:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-08  9:37 [PATCH] adjust __net_exit Jan Beulich
2012-03-17  5:18 ` David Miller
2012-03-17 23:03   ` Sam Ravnborg [this message]
2012-03-19  0:41     ` Eric W. Biederman
2012-03-19  8:14     ` Jan Beulich
2012-03-19  8:47       ` Sam Ravnborg
2012-03-19  9:09         ` Jan Beulich
2012-03-20  2:17           ` Eric W. Biederman

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=20120317230318.GA19460@merkur.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=JBeulich@suse.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=xemul@openvz.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).