mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Valentin Vidic <Valentin.Vidic@CARNet.hr>
To: Marcus Wolf <linux@Wolf-Entwicklungen.de>
Cc: "Simon Sandström" <simon@nikanor.nu>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Luca Söthe" <luca@acul.me>,
	"Marcin Ciupak" <marcin.s.ciupak@gmail.com>,
	"Michael Panzlaff" <michael.panzlaff@fau.de>,
	"Derek Robson" <robsonde@gmail.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	"Valentin Vidic" <Valentin.Vidic@CARNet.hr>
Subject: [PATCH] staging: pi433: cleanup tx_fifo locking
Date: Sat, 24 Mar 2018 23:09:01 +0100	[thread overview]
Message-ID: <20180324220901.28864-1-Valentin.Vidic@CARNet.hr> (raw)
In-Reply-To: <20180323221821.GP31333@gavran.carpriv.carnet.hr>

pi433_write requires locking due to multiple kfifo writers.  After
acquiring the lock check if enough free space is available in the kfifo
to write the whole message. This check should prevent partial writes to
kfifo so kfifo_reset is not needed anymore.

pi433_tx_thread is the only kfifo reader so it does not require locking
after kfifo_reset is also removed.

Signed-off-by: Valentin Vidic <Valentin.Vidic@CARNet.hr>
---
 drivers/staging/pi433/pi433_if.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index d1e0ddbc79ce..97239b0eb322 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -87,7 +87,7 @@ struct pi433_device {
 
 	/* tx related values */
 	STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo;
-	struct mutex		tx_fifo_lock; // TODO: check, whether necessary or obsolete
+	struct mutex		tx_fifo_lock; /* serialize multiple writers */
 	struct task_struct	*tx_task_struct;
 	wait_queue_head_t	tx_wait_queue;
 	u8			free_in_fifo;
@@ -589,19 +589,15 @@ pi433_tx_thread(void *data)
 		 * - size of message
 		 * - message
 		 */
-		mutex_lock(&device->tx_fifo_lock);
-
 		retval = kfifo_out(&device->tx_fifo, &tx_cfg, sizeof(tx_cfg));
 		if (retval != sizeof(tx_cfg)) {
 			dev_dbg(device->dev, "reading tx_cfg from fifo failed: got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg));
-			mutex_unlock(&device->tx_fifo_lock);
 			continue;
 		}
 
 		retval = kfifo_out(&device->tx_fifo, &size, sizeof(size_t));
 		if (retval != sizeof(size_t)) {
 			dev_dbg(device->dev, "reading msg size from fifo failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t));
-			mutex_unlock(&device->tx_fifo_lock);
 			continue;
 		}
 
@@ -634,7 +630,6 @@ pi433_tx_thread(void *data)
 				   sizeof(device->buffer) - position);
 		dev_dbg(device->dev,
 			"read %d message byte(s) from fifo queue.", retval);
-		mutex_unlock(&device->tx_fifo_lock);
 
 		/* if rx is active, we need to interrupt the waiting for
 		 * incoming telegrams, to be able to send something.
@@ -818,7 +813,7 @@ pi433_write(struct file *filp, const char __user *buf,
 	struct pi433_instance	*instance;
 	struct pi433_device	*device;
 	int                     retval;
-	unsigned int		copied;
+	unsigned int		required, available, copied;
 
 	instance = filp->private_data;
 	device = instance->device;
@@ -833,6 +828,16 @@ pi433_write(struct file *filp, const char __user *buf,
 	 * - message
 	 */
 	mutex_lock(&device->tx_fifo_lock);
+
+	required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
+	available = kfifo_avail(&device->tx_fifo);
+	if (required > available) {
+		dev_dbg(device->dev, "write to fifo failed: %d bytes required but %d available",
+			required, available);
+		mutex_unlock(&device->tx_fifo_lock);
+		return -EAGAIN;
+	}
+
 	retval = kfifo_in(&device->tx_fifo, &instance->tx_cfg,
 			  sizeof(instance->tx_cfg));
 	if (retval != sizeof(instance->tx_cfg))
@@ -856,7 +861,6 @@ pi433_write(struct file *filp, const char __user *buf,
 
 abort:
 	dev_dbg(device->dev, "write to fifo failed: 0x%x", retval);
-	kfifo_reset(&device->tx_fifo); // TODO: maybe find a solution, not to discard already stored, valid entries
 	mutex_unlock(&device->tx_fifo_lock);
 	return -EAGAIN;
 }
-- 
2.16.2

  reply	other threads:[~2018-03-24 22:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-23  9:47 [PATCH] staging: pi433: add descriptions for mutex locks Valentin Vidic
2018-03-23 10:22 ` Marcus Wolf
2018-03-23 11:42   ` Valentin Vidic
2018-03-23 15:38     ` Marcus Wolf
2018-03-23 18:00       ` Valentin Vidic
2018-03-23 22:18         ` Valentin Vidic
2018-03-24 22:09           ` Valentin Vidic [this message]
2018-03-25 13:00           ` Marcus Wolf
2018-03-25 13:09             ` Valentin Vidic
2018-03-25 13:12               ` Marcus Wolf
2018-03-25 14:24                 ` Valentin Vidic
2018-04-08 14:15                   ` Marcus Wolf
2018-04-12 17:15                     ` Valentin Vidic
2018-04-19  9:25                       ` Marcus Wolf
2018-04-19  9:47                         ` Valentin Vidic
2018-04-19  9:55                           ` Marcus Wolf
2018-04-08 15:22               ` Marcus Wolf
2018-04-12 18:46                 ` Valentin Vidic
2018-04-19  9:33                   ` Marcus Wolf
2018-04-19 10:25                     ` [PATCH v2] staging: pi433: cleanup tx_fifo locking Valentin Vidic
2018-04-19 10:40                       ` Marcus Wolf

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=20180324220901.28864-1-Valentin.Vidic@CARNet.hr \
    --to=valentin.vidic@carnet.hr \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@Wolf-Entwicklungen.de \
    --cc=luca@acul.me \
    --cc=marcin.s.ciupak@gmail.com \
    --cc=michael.panzlaff@fau.de \
    --cc=robsonde@gmail.com \
    --cc=simon@nikanor.nu \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®