Re: [PATCH 07/47] xfs: add function pointers for get/update keys to the btree

Darrick J. Wong
2016-08-01T17:34:11Z

On Sun, Jul 31, 2016 at 11:39:02PM -0700, Christoph Hellwig wrote:
> On Wed, Jul 20, 2016 at 09:56:46PM -0700, Darrick J. Wong wrote:
> > Add some function pointers to bc_ops to get the btree keys for
> > leaf and node blocks, and to update parent keys of a block.
> > Convert the _btree_updkey calls to use our new pointer, and
> > modify the tree shape changing code to call the appropriate
> > get_*_keys pointer instead of _btree_copy_keys because the
> > overlapping btree has to calculate high key values.
>
> I don't really like to add ops for something that isn't really
> per-btree type. Can you just add an overlapping flag and act based on
> that?

That's roughly the approach I took in previous versions of this patch,
but using the OVERLAPPING flag to dispatch the overlapped vs. non-
versions of the get*keys and updkey* functions confused Dave, so he
asked me to add to function pointers[1] to the btree ops and dispatch
that way. There are still a few places where we need to know if it's
an overlapped btree (like when we update a block and have to update
the high keys, which only applies to overlapped trees), so the flag
didn't entirely go away.

Now, one thing I could do differently is move those new function
pointers to the btree cursor and add a xfs_btree_init_cursor() that
checks the flag and sets the function pointers accordingly. It
seems a little funny to me to be sprinking function pointers in both
btree_cursor and btree_ops, but I suppose the flag is in the cursor,
so having function pointers there too probably isn't so bad.

That encapsulates some btree details so that each btree doesn't have
to get them right, so I'll at least code this up and add it to the end
of my -experimental tree. I'm not sure how far Dave has gotten in
evaluating/fixing/whatever the for-next-for-4.8-2 tree to send to
Linus, so I'll hold off on folding that fix into the existing
patch unless Dave wants it for his second merge-window pull request.

--D

[1] https://www.spinics.net/lists/xfs/msg40870.html