[RFC] How should delete_resource() handle children?

[RFC] How should delete_resource() handle children?

[Matthew Wilcox]

If you call release_resource() on a resource that has children, all
of those children are silently removed from the list too.  Does anybody
currently depend on that behaviour?

There are three valid behaviours I can think of for this:

 * Current behaviour, on the assumption that all the children of this
   resource are also owned by this device, and so removing them is the
   right thing to do.

 * BUG_ON() on the basis you should never do this.

 * Reparent the children to the parent resource of the one being released.

The first option has the problem that if the children of the resource
belong to some other part of the system, they still have pointers into
their parent which is now gone.  Memory corruption will follow.

The second option has the problem that if you *do* need to delete this
resource but keep its children linked in, you can't do it in a race-free
manner.

The third option has the problem that if you were relying on the current
behaviour of release_resource(), the parent/siblings of the deleted
resource still have references to objects you thought were deleted.


I like the third option best.  Now that we have insert_resource(), the
corresponding release_resource() should leave the children in the state
they were in before.

An alternative possibility would be to introduce a remove_resource()
that implements the reparenting option and leave the current behaviour
of release_resource() alone.


Comments?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [RFC] How should delete_resource() handle children?

[Randy.Dunlap]

On Tue, 10 Feb 2004 19:33:49 +0000 Matthew Wilcox <willy@debian.org> wrote:

| 
| If you call release_resource() on a resource that has children, all
| of those children are silently removed from the list too.  Does anybody
| currently depend on that behaviour?
| 
| There are three valid behaviours I can think of for this:
| 
|  * Current behaviour, on the assumption that all the children of this
|    resource are also owned by this device, and so removing them is the
|    right thing to do.
| 
|  * BUG_ON() on the basis you should never do this.
| 
|  * Reparent the children to the parent resource of the one being released.
| 
| The first option has the problem that if the children of the resource
| belong to some other part of the system, they still have pointers into
| their parent which is now gone.  Memory corruption will follow.
| 
| The second option has the problem that if you *do* need to delete this
| resource but keep its children linked in, you can't do it in a race-free
| manner.
| 
| The third option has the problem that if you were relying on the current
| behaviour of release_resource(), the parent/siblings of the deleted
| resource still have references to objects you thought were deleted.
| 
| 
| I like the third option best.  Now that we have insert_resource(), the
| corresponding release_resource() should leave the children in the state
| they were in before.
| 
| An alternative possibility would be to introduce a remove_resource()
| that implements the reparenting option and leave the current behaviour
| of release_resource() alone.
| 
| 
| Comments?

Ideally (or if nothing depends on the current behavior), I think it
should just be an error (return -EINVAL), not a BUG_ON().  I.e.,
releasing a resource should be an explicit action.

Do we know of cases where parents are removed but children need to
remain?  Examples?

--
~Randy

Re: [RFC] How should delete_resource() handle children?

[Nick Piggin]

Randy.Dunlap wrote:

>On Tue, 10 Feb 2004 19:33:49 +0000 Matthew Wilcox <willy@debian.org> wrote:
>
>| 
>| If you call release_resource() on a resource that has children, all
>| of those children are silently removed from the list too.  Does anybody
>| currently depend on that behaviour?
>| 
>
...

>| 
>| 
>| Comments?
>
>Ideally (or if nothing depends on the current behavior), I think it
>should just be an error (return -EINVAL), not a BUG_ON().  I.e.,
>releasing a resource should be an explicit action.
>
>

-EBUSY?

Re: [RFC] How should delete_resource() handle children?

[Randy.Dunlap]

On Thu, 19 Feb 2004 13:12:18 +1100 Nick Piggin <piggin@cyberone.com.au> wrote:

| 
| 
| Randy.Dunlap wrote:
| 
| >On Tue, 10 Feb 2004 19:33:49 +0000 Matthew Wilcox <willy@debian.org> wrote:
| >
| >| 
| >| If you call release_resource() on a resource that has children, all
| >| of those children are silently removed from the list too.  Does anybody
| >| currently depend on that behaviour?
| >| 
| >
| ...
| 
| >| 
| >| 
| >| Comments?
| >
| >Ideally (or if nothing depends on the current behavior), I think it
| >should just be an error (return -EINVAL), not a BUG_ON().  I.e.,
| >releasing a resource should be an explicit action.
| >
| >
| 
| -EBUSY?

Yes, that makes more sense.

--
~Randy

Re: [RFC] How should delete_resource() handle children?

[Matthew Wilcox]

On Wed, Feb 18, 2004 at 05:48:00PM -0800, Randy.Dunlap wrote:
> Ideally (or if nothing depends on the current behavior), I think it
> should just be an error (return -EINVAL), not a BUG_ON().  I.e.,
> releasing a resource should be an explicit action.

-EBUSY, perhaps?

> Do we know of cases where parents are removed but children need to
> remain?  Examples?

I don't know of any in-tree.  I was originally just looking at this code
while writing adjust_resource() and insert_resource() and noticed this
corner case.

But now, I was thinking about using the resource management code to handle
the PCI hotplug resource management (which currently is done independently
by each hotplug driver).  Since the drivers can be loaded independently
of everything under them, they'd need to use insert_resource() and
remove_resource() to avoid disturbing the child resources.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain