From: Greg KH <gregkh@linuxfoundation.org>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
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: Wed, 28 Nov 2012 15:10:46 -0800 [thread overview]
Message-ID: <20121128231046.GA15416@kroah.com> (raw)
In-Reply-To: <4960597.PcG7YIEMVH@vostro.rjw.lan>
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?
Are you also going to prevent module unloading of this driver?
> 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 :)
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...
hope this helps,
greg k-h
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@linuxfoundation.org>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
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: Wed, 28 Nov 2012 15:10:46 -0800 [thread overview]
Message-ID: <20121128231046.GA15416@kroah.com> (raw)
In-Reply-To: <4960597.PcG7YIEMVH@vostro.rjw.lan>
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?
Are you also going to prevent module unloading of this driver?
> 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 :)
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...
hope this helps,
greg k-h
next prev parent reply other threads:[~2012-11-28 23:10 UTC|newest]
Thread overview: 191+ 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 ` 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-23 17:50 ` Vasilis Liaskovitis
2012-11-27 0:10 ` Toshi Kani
2012-11-27 0:10 ` Toshi Kani
2012-11-27 18:36 ` Vasilis Liaskovitis
2012-11-27 18:36 ` Vasilis Liaskovitis
2012-11-27 23:18 ` Rafael J. Wysocki
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-23 17:50 ` Vasilis Liaskovitis
2012-11-24 16:23 ` Wen Congyang
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-23 17:50 ` Vasilis Liaskovitis
2012-11-24 16:20 ` Wen Congyang
2012-11-24 16:20 ` Wen Congyang
2012-11-26 8:36 ` Vasilis Liaskovitis
2012-11-26 8:36 ` Vasilis Liaskovitis
2012-11-26 9:11 ` Wen Congyang
2012-11-26 9:11 ` Wen Congyang
2012-11-27 0:19 ` Toshi Kani
2012-11-27 0:19 ` Toshi Kani
2012-11-27 18:32 ` Vasilis Liaskovitis
2012-11-27 18:32 ` Vasilis Liaskovitis
2012-11-27 22:03 ` Toshi Kani
2012-11-27 22:03 ` Toshi Kani
2012-11-27 23:41 ` Rafael J. Wysocki
2012-11-27 23:41 ` Rafael J. Wysocki
2012-11-28 16:01 ` Toshi Kani
2012-11-28 16:01 ` Toshi Kani
2012-11-28 18:40 ` Rafael J. Wysocki
2012-11-28 18:40 ` Rafael J. Wysocki
2012-11-28 21:02 ` Toshi Kani
2012-11-28 21:02 ` Toshi Kani
2012-11-28 21:40 ` Rafael J. Wysocki
2012-11-28 21:40 ` Rafael J. Wysocki
2012-11-28 21:40 ` Toshi Kani
2012-11-28 21:40 ` Toshi Kani
2012-11-28 22:01 ` Rafael J. Wysocki
2012-11-28 22:01 ` Rafael J. Wysocki
2012-11-28 22:04 ` Toshi Kani
2012-11-28 22:04 ` Toshi Kani
2012-11-28 22:21 ` Rafael J. Wysocki
2012-11-28 22:21 ` Rafael J. Wysocki
2012-11-28 22:16 ` Toshi Kani
2012-11-28 22:16 ` Toshi Kani
2012-11-28 22:39 ` Rafael J. Wysocki
2012-11-28 22:39 ` Rafael J. Wysocki
2012-11-28 22:46 ` Greg KH
2012-11-28 22:46 ` Greg KH
2012-11-28 23:05 ` Rafael J. Wysocki
2012-11-28 23:05 ` Rafael J. Wysocki
2012-11-28 23:10 ` Greg KH [this message]
2012-11-28 23:10 ` Greg KH
2012-11-28 23:31 ` Rafael J. Wysocki
2012-11-28 23:31 ` Rafael J. Wysocki
2012-11-28 23:49 ` Rafael J. Wysocki
2012-11-28 23:49 ` Rafael J. Wysocki
2012-11-29 1:02 ` Toshi Kani
2012-11-29 1:02 ` Toshi Kani
2012-11-29 1:15 ` Toshi Kani
2012-11-29 1:15 ` Toshi Kani
2012-11-29 10:03 ` Rafael J. Wysocki
2012-11-29 10:03 ` Rafael J. Wysocki
2012-11-29 11:30 ` Vasilis Liaskovitis
2012-11-29 11:30 ` Vasilis Liaskovitis
2012-11-29 16:57 ` Rafael J. Wysocki
2012-11-29 16:57 ` Rafael J. Wysocki
2012-11-29 17:56 ` Toshi Kani
2012-11-29 17:56 ` Toshi Kani
2012-11-29 20:25 ` Rafael J. Wysocki
2012-11-29 20:25 ` Rafael J. Wysocki
2012-11-29 20:38 ` Toshi Kani
2012-11-29 20:38 ` Toshi Kani
2012-11-29 21:23 ` Rafael J. Wysocki
2012-11-29 21:23 ` Rafael J. Wysocki
2012-11-29 21:46 ` Toshi Kani
2012-11-29 21:46 ` Toshi Kani
2012-11-29 22:11 ` Rafael J. Wysocki
2012-11-29 22:11 ` Rafael J. Wysocki
2012-11-29 23:17 ` Toshi Kani
2012-11-29 23:17 ` Toshi Kani
2012-11-30 0:13 ` Rafael J. Wysocki
2012-11-30 0:13 ` Rafael J. Wysocki
2012-11-30 1:09 ` Toshi Kani
2012-11-30 1:09 ` Toshi Kani
2012-11-29 16:43 ` Toshi Kani
2012-11-29 16:43 ` Toshi Kani
2012-11-29 11:04 ` Vasilis Liaskovitis
2012-11-29 11:04 ` Vasilis Liaskovitis
2012-11-29 17:44 ` Toshi Kani
2012-11-29 17:44 ` Toshi Kani
2012-12-06 9:30 ` Vasilis Liaskovitis
2012-12-06 9:30 ` Vasilis Liaskovitis
2012-12-06 12:50 ` Rafael J. Wysocki
2012-12-06 12:50 ` Rafael J. Wysocki
2012-12-06 15:41 ` Toshi Kani
2012-12-06 15:41 ` Toshi Kani
2012-12-06 20:32 ` Rafael J. Wysocki
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 11:05 ` Hanjun Guo
2012-11-28 11:05 ` Hanjun Guo
2012-11-28 18:41 ` Toshi Kani
2012-11-28 18:41 ` Toshi Kani
2012-11-29 4:48 ` Hanjun Guo
2012-11-29 4:48 ` Hanjun Guo
2012-11-29 4:48 ` Hanjun Guo
2012-11-29 22:27 ` Toshi Kani
2012-11-29 22:27 ` Toshi Kani
2012-12-03 4:25 ` Hanjun Guo
2012-12-03 4:25 ` Hanjun Guo
2012-12-03 4:25 ` Hanjun Guo
2012-12-04 0:10 ` Toshi Kani
2012-12-04 0:10 ` Toshi Kani
2012-12-04 9:16 ` Hanjun Guo
2012-12-04 9:16 ` Hanjun Guo
2012-12-04 9:16 ` Hanjun Guo
2012-12-04 23:23 ` Toshi Kani
2012-12-04 23:23 ` Toshi Kani
2012-12-05 12:10 ` Hanjun Guo
2012-12-05 12:10 ` Hanjun Guo
2012-12-05 12:10 ` Hanjun Guo
2012-12-05 22:31 ` Toshi Kani
2012-12-05 22:31 ` Toshi Kani
2012-12-06 16:47 ` Jiang Liu
2012-12-06 16:47 ` Jiang Liu
2012-12-07 2:25 ` Toshi Kani
2012-12-07 2:25 ` Toshi Kani
2012-12-06 16:40 ` Jiang Liu
2012-12-06 16:40 ` Jiang Liu
2012-12-06 16:40 ` Jiang Liu
2012-12-06 20:30 ` Rafael J. Wysocki
2012-12-06 20:30 ` Rafael J. Wysocki
2012-12-07 2:57 ` Toshi Kani
2012-12-07 2:57 ` Toshi Kani
2012-12-07 5:57 ` Jiang Liu
2012-12-07 5:57 ` Jiang Liu
2012-12-07 5:57 ` Jiang Liu
2012-12-08 1:08 ` Toshi Kani
2012-12-08 1:08 ` Toshi Kani
2012-12-11 14:34 ` Jiang Liu
2012-12-11 14:34 ` Jiang Liu
2012-12-13 14:42 ` Toshi Kani
2012-12-13 14:42 ` Toshi Kani
2012-12-13 15:15 ` Jiang Liu
2012-12-13 15:15 ` Jiang Liu
2012-12-15 1:19 ` Toshi Kani
2012-12-15 1:19 ` Toshi Kani
2012-11-29 10:15 ` Rafael J. Wysocki
2012-11-29 10:15 ` Rafael J. Wysocki
2012-11-29 11:36 ` Vasilis Liaskovitis
2012-11-29 11:36 ` Vasilis Liaskovitis
2012-12-06 16:59 ` Jiang Liu
2012-12-06 16:59 ` Jiang Liu
2012-11-29 17:03 ` Toshi Kani
2012-11-29 17:03 ` Toshi Kani
2012-11-29 20:30 ` Rafael J. Wysocki
2012-11-29 20:30 ` Rafael J. Wysocki
2012-11-29 20:39 ` Toshi Kani
2012-11-29 20:39 ` Toshi Kani
2012-11-29 20:56 ` Toshi Kani
2012-11-29 20:56 ` Toshi Kani
2012-11-29 21:25 ` Rafael J. Wysocki
2012-11-29 21:25 ` Rafael J. Wysocki
2012-12-06 17:10 ` Jiang Liu
2012-12-06 17:10 ` Jiang Liu
2012-12-06 17:07 ` Jiang Liu
2012-12-06 17:07 ` Jiang Liu
2012-12-06 17:01 ` Jiang Liu
2012-12-06 17:01 ` Jiang Liu
2012-12-06 16:56 ` Jiang Liu
2012-12-06 16:56 ` Jiang Liu
2012-12-06 16:00 ` Jiang Liu
2012-12-06 16:00 ` Jiang Liu
2012-12-06 16:03 ` Toshi Kani
2012-12-06 16:03 ` Toshi Kani
2012-12-06 16:25 ` Jiang Liu
2012-12-06 16:25 ` Jiang Liu
2012-12-06 16:31 ` Toshi Kani
2012-12-06 16:31 ` Toshi Kani
2012-12-06 16:52 ` Jiang Liu
2012-12-06 16:52 ` Jiang Liu
2012-12-06 17:09 ` Toshi Kani
2012-12-06 17:09 ` Toshi Kani
2012-12-06 17:30 ` Jiang Liu
2012-12-06 17:30 ` Jiang Liu
2012-12-06 17:28 ` Toshi Kani
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=20121128231046.GA15416@kroah.com \
--to=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=rjw@sisk.pl \
--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 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.