Date: Wed, 28 Jan 2015 18:59:08 -0700 From: Warner Losh <imp@bsdimp.com> To: John Baldwin <jhb@freebsd.org> Cc: Hans Petter Selasky <hps@selasky.org>, arch@freebsd.org Subject: Re: devctl(8): A device control utility Message-ID: <0EB7A69C-8623-49B8-96DC-AB7A84124D8A@bsdimp.com> In-Reply-To: <17592052.bbsckK6u9F@ralph.baldwin.cx> References: <3200196.9ZgXApgRdA@ralph.baldwin.cx> <54B44448.1090901@FreeBSD.org> <E3CAE124-1D8E-4A89-8113-D8301436BFE9@bsdimp.com> <17592052.bbsckK6u9F@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Jan 28, 2015, at 3:45 PM, John Baldwin <jhb@freebsd.org> wrote: > > On Wednesday, January 14, 2015 04:56:18 PM Warner Losh wrote: >>> On Jan 12, 2015, at 3:01 PM, John Baldwin <jhb@FreeBSD.org> wrote: >>> >>> On 1/12/15 12:01 PM, Warner Losh wrote: >>>>> On Jan 12, 2015, at 9:16 AM, John Baldwin <jhb@FreeBSD.org> wrote: >>>>> >>>>> On 1/5/15 4:18 PM, John Baldwin wrote: >>>>>> On Monday, January 05, 2015 09:58:19 PM Hans Petter Selasky wrote: >>>>>>> On 01/05/15 21:37, John Baldwin wrote: >>>>>>>> On 1/5/15 3:13 PM, Hans Petter Selasky wrote: >>>>>>>>> On 01/05/15 21:01, John Baldwin wrote: >>>>>>>>>> The devctl(8) utility is then a thin wrapper around libdevctl (and >>>>>>>>>> does not >>>>>>>>>> yet have a manpage). >>>>>>>>>> >>>>>>>>>> Do folks have any feedback? >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> In the USB area attach and detach must be synchronized to the USB >>>>>>>>> stack >>>>>>>>> and "usbconfig -d X.Y set_config Z" or "usbconfig -d X.Y reset" >>>>>>>>> should >>>>>>>>> be used to avoid races attaching and detaching drivers! >>>>>>>> >>>>>>>> I think this points to one or more missing bus methods so that the >>>>>>>> bus >>>>>>>> can hook into device_probe_and_attach() to reset a device as needed. >>>>>>>> (e.g. if you had bus_probe_started / bus_probe_finished and possibly >>>>>>>> similar methods for attach. PCI could use a bus_attach_finished() >>>>>>>> callback so that it could clean up any dangling resources and >>>>>>>> possibly >>>>>>>> power down on a failed attach the way it does in bus_child_detached >>>>>>>> as >>>>>>>> well). >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> USB has its own threads to allocate/free devices. Another problem is >>>>>>> how >>>>>>> to atomically get a reference count across multiple layers like PCI >>>>>>> and >>>>>>> USB. It doesn't allow probe/attach when called from outside these >>>>>>> threads. >>>>>> >>>>>> That just means you need to use some locks. :) Cardbus also uses an >>>>>> event >>>>>> thread to handle auto-attach of devices when it detected a card change >>>>>> event, but that doesn't prevent it from servicing an ioctl request. >>>>> >>>>> Another option btw would be to add bus methods that wrap probe and >>>>> attach (rather than pre and post event hooks). I wish bus_add_child() >>>>> were done this way such that device_add_child_ordered() were renamed to >>>>> bus_generic_add_child() (and was the default add_child method) and that >>>>> device_add_child_ordered() called 'BUS_ADD_CHILD()' so that >>>>> 'device_add_child()' was the proper public API (instead of exposing >>>>> BUS_ADD_CHILD()). Similarly, I think that 'device_attach()' and >>>>> 'device_probe_and_attach()' should be the public API and that one way or >>>>> another we should add hooks to allow bus drivers to modify their >>>>> behavior if needed. However, they should be fine for devctl ioctls to >>>>> invoke as well as other kernel bits. >>>> >>>> When I was doing CardBus and PC Card I wished for similar things. Then >>>> I realized I didn’t need them because as the bus author, I know when >>>> these >>>> events happened and could take appropriate actions for the bus. I didn’t >>>> have that atomic access issues though, since as the bus author I also >>>> controlled how and when mutexes were taken out and when I allowed access >>>> to the bus. I only used mutexes in CardBus and PC Card because most of >>>> the sleeps were short, but other ways to do locking are quite >>>> possible... >>> >>> I think the problem here is that devctl introduces events that happen >>> without the bus's knowledge. >> >> When we did the kludge sysctl power stuff for cardbus (which was never >> committed), we sent a message to the bus to tell it to do the power off and >> cope with whatever else was needed. There were times that it couldn’t >> comply, iirc, so this ‘command’ allowed errors to be returned for things >> that were forbidden / not allowed for some reason at the time rather than >> getting a message that this thing happened and we had to mop up now. > > devctl requests would always be ones that you can gracefully fail (they are > administrative requests, not a surprise hardware removal). I think we should > able to make that work just fine either by wrapping device_attach, etc. in new > bus methods, or adding hooks into those as bus methods. To that end, I'd like > to move forward with this current version in HEAD. At some point we can > decide which way we want to allow bus drivers to hook into these requests. I > don't think that will affect the API exposed to userland at all however, only > the in-kernel implementation. Yes. The only caveat in that would be if we wanted to have a force flag passed down that the bus wouldn’t be allowed to reject. But that’s a minor caveat, so I’m good moving forward with this model. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0EB7A69C-8623-49B8-96DC-AB7A84124D8A>