Re: [PATCH] radix_tree: take radix_tree_path off stack

[Hugh Dickins <hughd@google.com>]

On Thu, 22 Dec 2011, Dave Chinner wrote:
> On Tue, Dec 20, 2011 at 10:53:17PM -0800, Hugh Dickins wrote:
> > On Wed, 21 Dec 2011, Dave Chinner wrote:
> > 
> > We do need to set node->parent NULL in all cases (and cannot clear
> > it when freeing).  I chose the "slot = blah(slot)" style to follow the
> > "newptr = blah(newptr)" over in radix_tree_shrink(), thought it helped
> > to keep those blocks alike.
> 
> You're right. I really was being dense yesterday. To tell the truth,
> though, I found the "newptr" style easier to follow because it was
> obvious which was the object being initialised. I think that it not
> being obvious which object needed full initialisation contribted to
> my mix up of node and slot parent pointers in my above comment...

Yes, I too get confused between parent and child and node and slot,
slot being a node which contains an array of slots.  I did experiment
with saying "parent" instead of "node" in several of the places touched,
or "child" instead of "slot", but didn't end up with anything that
satisfied me very much; so stuck with the bland node and slot.

> > At the top of the hunk, we can see the tag_set(slot, settag, offset)
> > where it sets the tag in the leafnode "slot"; then it loops up to parent
> > "node" of slot, to parent of parent, etc, setting tag in those, but
> > breaking as soon as it finds the tag already set - it can be sure that
> > the tag must already be set on all nodes above.
> > 
> > If afterwards it comes to set tag at another offset (most likely the
> > very next) in this same leafnode, we know that it has already set tag
> > on the parent, the parent's parent etc., so need not bother to tag_get
> > from the level above to discover that.  And since we happen to have a
> > variable "node" which stops the loop when it's NULL, let's set it to
> > NULL now to stop the loop immediately in future.
> 
> Ok, gotcha. perhaps a more expansive comment along the lines of:
> 
> /*
>  * we can clear the node pointer now as all it's ancestors have the
>  * tage set due to setting it on the slot above. Hence we have no
>  * need to walk back up the tree to set tags if there is no further
>  * tags to set.
>  */
> 
> is in order to remind me in a few months time why it this was done?

I've plagiarized your wording, but changed it enough that I cannot
honestly cite you as the Author.  Incremental patch to akpm follows
in a moment.

Hugh

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>