From: Corey Minyard <minyard@acm.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@infradead.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
openipmi-developer@lists.sourceforge.net,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ipmi: avoid atomic_inc in exit function
Date: Tue, 16 Apr 2019 07:46:56 -0500 [thread overview]
Message-ID: <20190416124656.GI4121@minyard.net> (raw)
In-Reply-To: <CAK8P3a0MN0U1cZac2t-SE4=nzWfJ-WJCAz=zHAcAKofD_sM1_Q@mail.gmail.com>
On Mon, Apr 15, 2019 at 09:00:46PM +0200, Arnd Bergmann wrote:
> On Mon, Apr 15, 2019 at 7:39 PM Corey Minyard <minyard@acm.org> wrote:
> >
> > On Mon, Apr 15, 2019 at 09:40:22AM -0700, Christoph Hellwig wrote:
> > > On Mon, Apr 15, 2019 at 05:55:00PM +0200, Arnd Bergmann wrote:
> > > > This causes a link failure on ARM in certain configurations,
> > > > when we reference each atomic operation from .alt.smp.init in
> > > > order to patch out atomics on non-SMP systems:
> > > >
> > > > `.exit.text' referenced in section `.alt.smp.init' of drivers/char/ipmi/ipmi_msghandler.o: defined in discarded section `.exit.text' of drivers/char/ipmi/ipmi_msghandler.o
> > > >
> > > > In this case, we can trivially replace the atomic_inc() with
> > > > an atomic_set() that has the same effect and does not require
> > > > a fixup.
> > >
> > > I'd rather fіx the arm section management. Using atomic in exit
> > > routines is perfectly valid, and it would seem odd to forbid it.
> >
> > That was my first thought, too. It's kind of hard to believe that
> > the IPMI driver is the only thing that does an atomic_inc() in the
> > exit code.
>
> That's what I had thought as well at first, and I carried a patch
> to work around this by not dropping the .text.exit section on ARM
> when SMP patching is enabled for a few years. I never sent this
> because that can waste a significant amount of kernel memory,
> and I knew the warning is harmless.
>
> When revisiting it now, I found that this one was the only instance
> I ever hit. It seems to be that using atomics in module_exit() is
> indeed odd, because the function is rarely concurrent with anything
> else.
I've added the change to my tree; it actually makes a little more
sense, so I'm ok with it.
I guess it's up to you to deal with any new ones that happen in
the future ;-).
-corey
prev parent reply other threads:[~2019-04-16 12:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-15 15:55 [PATCH] ipmi: avoid atomic_inc in exit function Arnd Bergmann
2019-04-15 16:40 ` Christoph Hellwig
2019-04-15 17:39 ` Corey Minyard
2019-04-15 19:00 ` Arnd Bergmann
2019-04-16 12:46 ` Corey Minyard [this message]
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=20190416124656.GI4121@minyard.net \
--to=minyard@acm.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=openipmi-developer@lists.sourceforge.net \
/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.