Re: Slab: Remove kmem_cache_t

Re: Slab: Remove kmem_cache_t

[Nick Piggin]

Linus Torvalds wrote:

> So typedefs are good for
> 
>  - "u8"/"u16"/"u32"/"u64" kind of things, where the underlying types 
>    really are potentially different on different architectures.
> 
>  - "sector_t"-like things which may be 32-bit or 64-bit depending on some 
>    CONFIG_LBD option or other.
> 
>  - as a special case, "sparse" actually makes bitwise typedefs have real 
>    meaning as types, so if you are using sparse to distinguish between a 
>    little-endian 16-bit entity or a big-endian 16-bit entity, the typedef 
>    there is actually important and has real meaning to sparse (without the 
>    typedef, each bitwise type declaration would be strictly a _different_ 
>    type from another bitwise type declaration that otherwise looks the 
>    same).
> 
> But typedefs are NOT good for:
> 
>  - trying to avoid typing a few characters:
> 
> 	"kmem_cache_t" is strictly _worse_ than "struct kmem_cache", not 
> 	just because it causes declaration issues. It also hides the fact 
> 	that the thing really is a structure (and hiding the fact that 
> 	it's a pointer is a shooting offense: things like "voidptr_t" 
> 	should not be allowed at all)
> 
>  - incorrect "portability". 
> 
> 	the POSIX "socklen_t" was not only a really bad way to write
> 	"int", it actually caused a lot of NON-portability, and made some 
> 	people think it should be "size_t" or something equally broken.
> 
> The one excuse for typedefs in the "typing" sense can be complicated 
> function pointer types. Some function pointers are just too easy to screw 
> up, and using a
> 
> 	typedef (*myfn_t)(int, ...);
> 
> can be preferable over forcing people to write that really complex kind of 
> type out every time. But that shouldn't be overused either (but we use it 
> for things like "readdir_t", for example, for exactly this reason).


You are saying that they should only be used to create new "primitive"
types (ie. that you can use in arithmetic / logical ops) that can
change depending on the config.

That's fair enough. I'm sure you've also said in the past that they can
be used (IIRC you even encouraged it) when the type is opaque in the
context it is being used. I won't bother trying to dig out the post,
because I could be wrong and you are entitled to change your mind. I
just want to get this straight.

Thanks,
Nick

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: Slab: Remove kmem_cache_t

[Linus Torvalds]

On Wed, 29 Nov 2006, Nick Piggin wrote:
> 
> You are saying that they should only be used to create new "primitive"
> types (ie. that you can use in arithmetic / logical ops) that can
> change depending on the config.

Well, it doesn't have to be something that is "arithmetic".

For an example of a primitive type that isn't arithmetic, the page table 
entries are (pgt_t/pud_t/pmd_t/pte_t) are excellent - they don't do any 
arithmetic or logical ops, but they do change depending on config, and no, 
they aren't always opaque structures.

(Actually, these days they mostly are, but on many architectures it's much 
slower to pass even a small struct around than it is to pass an integer 
around - due simply to calling conventions - so for truly opaque things, 
the typedef has the advantage that it _can_ be an opaque integer type, and 
nobody will notice).

> That's fair enough. I'm sure you've also said in the past that they can
> be used (IIRC you even encouraged it) when the type is opaque in the
> context it is being used.

I'm sure I've been inconsistent, but in general, typedefs are bad. I think 
you'll notice that I almost never use them myself. I much prefer passing 
an opaque structure around, _unless_ I know the structure is so small that 
it makes sense to do the above optimization (ie allow the case where the 
opaque thing actually ends up being an integer).

Opaque integer types are generally useless in C, because they lose all the 
type information _way_ too easily. There are no warnings for mis-use, 
unless you use a sparse "bitwise" type and actually run sparse on the 
thing. So even when there are performance reasons to use opaque integer 
types (and on x86, the page table things were one such thing), usign a 
struct is often preferable just for type-checking.

And as mentioned, there _are_ exceptions. Some types just get _sooo_ 
complex that it's inconvenient to type them out, even if they are 
perfectly regular types, and don't depend on any config option. The 
"filldir_t" typedef in fs.h is such an example - it's not really opaque, 
_nor_ is it a config option, but it sure as hell would be inconvenient for 
all low-level filesystems to do

	int my_readdir(struct file *filp, void *dirent,
		int (*filldir)(void *, const char *, int, loff_t,
		u64, unsigned))
	{
		...
	}

because let's face it, having to write out that "filldir" type just made 
me use two lines (and potential for totally unnecessary tupos) because the 
thing was so complex. So at that point, using a typedef is just common 
sense, and we can do

	int my_readdir(struct file *filp, void *dirent, filldir_t filldir)
	{
		...
	}

instead.

But it's really quite hard to make that kind of complex type in C. It's 
almost always a function pointer that takes complex arguments.

[ That said, I generally won't _complain_ if people use typedefs, but on 
  the other hand, some people definitely are too eager to do it, and I'll 
  happily remove them if people send me a patch. For example, we used to 
  have "task_t" for "struct task_struct", and that was just _unnecessary_, 
  and made it just harder to pick out what it was. Sometimes long names 
  and the explicit "struct" is a _good_ thing. ]

One final thing: for _small_ structures, typedefs are much better than for 
large ones. Why? Because of stack usage. I want people to really _think_ 
about local variable sizes, and that's one thing that a typedef sometimes 
causes - especially if it's opaque, so that users don't have any "handle" 
on whether it is big or small, it's really nasty to use them for automatic 
storage on the stack, because you may simply blow your stack usage on a 
single (or a couple) of structures.

Making things be "struct something_or_other" makes at least _me_ think 
more about it than if it's "file_t". Maybe it's just me, but I immediately 
think "complex structure" when I see "struct", but "file_t" to me mentally 
says "single word".

			Linus