Re: [PATCH] rxrpc: use kthread_ API

[David Howells]

Christoph Hellwig <hch@xxxxxx> wrote:

> > > +	krxiod_thread = kthread_run(rxrpc_krxiod, NULL, "krxiod");
> > > +	if (IS_ERR(krxiod_thread))
> > > +		return PTR_ERR(krxiod_thread);
> > > +	return 0;
> > 
> > Don't assign an error to (rxrpc_)krxiod_thread.
> 
> Well, kthread_run can returns errors.  If you want to avoid that for
> some reasons we'd need a local varibale

So what? There's an implicit local variable anyway. The compiler doesn't pass
the address of rxrpc_krxiod_thread to kthread_run() so that the latter can
fill it in; the result is returned in a register. Sticking a local variable in
the source would just attach a label to that register. Moving the assignment
to the global variable to after the error checking if-statement would then
mean the store instruction is emitted after the branch rather than before it.

> , which would be rather silly.

No. It'd probably be faster in the error case (no write to the memory).

> Note that there's nothing it could pollute, once one of these fails
> rxrpc_initialise goes to the error path, unwinds and returns a failure,
> so the rxrpc module never goes live.

At the moment, there's nothing it can pollute, you mean. Although you're
probably correct, it costs very little to program defensively in this case,
and in fact it's probably cheaper in execution time, even if it's a little
more expensive in terms of source code and work for you.

David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/