Re: [PATCH 02/36] scsi,rcu: convert call_rcu(fc_rport_free_rcu) to kfree_rcu()

On Wed, Mar 23, 2011 at 05:45:32PM -0500, James Bottomley wrote:

On Wed, 2011-03-23 at 15:24 -0700, Paul E. McKenney wrote:

quoted

On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:

quoted

On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:

quoted

The kfree_rcu() definition is as
follows:

#define kfree_rcu(ptr, rcu_head)					\
	__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))

Isn't this one of those cases where the obvious use of the interface is
definitely wrong?

It's also another nasty pseudo C prototype.  I know we do this sort of
thing for container_of et al, but I don't really think we want to extend
it.

Why not make the interface take a pointer to the embedding structure and
one to the rcu_head ... that way all pointer mathematics can be
contained inside the RCU routines.

Hello, James,

If you pass in a pair of pointers, then it is difficult for RCU to detect
bugs where the two pointers are unrelated.  Yes, you can do some sanity
checks, but these get cumbersome and have corner cases where they can
be fooled.  In contrast, Lai's interface allows the compiler to do the
needed type checking -- unless the second argument is a field of type
struct rcu_head in the structure pointed to by the first argument, the
compiler will complain.

Either way, the pointer mathematics are buried in the RCU API.

Or am I missing something here?

No ... I like the utility ... I just dislike the inelegance of having to
name a structure element in what looks like a C prototype.

I can see this proliferating everywhere since most of our reference
counting release callbacks basically free the enclosing object ...

Indeed!  Improvements are welcome -- it is just that I am not convinced
that the dual-pointer approach is really an improvement.

The C preprocessor...  It is ugly, inelegant, painful, annoying, and
should have been strangled at birth -- but it is always there when you
need it!

							Thanx, Paul