On Mon, Sep 30, 2024 at 01:30:58PM +0200, Przemek Kitszel wrote:

On 9/30/24 13:08, Dan Carpenter wrote:

quoted

On Mon, Sep 30, 2024 at 12:21:44PM +0200, Przemek Kitszel wrote:

quoted

Most of the time it is just easier to bend your driver than change or
extend the core of the kernel.

There is actually scoped_cond_guard() which is a trylock variant.

scoped_guard(mutex_try, &ts->mutex) you have found is semantically
wrong and must be fixed.

What?  I'm so puzzled by this conversation.

there are two variants of scoped_guard() and you have found a place
where the wrong one is used

"Yeah? Well, you know, that's just like uh, your opinion, man."

From include/linux/cleanup.h:

 * scoped_guard (name, args...) { }:
 *	similar to CLASS(name, scope)(args), except the variable (with the
 *	explicit name 'scope') is declard in a for-loop such that its scope is
 *	bound to the next (compound) statement.
 *
 *	for conditional locks the loop body is skipped when the lock is not
 *	acquired.

Please note the 2nd paragraph that explains this particular usage and
that it was done this way on purpose.

quoted

Anyway, I don't have a problem with your goal, but your macro is wrong and will
need to be re-written.  You will need to update any drivers which use the
scoped_guard() for try locks.  I don't care how you do that.  Use
scoped_cond_guard() if you want or invent a new macro.  But that work always
falls on the person changing the API.  Plus, it's only the one tsc200x-core.c
driver so I don't understand why you're making a big deal about it.

I think if you also count uses of "scoped_guard(mutex_intr, ...)" you
will find more of such examples.

apologies for upsetting you
I will send next iteration of this series with additional patches fixing
current code (thanks you for finding it for me in this case!)

No, please do not. Your "fix" it looks like will prevent writing
code like:

	scoped_guard(mutex_intr, &some_mutex) {
		do_stuff();

		return 0;
	}

	return -EINTR;

You might not like it, but it is a valid pattern.

I didn't said so in prev mail to leave you an option to send the fix for
the usage bug you have reported, just confirmed it. But by all means I'm
happy to fix current code myself.

quoted

but your macro is wrong and will need to be re-written

could you please elaborate here?

i
Dan explained that you are changing the behavior of the guards, in an
undesirable way, breaking users. Please re-read what was written before.

Thanks.

-- 
Dmitry