On Mon, Sep 27, 2021 at 12:20:00PM +0100, Jonathan Cameron wrote:

On Mon, 27 Sep 2021 19:21:17 +0900
William Breathitt Gray [off-list ref] wrote:

quoted

On Sun, Sep 26, 2021 at 04:15:42PM +0100, Jonathan Cameron wrote:

quoted

On Mon, 20 Sep 2021 19:09:13 +0900
William Breathitt Gray [off-list ref] wrote:
  

quoted

On Sun, Sep 12, 2021 at 05:18:42PM +0100, Jonathan Cameron wrote:  

quoted

On Fri, 27 Aug 2021 12:47:51 +0900
William Breathitt Gray [off-list ref] wrote:
    

quoted

This patch introduces a character device interface for the Counter
subsystem. Device data is exposed through standard character device read
operations. Device data is gathered when a Counter event is pushed by
the respective Counter device driver. Configuration is handled via ioctl
operations on the respective Counter character device node.

Cc: David Lechner <david@lechnology.com>
Cc: Gwendal Grignou <redacted>
Cc: Dan Carpenter <redacted>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: William Breathitt Gray <redacted>    

Hi William,

Why the bit based lock?  It feels like a mutex_trylock() type approach or
spinlock_trylock() would be a more common solution to this problem.
There is precedence for doing what you have here though so I'm not that
worried about it.    

Hi Jonathan,

We originally used a mutex for this, but Jarkko discovered that this
produced a warning because chrdev_lock would be held when returning to
user space:
https://lore.kernel.org/linux-arm-kernel/YOq19zTsOzKA8v7c@shinobu/T/#m6072133d418d598a5f368bb942c945e46cfab9a5 (local)

Following David Lechner's suggestion, I decided to reimplement
chrdev_lock as a bitmap using an atomic flag.  

Ok.  I'm not sure bit lock was quite what was intended (as there is only one of them)
but I suppose it doesn't greatly matter.  

It didn't cross my mind before, but would declaring chrdev_lock as an
atomic_t be a more appropriate solution here because we have only one
flag?

William Breathitt Gray

It would be less esoteric.  This was the first time I've ever come across the bitlock stuff
whereas atomics are an every day thing.

Thanks,

Jonathan

I agree. I'll try that out then and reimplement this using
atomic_inc_and_test() instead of test_and_set_bit_lock().

William Breathitt Gray