Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: David Lechner <david@lechnology.com>
Cc: jic23@kernel.org, kamel.bouhara@bootlin.com,
	gwendal@chromium.org, alexandre.belloni@bootlin.com,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com,
	patrick.havelange@essensium.com, fabrice.gasnier@st.com,
	mcoquelin.stm32@gmail.com, alexandre.torgue@st.com
Subject: Re: [PATCH v5 3/5] counter: Add character device interface
Date: Sun, 25 Oct 2020 09:18:09 -0400
Message-ID: <20201025131809.GB3458@shinobu> (raw)
In-Reply-To: <f2bac8b2-108d-fa4c-cb63-8ff85ce04d1f@lechnology.com>


[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

On Tue, Oct 20, 2020 at 11:06:42AM -0500, David Lechner wrote:
> On 10/18/20 11:58 AM, William Breathitt Gray wrote:
> > On Wed, Oct 14, 2020 at 05:40:44PM -0500, David Lechner wrote:
> >> On 9/26/20 9:18 PM, William Breathitt Gray wrote:
> >>> +static ssize_t counter_chrdev_read(struct file *filp, char __user *buf,
> >>> +				   size_t len, loff_t *f_ps)
> >>> +{
> >>> +	struct counter_device *const counter = filp->private_data;
> >>> +	int err;
> >>> +	unsigned long flags;
> >>> +	unsigned int copied;
> >>> +
> >>> +	if (len < sizeof(struct counter_event))
> >>> +		return -EINVAL;
> >>> +
> >>> +	do {
> >>> +		if (kfifo_is_empty(&counter->events)) {
> >>> +			if (filp->f_flags & O_NONBLOCK)
> >>> +				return -EAGAIN;
> >>> +
> >>> +			err = wait_event_interruptible(counter->events_wait,
> >>> +					!kfifo_is_empty(&counter->events));
> >>> +			if (err)
> >>> +				return err;
> >>> +		}
> >>> +
> >>> +		raw_spin_lock_irqsave(&counter->events_lock, flags);
> >>> +		err = kfifo_to_user(&counter->events, buf, len, &copied);
> >>> +		raw_spin_unlock_irqrestore(&counter->events_lock, flags);
> >>> +		if (err)
> >>> +			return err;
> >>> +	} while (!copied);
> >>> +
> >>> +	return copied;
> >>> +}
> >>
> >> All other uses of kfifo_to_user() I saw use a mutex instead of spin
> >> lock. I don't see a reason for disabling interrupts here.
> > 
> > The Counter character device interface is special in this case because
> > counter->events could be accessed from an interrupt context. This is
> > possible if counter_push_event() is called for an interrupt (as is the
> > case for the 104_quad_8 driver). In this case, we can't use mutex
> > because we can't sleep in an interrupt context, so our only option is to
> > use spin lock.
> > 
> 
> 
> The way I understand it, locking is only needed for concurrent readers
> and locking between reader and writer is not needed.

You're right, it does say in the kfifo.h comments that with only one
concurrent reader and one current write, we don't need extra locking to
use these macros. Because we only have one kfifo_to_user() operating on
counter->events, does that mean we don't need locking at all here for
the counter_chrdev_read() function?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-27  2:18 [PATCH v5 0/5] Introduce the Counter " William Breathitt Gray
2020-09-27  2:18 ` [PATCH v5 2/5] docs: counter: Update to reflect sysfs internalization William Breathitt Gray
2020-09-27  2:18 ` [PATCH v5 3/5] counter: Add character device interface William Breathitt Gray
2020-10-14  1:40   ` David Lechner
2020-10-18 16:49     ` William Breathitt Gray
2020-10-20 15:53       ` David Lechner
2020-10-25 12:55         ` William Breathitt Gray
2020-10-25 16:36           ` David Lechner
2020-10-14 17:43   ` David Lechner
2020-10-14 19:05     ` William Breathitt Gray
2020-10-14 22:32       ` David Lechner
2020-10-14 22:40   ` David Lechner
2020-10-18 16:58     ` William Breathitt Gray
2020-10-20 16:06       ` David Lechner
2020-10-25 13:18         ` William Breathitt Gray [this message]
2020-10-25 16:34           ` David Lechner
2020-10-25 17:53             ` William Breathitt Gray
2020-09-27  2:18 ` [PATCH v5 4/5] docs: counter: Document " William Breathitt Gray
2020-10-08  8:09   ` Pavel Machek
2020-10-08 12:28     ` William Breathitt Gray
2020-10-12 17:04       ` David Lechner
2020-10-13 18:58         ` William Breathitt Gray
2020-10-13 19:08           ` David Lechner
2020-10-13 19:27             ` William Breathitt Gray
2020-09-27  2:18 ` [PATCH v5 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 William Breathitt Gray
2020-10-14  0:13   ` David Lechner
2020-10-18 14:50     ` William Breathitt Gray
2020-10-13  0:35 ` [PATCH v5 0/5] Introduce the Counter character device interface David Lechner
2020-10-18 14:14   ` William Breathitt Gray
     [not found] ` <e38f6dc3a08bf2510034334262776a6ed1df8b89.1601170670.git.vilhelm.gray@gmail.com>
2020-10-13  2:15   ` [PATCH v5 1/5] counter: Internalize sysfs interface code David Lechner
2020-10-18 14:49     ` William Breathitt Gray
2020-10-20 15:38       ` David Lechner
2020-10-23 13:12         ` William Breathitt Gray
2020-10-15  1:38   ` David Lechner
2020-10-18 17:00     ` William Breathitt Gray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201025131809.GB3458@shinobu \
    --to=vilhelm.gray@gmail.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alexandre.torgue@st.com \
    --cc=david@lechnology.com \
    --cc=fabrice.gasnier@st.com \
    --cc=gwendal@chromium.org \
    --cc=jic23@kernel.org \
    --cc=kamel.bouhara@bootlin.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=patrick.havelange@essensium.com \
    --cc=syednwaris@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git