From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Toshi Kani <toshi.kani@hp.com>,
linux-acpi@vger.kernel.org,
Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>,
Wen Congyang <wency@cn.fujitsu.com>,
Wen Congyang <wencongyang@gmail.com>,
isimatu.yasuaki@jp.fujitsu.com, lenb@kernel.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [RFC PATCH v3 3/3] acpi_memhotplug: Allow eject to proceed on rebind scenario
Date: Thu, 29 Nov 2012 00:31:02 +0100 [thread overview]
Message-ID: <2363618.GHMYUtrpGK@vostro.rjw.lan> (raw)
In-Reply-To: <20121128231046.GA15416@kroah.com>
On Wednesday, November 28, 2012 03:10:46 PM Greg KH wrote:
> On Thu, Nov 29, 2012 at 12:05:20AM +0100, Rafael J. Wysocki wrote:
> > On Wednesday, November 28, 2012 02:46:33 PM Greg KH wrote:
> > > > So, it looks like the driver core wants us to handle driver unbinding no
> > > > matter what.
> > >
> > > Yes. Well, the driver core does the unbinding no matter what, if it was
> > > told, by a user, to do so. Why is that a problem? The user then is
> > > responsible for any bad things (i.e. not able to control the device any
> > > more), if they do so.
> >
> > I don't really agree with that, because the user may simply not know what
> > the consequences of that will be. In my not so humble opinion any interface
> > allowing user space to crash the kernel is a bad one. And this is an example
> > of that.
>
> This has been in place since 2005, over 7 years now, and I have never
> heard any problems with it being used to crash the kernel, despite the
> easy ability for people to unbind all of their devices from drivers and
> instantly cause a system hang. So really doubt this is a problem in
> real life :)
>
> > > > This pretty much means that it is a bad idea to have a driver that is
> > > > exposed as a "device driver" in sysfs for memory hotplugging.
> > >
> > > Again, why? All this means is that the driver is now not connected to
> > > the device (memory in this case.) The memory is still there, still
> > > operates as before, only difference is, the driver can't touch it
> > > anymore.
> > >
> > > This is the same for any ACPI driver, and has been for years.
> >
> > Except that if this driver has been unbound and the removal is triggered by
> > an SCI, the core will just go on and remove the memory, although it may
> > be killing the kernel this way.
>
> Why would memory go away if a driver is unbound from a device? The
> device didn't go away. It's the same if the driver was a module and it
> was unloaded, you should not turn memory off in that situation, right?
Right. It looks like there's some confusion about the role of .remove()
in the ACPI subsystem, but I need to investigate it a bit more.
> Are you also going to prevent module unloading of this driver?
I'm not sure what I'm going to do with that driver at the moment to be honest. :-)
> > Arguably, this may be considered as the core's fault, but the only way to
> > fix that would be to move the code from that driver into the core and not to
> > register it as a "driver" any more. Which was my point. :-)
>
> No, I think people are totally overreacting to the unbind/bind files,
> which are there to aid in development, and in adding new device ids to
> drivers, as well as sometimes doing a hacky revoke() call.
>
> > > Please don't confuse unbind with any "normal" system operation, it is
> > > not to be used for memory hotplug, or anything else like this.
> > >
> > > Also, if you really do not want to do this, turn off the ability to
> > > unbind/bind for these devices, that is under your control in your bus
> > > logic.
> >
> > OK, but how? I'm looking at driver_unbind() and not seeing any way to do
> > that actually.
>
> See the suppress_bind_attrs field in struct device_driver. It's even
> documented in device.h, but sadly, no one reads documentation :)
That's good to know, thanks. :-)
And if I knew I could find that information in device.h, I'd look in there.
> I recommend you set this field if you don't want the bind/unbind files
> to show up for your memory driver, although I would argue that the
> driver needs to be fixed up to not do foolish things like removing
> memory from a system unless it really does go away...
Quite frankly, I need to look at that driver and how things are supposed to
work more thoroughly, because I don't seem to see a reason to do various
things the way they are done. Well, maybe it's just me.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
next prev parent reply other threads:[~2012-11-28 23:26 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-23 17:50 [RFC PATCH v3 0/3] acpi: Introduce prepare_remove device operation Vasilis Liaskovitis
2012-11-23 17:50 ` [RFC PATCH v3 1/3] acpi: Introduce prepare_remove operation in acpi_device_ops Vasilis Liaskovitis
2012-11-27 0:10 ` Toshi Kani
2012-11-27 18:36 ` Vasilis Liaskovitis
2012-11-27 23:18 ` Rafael J. Wysocki
2012-11-23 17:50 ` [RFC PATCH v3 2/3] acpi_memhotplug: Add prepare_remove operation Vasilis Liaskovitis
2012-11-24 16:23 ` Wen Congyang
2012-11-23 17:50 ` [RFC PATCH v3 3/3] acpi_memhotplug: Allow eject to proceed on rebind scenario Vasilis Liaskovitis
2012-11-24 16:20 ` Wen Congyang
2012-11-26 8:36 ` Vasilis Liaskovitis
2012-11-26 9:11 ` Wen Congyang
2012-11-27 0:19 ` Toshi Kani
2012-11-27 18:32 ` Vasilis Liaskovitis
2012-11-27 22:03 ` Toshi Kani
2012-11-27 23:41 ` Rafael J. Wysocki
2012-11-28 16:01 ` Toshi Kani
2012-11-28 18:40 ` Rafael J. Wysocki
2012-11-28 21:02 ` Toshi Kani
2012-11-28 21:40 ` Rafael J. Wysocki
2012-11-28 21:40 ` Toshi Kani
2012-11-28 22:01 ` Rafael J. Wysocki
2012-11-28 22:04 ` Toshi Kani
2012-11-28 22:21 ` Rafael J. Wysocki
2012-11-28 22:16 ` Toshi Kani
2012-11-28 22:39 ` Rafael J. Wysocki
2012-11-28 22:46 ` Greg KH
2012-11-28 23:05 ` Rafael J. Wysocki
2012-11-28 23:10 ` Greg KH
2012-11-28 23:31 ` Rafael J. Wysocki [this message]
2012-11-28 23:49 ` Rafael J. Wysocki
2012-11-29 1:02 ` Toshi Kani
2012-11-29 1:15 ` Toshi Kani
2012-11-29 10:03 ` Rafael J. Wysocki
2012-11-29 11:30 ` Vasilis Liaskovitis
2012-11-29 16:57 ` Rafael J. Wysocki
2012-11-29 17:56 ` Toshi Kani
2012-11-29 20:25 ` Rafael J. Wysocki
2012-11-29 20:38 ` Toshi Kani
2012-11-29 21:23 ` Rafael J. Wysocki
2012-11-29 21:46 ` Toshi Kani
2012-11-29 22:11 ` Rafael J. Wysocki
2012-11-29 23:17 ` Toshi Kani
2012-11-30 0:13 ` Rafael J. Wysocki
2012-11-30 1:09 ` Toshi Kani
2012-11-29 16:43 ` Toshi Kani
2012-11-29 11:04 ` Vasilis Liaskovitis
2012-11-29 17:44 ` Toshi Kani
2012-12-06 9:30 ` Vasilis Liaskovitis
2012-12-06 12:50 ` Rafael J. Wysocki
2012-12-06 15:41 ` Toshi Kani
2012-12-06 20:32 ` Rafael J. Wysocki
2012-11-28 11:05 ` [RFC PATCH v3 0/3] acpi: Introduce prepare_remove device operation Hanjun Guo
2012-11-28 18:41 ` Toshi Kani
2012-11-29 4:48 ` Hanjun Guo
2012-11-29 22:27 ` Toshi Kani
2012-12-03 4:25 ` Hanjun Guo
2012-12-04 0:10 ` Toshi Kani
2012-12-04 9:16 ` Hanjun Guo
2012-12-04 23:23 ` Toshi Kani
2012-12-05 12:10 ` Hanjun Guo
2012-12-05 22:31 ` Toshi Kani
2012-12-06 16:47 ` Jiang Liu
2012-12-07 2:25 ` Toshi Kani
2012-12-06 16:40 ` Jiang Liu
2012-12-06 20:30 ` Rafael J. Wysocki
2012-12-07 2:57 ` Toshi Kani
2012-12-07 5:57 ` Jiang Liu
2012-12-08 1:08 ` Toshi Kani
2012-12-11 14:34 ` Jiang Liu
2012-12-13 14:42 ` Toshi Kani
2012-12-13 15:15 ` Jiang Liu
2012-12-15 1:19 ` Toshi Kani
2012-11-29 10:15 ` Rafael J. Wysocki
2012-11-29 11:36 ` Vasilis Liaskovitis
2012-12-06 16:59 ` Jiang Liu
2012-11-29 17:03 ` Toshi Kani
2012-11-29 20:30 ` Rafael J. Wysocki
2012-11-29 20:39 ` Toshi Kani
2012-11-29 20:56 ` Toshi Kani
2012-11-29 21:25 ` Rafael J. Wysocki
2012-12-06 17:10 ` Jiang Liu
2012-12-06 17:07 ` Jiang Liu
2012-12-06 17:01 ` Jiang Liu
2012-12-06 16:56 ` Jiang Liu
2012-12-06 16:00 ` Jiang Liu
2012-12-06 16:03 ` Toshi Kani
2012-12-06 16:25 ` Jiang Liu
2012-12-06 16:31 ` Toshi Kani
2012-12-06 16:52 ` Jiang Liu
2012-12-06 17:09 ` Toshi Kani
2012-12-06 17:30 ` Jiang Liu
2012-12-06 17:28 ` Toshi Kani
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=2363618.GHMYUtrpGK@vostro.rjw.lan \
--to=rjw@sisk.pl \
--cc=gregkh@linuxfoundation.org \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=toshi.kani@hp.com \
--cc=vasilis.liaskovitis@profitbricks.com \
--cc=wencongyang@gmail.com \
--cc=wency@cn.fujitsu.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 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).