From: Greg KH <gregkh@linuxfoundation.org>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Igor Mammedov <imammedo@redhat.com>,
chuck.anderson@oracle.com, linux-kernel@vger.kernel.org
Subject: Re: udev races with 'arch_register_cpu' to write 1 to /sys/devices/system/cpu/cpu1/online (which is not yet created)
Date: Mon, 13 May 2013 07:25:32 -0700 [thread overview]
Message-ID: <20130513142532.GA15808@kroah.com> (raw)
In-Reply-To: <20130513133128.GJ6811@phenom.dumpdata.com>
On Mon, May 13, 2013 at 09:31:28AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, May 10, 2013 at 06:34:34PM +0200, Igor Mammedov wrote:
> > >On Mon, Apr 30, 2012 at 11:51:49AM -0400, Greg KH wrote:
> > >> On Mon, Apr 30, 2012 at 11:50:18AM -0400, Greg KH wrote:
> > >> > On Mon, Apr 30, 2012 at 11:36:23AM -0400, Konrad Rzeszutek Wilk wrote:
> > >> > > Hey Greg,
> > >> > >
> > >> > > Hoping you can help with some guidance on how to fix this.
> > >> > >
> > >> > > The issue is with CPU hotplug is that when a CPU goes up
> > >> > > it calls 'arch_register_cpu' which eventually calls
> > >> > > register_cpu. That function does these two things:
> > >> > >
> > >> > > 251 error = device_register(&cpu->dev);
> > >> > > 252 if (!error && cpu->hotpluggable)
> > >> > > 253 register_cpu_control(cpu);
> > >> > >
> > >> > > and the device_register creates a nice little SysFS directory:
> > >> > >
> > >> > > /sys/devices/system/cpu/cpu2/ which at line 251 has the 'add' attribute
> > >> > > but no 'online' attribute. udev then tries to echo 1 to the 'online'
> > >> > > and it we get:
> > >> > > udevd-work[2421]: error opening ATTR{/sys/devices/system/cpu/cpu2/online} for writing: No such file or directory
> > >> > >
> > >> > > Line 253 creates said 'online' and at that time udev [or the system admin]
> > >> > > can write 1 to 'online' and the CPU goes up.
> > >> > >
> > >> > > So .. any thoughts? Is there some way to inhibit from uevent being sent
> > >> > > until line 253 has run?
> > >> >
> > >> > Yes.
> > >>
> > >> Oh, I imagine you want to know _how_ to do it too, right? (sorry, I
> > >> couldn't resist...)
> > >
> > >Heh.
> > >>
> > >> Make this a default attribute of the cpu device, and then it will be
> > >> created by the driver core before the uevent is sent to userspace.
> > >> That's what you are supposed to do in the first place, adding files "by
> > >> hand" is wrong, for this very reason.
> > >
> > >OK, will prep up a patch shortly.
> >
> > Hello Konrad,
> >
> > Is there a posted/accepted patch or idea was dropped?
>
> Hey Igor,
>
> CC-ing here Chuck here who is looking at that. My recollection (and Chuck please
> correct me if I am wrong) is that the idea Greg outlined won't work very well.
> The reason is that making 'online' an attribute of 'struct dev' means that:
> 1) A lot of SysFS attributes that have no notion of online/offline (say ISA bus)
> will now have.
Then only create that attribute for busses that ask for it to be enabled.
> 2) The default action item (so function) to do something based on
> writting/reading from 'online' will have to be overridden by the
> driver using it. Which means another race - we can create an SysFS
> attribute but the default points to something that does nothing.
I don't understand this at all, how will there be a race exactly?
greg k-h
next prev parent reply other threads:[~2013-05-13 14:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20120430153623.GA23485@phenom.dumpdata.com>
2012-04-30 15:37 ` udev races with 'arch_register_cpu' to write 1 to /sys/devices/system/cpu/cpu1/online (which is not yet created) Konrad Rzeszutek Wilk
2012-04-30 15:50 ` Greg KH
2012-04-30 15:51 ` Greg KH
2012-04-30 16:17 ` Konrad Rzeszutek Wilk
2013-05-10 16:34 ` Igor Mammedov
2013-05-13 13:31 ` Konrad Rzeszutek Wilk
2013-05-13 14:25 ` Greg KH [this message]
2013-05-13 22:05 ` [RFC 0/2] cpu: fix leak and udev race in register_cpu() Igor Mammedov
2013-05-14 13:19 ` Konrad Rzeszutek Wilk
2013-05-14 14:45 ` Greg KH
2013-05-13 22:05 ` [PATCH 1/2] cpu: fix "crash_notes" leak " Igor Mammedov
2013-05-14 13:16 ` Konrad Rzeszutek Wilk
2013-05-13 22:05 ` [PATCH 2/2] cpu: make sure that cpu/online file created before KOBJ_ADD is emitted Igor Mammedov
2013-05-14 13:17 ` Konrad Rzeszutek Wilk
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=20130513142532.GA15808@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=chuck.anderson@oracle.com \
--cc=imammedo@redhat.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.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 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.