On 8/19/20 6:11 AM, Greg KH wrote:

On Wed, Aug 19, 2020 at 07:00:53AM -0600, Jens Axboe wrote:

quoted

On 8/18/20 1:00 PM, James Bottomley wrote:

quoted

On Mon, 2020-08-17 at 13:02 -0700, Jens Axboe wrote:

quoted

On 8/17/20 12:48 PM, Kees Cook wrote:

quoted

On Mon, Aug 17, 2020 at 12:44:34PM -0700, Jens Axboe wrote:

quoted

On 8/17/20 12:29 PM, Kees Cook wrote:

quoted

On Mon, Aug 17, 2020 at 06:56:47AM -0700, Jens Axboe wrote:

quoted

On 8/17/20 2:15 AM, Allen Pais wrote:

quoted

From: Allen Pais <redacted>

In preparation for unconditionally passing the
struct tasklet_struct pointer to all tasklet
callbacks, switch to using the new tasklet_setup()
and from_tasklet() to pass the tasklet pointer explicitly.

Who came up with the idea to add a macro 'from_tasklet' that
is just container_of? container_of in the code would be
_much_ more readable, and not leave anyone guessing wtf
from_tasklet is doing.

I'd fix that up now before everything else goes in...

As I mentioned in the other thread, I think this makes things
much more readable. It's the same thing that the timer_struct
conversion did (added a container_of wrapper) to avoid the
ever-repeating use of typeof(), long lines, etc.

But then it should use a generic name, instead of each sub-system 
using some random name that makes people look up exactly what it
does. I'm not huge fan of the container_of() redundancy, but
adding private variants of this doesn't seem like the best way
forward. Let's have a generic helper that does this, and use it
everywhere.

I'm open to suggestions, but as things stand, these kinds of
treewide

On naming? Implementation is just as it stands, from_tasklet() is
totally generic which is why I objected to it. from_member()? Not
great with naming... But I can see this going further and then we'll
suddenly have tons of these. It's not good for readability.

Since both threads seem to have petered out, let me suggest in
kernel.h:

#define cast_out(ptr, container, member) \
	container_of(ptr, typeof(*container), member)

It does what you want, the argument order is the same as container_of
with the only difference being you name the containing structure
instead of having to specify its type.

Not to incessantly bike shed on the naming, but I don't like cast_out,
it's not very descriptive. And it has connotations of getting rid of
something, which isn't really true.

I agree, if we want to bike shed, I don't like this color either.

quoted

FWIW, I like the from_ part of the original naming, as it has some clues
as to what is being done here. Why not just from_container()? That
should immediately tell people what it does without having to look up
the implementation, even before this becomes a part of the accepted
coding norm.

Why are people hating on the well-known and used container_of()?

If you really hate to type the type and want a new macro, what about
'container_from()'?  (noun/verb is nicer to sort symbols by...)

But really, why is this even needed?

container_from() or from_container(), either works just fine for me
in terms of naming.

I think people are hating on it because it makes for _really_ long
lines, and it's arguably cleaner/simpler to just pass in the pointer
type instead. Then you end up with lines like this:

	struct request_queue *q =                                               
		container_of(work, struct request_queue, requeue_work.work);  

But I'm not the one that started this addition of from_tasklet(), my
objection was adding a private macro for something that should be
generic functionality. Hence I think we either need to provide that, or
tell the from_tasklet() folks that they should just use container_of().

-- 
Jens Axboe