Message-ID: <m163hxrnkb.fsf@fess.ebiederm.org>
Date:	Wed, 25 Mar 2009 20:05:24 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Tejun Heo <htejun@...il.com>
Cc:	Alex Chiang <achiang@...com>, greg@...ah.com,
	cornelia.huck@...ibm.com, stern@...land.harvard.edu,
	kay.sievers@...y.org, rusty@...tcorp.com.au,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 0/3] sysfs: allow suicide

Tejun Heo <htejun@...il.com> writes:

> Thanks for the points.  I do agree that it could be a bit too clever,
> but the thing is that protecting the code area from going underneath
> something is a pretty special thing to begin with and I think it's
> better to apply special solution rather than trying to work around it
> using general mechanisms.  So, I actually think the global inhibit
> thing is one of the better ways to deal with the nasty-in-nature
> problem.

Protecting the data structures from going away is just as important,
and the module_inhibit does not address that.

When I looked at it I could not see any touches of kobj in the sysfs
code after we dropped the reference count in a strange place, but
I haven't been able to convince myself that we will be safe.

>>>> My hypothesis is once we solve this for the general case of
>>>> device hotplug and removal we won't need special support from
>>>> sysfs.  At least not in the suicidal way.
>>> I agree that we have problems in our infrastructure, especially,
>>> as you point out, overlapping device trees, etc.
>
> I don't really see how some grand general solution would solve
> deadlock problem at sysfs layer, care to elaborate a bit?

See below.  I'm really not thinking of doing anything different
just putting the code somewhere different that sysfs.

>>> I see your point about adding extra cruft into sysfs to work
>>> around a special case while leaving the hard problem unsolved.
>>>
>>> Perhaps the status quo is better. I do think that getting
>>> suicidal sysfs attributes off the global workqueue is a band-aid
>>> that actually helps, vs. the proposed patches here which are
>>> questionable in nature.
>> 
>> Sounds like it.    I'm not trying to shoot this down, rather
>> I'm trying to figure out how to solve this cleanly, as I am slowly
>> trying to sort out the pci hotplug and unplug issues.
>
> The problem I see is that there aren't too many users and the solution
> is a bit too narrow focused, but with increasing support for
> hotplug/unplug, I think the problem is becoming more widespread and
> the workqueue solution is quite fragile and cumbersome for each and
> every user, so unless there are other directions we can pursue (the
> general one above maybe?), I think it's better to add a bit of
> complexity to sysfs rather than forcing everyone user of it to do it.

My view is that this is a general hotplug problem and not a sysfs problem.
Further I see inhibiting module reload as only solving have the problem
as dropping the kobject reference opens a window to a use after free on
the kobj.

The problem that I see is that we are missing support from the device
model for hotunplug.  Running the device remove method from process
context is required.  Typically hotplug controllers discover a
device has been removed or will be removed in interrupt context.

Therefore every hotplug driver I have looked at has it's own workqueue
to solve the problem of getting the notification of a hotplug event
from an inappropriate context.

So the general problem that I see is that I need a solution to trigger
a remove from interrupt context and that same solution will happen to
work just fine for sysfs.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists