On Sun, Sep 26, 2021 at 04:15:42PM +0100, Jonathan Cameron wrote: > On Mon, 20 Sep 2021 19:09:13 +0900 > William Breathitt Gray wrote: > > > On Sun, Sep 12, 2021 at 05:18:42PM +0100, Jonathan Cameron wrote: > > > On Fri, 27 Aug 2021 12:47:51 +0900 > > > William Breathitt Gray wrote: > > > > > > > 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 > > > > Cc: Gwendal Grignou > > > > Cc: Dan Carpenter > > > > Cc: Oleksij Rempel > > > > Signed-off-by: William Breathitt Gray > > > > > > 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 > > > > 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