Re: owner field in `struct fs'

[Philipp Rumpf]

On Tue, Jun 27, 2000 at 10:05:49AM +0100, David Woodhouse wrote:
> [e-mail] said:
> >  Actually, one rather nice side-effect of the grab-all-CPU's patch is
> > you can get rid of try_inc_mod_count (at least if you keep your other
> > locking structures sane).  It always succeeds.
> 
> It's not obviously and immediately clear to me on first glance that it's
> correct.

1 cleanup_module is the only thing that runs, on all CPUs.
2 cleanup_module unregisters its structure.

There is just no way you can do anything (like try_inc_mod_count and fail
between 1 and 2).

Even if cleanup_module looks like this:
	
	down(&sem);
	unregister_structure();
	up(&sem);

it will work, as everyone who can down(&sem) either incremented the module
use count before or is a dependent module (which incremented the module
usecount as well).

Of course,

	if(down_trylock(&sem))
		BUG();
	unregister_structure();
	up(&sem);

would be slightly cleaner.  (Hey, and BUG() unloads the module as well).

No, it's not quite obvious.  IMHO it's less subtle than "what do I do if
try_inc_mod_count failed" though.

> Therefore, it's too complex for my liking.

It's a complex problem.  try_inc_mod_count has the complexity in the failure
code.  MOD_INC_USE_COUNT/grab_all_cpus tries to avoid running into the
complex problems at runtime.

> The current setup is relatively easy for even a driver writer to understand
> and get right.

How do you handle try_inc_mod_count in the generic case ?  It sure doesn't
look easy to me (in fact, it looks to me like you need to write tons of code
for every subsystem that needs to do it).

> As soon as we start making them think, they'll find new and innovative
> ways to screw up - in this case possibly by causing deadlocks.

They might find innovative ways to copy-and-paste the example code and run
into a BUG().  With try_inc_mod_count they'd find ways of breaking auto
unloading.