[PATCH 0/8] char: xillybus: Fix issues reported by Sashiko

[PATCH 0/8] char: xillybus: Fix issues reported by Sashiko

[Eli Billauer]

This series is a follow-up to the previously submitted patchset [1].
It addresses Sashiko review comments on surrounding code.

No functional or user-visible change is expected.

The issues addressed here originate from the initial submission of each
driver (Xillybus first, and XillyUSB several years later).

Regards,
  Eli

[1] https://lore.kernel.org/all/20261009074557.19996-1-eli.billauer@gmail.com/

Eli Billauer (8):
  char: xillybus: Use unsigned int literal
  char: xillybus: Safer access of unaligned pointers
  char: xillybus: Return error in fops read method handler
  char: xillybus: Rely on FMODE flags in poll method
  char: xillybus: Refactor flush function
  char: xillybus: Correct handling of wait queue
  char: xillybus: Avoid indefinite sleep of work item
  char: xillybus: Cancel delayed work items before teardown

 drivers/char/xillybus/xillybus.h      |   3 +
 drivers/char/xillybus/xillybus_core.c | 109 ++++++++++++-----
 drivers/char/xillybus/xillyusb.c      | 170 ++++++++++++--------------
 3 files changed, 159 insertions(+), 123 deletions(-)

-- 
2.17.1

[PATCH 1/8] char: xillybus: Use unsigned int literal

[Eli Billauer]

Use "1U" rather than "1" left-shift operations in order to avoid signed
integer arithmetic, which has an undefined result if it wraps.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260805093436.59740-1-eli.billauer%40gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index bd3babd39f35..56afb0a577af 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -381,13 +381,13 @@ static int fifo_init(struct xillyfifo *fifo,
 
 	if (log2_size > log2_fifo_buf_size) {
 		log2_bufnum = log2_size - log2_fifo_buf_size;
-		fifo->bufsize = 1 << log2_fifo_buf_size;
+		fifo->bufsize = 1U << log2_fifo_buf_size;
 	} else {
 		log2_bufnum = 0;
-		fifo->bufsize = 1 << log2_size;
+		fifo->bufsize = 1U << log2_size;
 	}
 
-	fifo->bufnum = 1 << log2_bufnum;
+	fifo->bufnum = 1U << log2_bufnum;
 	fifo->size = fifo->bufnum * fifo->bufsize;
 
 	if (!fifo->size || /* Unsigned integer overflow */
@@ -516,7 +516,7 @@ static struct xillyusb_endpoint
 	init_usb_anchor(&ep->anchor);
 	INIT_WORK(&ep->workitem, work);
 
-	ep->buffer_size = 1 << (PAGE_SHIFT + order);
+	ep->buffer_size = 1U << (PAGE_SHIFT + order);
 	ep->outstanding_urbs = 0;
 	ep->drained = true;
 	ep->wake_on_drain = false;
@@ -1320,7 +1320,7 @@ static int xillyusb_open(struct inode *inode, struct file *filp)
 		if (rc)
 			goto late_unopen;
 
-		out_ep->fill_mask = -(1 << chan->out_log2_element_size);
+		out_ep->fill_mask = -(1U << chan->out_log2_element_size);
 		chan->out_bytes = 0;
 		chan->flushed = 0;
 
-- 
2.17.1

[PATCH 2/8] char: xillybus: Safer access of unaligned pointers

[Eli Billauer]

Use get_unaligned_le16() rather than dereferencing unaligned pointers
directly for portability.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260805093436.59740-1-eli.billauer%40gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 56afb0a577af..4c53d808737a 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <asm/byteorder.h>
+#include <linux/unaligned.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
@@ -1960,8 +1961,8 @@ static int setup_channels(struct xillyusb_dev *xdev,
 	new_channels = chan;
 
 	for (i = 0; i < num_channels; i++, chan++) {
-		unsigned int in_desc = le16_to_cpu(*chandesc++);
-		unsigned int out_desc = le16_to_cpu(*chandesc++);
+		unsigned int in_desc = get_unaligned_le16(chandesc++);
+		unsigned int out_desc = get_unaligned_le16(chandesc++);
 
 		chan->xdev = xdev;
 		mutex_init(&chan->in_mutex);
@@ -2108,7 +2109,7 @@ static int xillyusb_discovery(struct usb_interface *interface)
 
 	/* Phase II: Set up the streams as defined in IDT */
 
-	num_channels = le16_to_cpu(*((__le16 *)(idt + 1)));
+	num_channels = get_unaligned_le16(idt + 1);
 	names_offset = 3 + num_channels * 4;
 	idt_len -= 4; /* Exclude CRC */
 
-- 
2.17.1

[PATCH 3/8] char: xillybus: Return error in fops read method handler

[Eli Billauer]

Be sure to return an error status from the driver's read fops method when
the USB device is unplugged or a hardware error occurs, and all available
data has been delivered.

Before this patch, the return value was zero in this case, meaning EOF,
which didn't expose the reason for ending the data stream.

The existing memory barrier smp_rmb(), together with the assignment of
xdev->error in report_io_error() with a spinlock taken, ensure that
xdev->error has its updated value when the hereby corrected assignment
takes place.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260724094302.50761-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 4c53d808737a..45612fd760f0 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -1544,7 +1544,7 @@ static ssize_t xillyusb_read(struct file *filp, char __user *userbuf,
 			if (READ_ONCE(fifo->fill))
 				continue;
 
-			rc = 0;
+			rc = xdev->error; /* Usually zero, meaning EOF */
 			break;
 		}
 
-- 
2.17.1

[PATCH 4/8] char: xillybus: Rely on FMODE flags in poll method

[Eli Billauer]

The poll method of xillyusb.c previously tested chan->in_fifo and
chan->out_ep for being NULL in order to determine if the file is opened
for read and/or write, respectively. This works incorrectly if two files
are opened on the same device file, one for read and one for write.

In particular, if the release method is called for the file opened for
read while a poll is called for the file opened for write (and vice
versa), a race condition could lead to access of freed memory.

Relying on the FMODE flags as a guarantee for the validity of
chan->in_fifo and chan->out_ep as pointers to allocated memory is safe,
as the allocation and deallocation occurs with the open and release
methods for each file.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260724094302.50761-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 45612fd760f0..73e26f254693 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -1856,10 +1856,13 @@ static __poll_t xillyusb_poll(struct file *filp, poll_table *wait)
 	struct xillyusb_channel *chan = filp->private_data;
 	__poll_t mask = 0;
 
-	if (chan->in_fifo)
+	bool open_for_read = !!(filp->f_mode & FMODE_READ);
+	bool open_for_write = !!(filp->f_mode & FMODE_WRITE);
+
+	if (open_for_read)
 		poll_wait(filp, &chan->in_fifo->waitq, wait);
 
-	if (chan->out_ep)
+	if (open_for_write)
 		poll_wait(filp, &chan->out_ep->fifo.waitq, wait);
 
 	/*
@@ -1872,7 +1875,7 @@ static __poll_t xillyusb_poll(struct file *filp, poll_table *wait)
 	 * wait queue.
 	 */
 
-	if (!chan->poll_used && chan->in_fifo) {
+	if (!chan->poll_used && open_for_read) {
 		chan->poll_used = 1;
 		request_read_anything(chan, OPCODE_SET_PUSH);
 	}
@@ -1884,11 +1887,11 @@ static __poll_t xillyusb_poll(struct file *filp, poll_table *wait)
 	 * which it may not. So make it never work.
 	 */
 
-	if (chan->in_fifo && !chan->in_synchronous &&
+	if (open_for_read && !chan->in_synchronous &&
 	    (READ_ONCE(chan->in_fifo->fill) || !chan->read_data_ok))
 		mask |= EPOLLIN | EPOLLRDNORM;
 
-	if (chan->out_ep &&
+	if (open_for_write &&
 	    (READ_ONCE(chan->out_ep->fifo.fill) != chan->out_ep->fifo.size))
 		mask |= EPOLLOUT | EPOLLWRNORM;
 
-- 
2.17.1

[PATCH 5/8] char: xillybus: Refactor flush function

[Eli Billauer]

Previous flush_downstream() looped on wait_event_interruptible() and
wait_event_interruptible_timeout() without returning if an interrupt
occurred. Replace these with non-interruptible counterparts and use
wake_up() instead of wake_up_interruptible() on the relevant wait queue.

In addition, flush_downstream()'s @interruptible argument is removed.
Instead, this function either waits with a timeout only, or with an
interruptible wait. This is what the callers of this function requested
anyhow.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260724094302.50761-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 89 +++++++++++++++-----------------
 1 file changed, 41 insertions(+), 48 deletions(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 73e26f254693..3afd9bd38a1a 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -604,7 +604,7 @@ static void wakeup_all(struct work_struct *work)
 
 		mutex_unlock(&chan->lock);
 
-		wake_up_interruptible(&chan->flushq);
+		wake_up(&chan->flushq);
 	}
 
 	mutex_unlock(&xdev->process_in_mutex);
@@ -923,12 +923,12 @@ static int process_in_opcode(struct xillyusb_dev *xdev,
 
 	case OPCODE_REACHED_CHECKPOINT:
 		chan->flushing = 0;
-		wake_up_interruptible(&chan->flushq);
+		wake_up(&chan->flushq);
 		break;
 
 	case OPCODE_CANCELED_CHECKPOINT:
 		chan->canceled = 1;
-		wake_up_interruptible(&chan->flushq);
+		wake_up(&chan->flushq);
 		break;
 
 	default:
@@ -1130,11 +1130,13 @@ static int xillyusb_send_opcode(struct xillyusb_dev *xdev,
  * chan->flushed is there to avoid multiple flushes at the same position,
  * in particular as a result of programs that close the file descriptor
  * e.g. after a dup2() for redirection.
+ *
+ * If timeout == 0, no timeout is applied and the wait is interruptible,
+ * otherwise a timeout is applied but the wait is non-interruptible.
  */
 
 static int flush_downstream(struct xillyusb_channel *chan,
-			    unsigned long timeout,
-			    bool interruptible)
+			    unsigned long timeout)
 {
 	struct xillyusb_dev *xdev = chan->xdev;
 	int chan_num = chan->chan_idx << 1;
@@ -1159,21 +1161,22 @@ static int flush_downstream(struct xillyusb_channel *chan,
 			return rc; /* Only real error, never -EINTR */
 
 		/* Ignoring interrupts. Cancellation must be handled */
-		while (!chan->canceled) {
-			left_to_sleep = cancel_deadline - jiffies;
+		left_to_sleep = cancel_deadline - jiffies;
 
-			if (left_to_sleep <= 0) {
-				report_io_error(xdev, -EIO);
-				return -EIO;
-			}
+		rc = 0;
 
-			rc = wait_event_interruptible_timeout(chan->flushq,
-							      chan->canceled ||
-							      xdev->error,
-							      left_to_sleep);
+		if (left_to_sleep > 0)
+			rc = wait_event_timeout(chan->flushq,
+						chan->canceled ||
+						xdev->error,
+						left_to_sleep);
+
+		if (xdev->error)
+			return xdev->error;
 
-			if (xdev->error)
-				return xdev->error;
+		if (!rc) {
+			report_io_error(xdev, -EIO);
+			return -EIO;
 		}
 	}
 
@@ -1194,40 +1197,30 @@ static int flush_downstream(struct xillyusb_channel *chan,
 	if (rc)
 		return rc; /* Only real error, never -EINTR */
 
-	if (!timeout) {
-		while (chan->flushing) {
-			rc = wait_event_interruptible(chan->flushq,
-						      !chan->flushing ||
-						      xdev->error);
-			if (xdev->error)
-				return xdev->error;
-
-			if (interruptible && rc)
-				return -EINTR;
-		}
+	if (timeout) {
+		left_to_sleep = deadline - jiffies;
 
-		goto done;
-	}
+		rc = 0;
 
-	while (chan->flushing) {
-		left_to_sleep = deadline - jiffies;
+		if (left_to_sleep > 0)
+			rc = wait_event_timeout(chan->flushq,
+						!chan->flushing ||
+						xdev->error,
+						left_to_sleep);
 
-		if (left_to_sleep <= 0)
+		if (!rc)
 			return -ETIMEDOUT;
+	} else {
+		rc = wait_event_interruptible(chan->flushq,
+					      !chan->flushing || xdev->error);
+	}
 
-		rc = wait_event_interruptible_timeout(chan->flushq,
-						      !chan->flushing ||
-						      xdev->error,
-						      left_to_sleep);
-
-		if (xdev->error)
-			return xdev->error;
+	if (xdev->error)
+		return xdev->error;
 
-		if (interruptible && rc < 0)
-			return -EINTR;
-	}
+	if (rc < 0)
+		return -EINTR;
 
-done:
 	chan->flushed = 1;
 	return 0;
 }
@@ -1335,7 +1328,7 @@ static int xillyusb_open(struct inode *inode, struct file *filp)
 		 * This can't go wrong unless there's a serious hardware error
 		 * (or the computer is stuck for 500 ms?)
 		 */
-		rc = flush_downstream(chan, XILLY_RESPONSE_TIMEOUT, false);
+		rc = flush_downstream(chan, XILLY_RESPONSE_TIMEOUT);
 
 		if (rc == -ETIMEDOUT) {
 			rc = -EIO;
@@ -1629,7 +1622,7 @@ static int xillyusb_flush(struct file *filp, fl_owner_t id)
 	 * the user pressed CTRL-C, that interrupt will still be in flight by
 	 * the time we reach here, and the opportunity to flush is lost.
 	 */
-	rc = flush_downstream(chan, HZ, false);
+	rc = flush_downstream(chan, HZ);
 
 	mutex_unlock(&chan->out_mutex);
 
@@ -1699,7 +1692,7 @@ static ssize_t xillyusb_write(struct file *filp, const char __user *userbuf,
 	}
 
 	if (chan->out_synchronous) {
-		int flush_rc = flush_downstream(chan, 0, true);
+		int flush_rc = flush_downstream(chan, 0);
 
 		if (flush_rc && !rc)
 			rc = flush_rc;
@@ -1836,7 +1829,7 @@ static loff_t xillyusb_llseek(struct file *filp, loff_t offset, int whence)
 
 	if (chan->writable) {
 		chan->flushed = 0;
-		rc = flush_downstream(chan, HZ, false);
+		rc = flush_downstream(chan, HZ);
 	}
 
 end:
-- 
2.17.1

[PATCH 6/8] char: xillybus: Correct handling of wait queue

[Eli Billauer]

Don't use wait_event_interruptible() in an infinite while loop, and
replace with a non-interruptible timed-out wait instead. Use wake_up()
instead of wake_up_interruptible() to match that change.

Also, use the correct form of wait function in the disconnect method.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260724094302.50761-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 49 ++++++++++++--------------------
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 3afd9bd38a1a..09c7b0ecab59 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -40,6 +40,7 @@ MODULE_LICENSE("GPL v2");
 
 #define XILLY_RX_TIMEOUT		(10 * HZ / 1000)
 #define XILLY_RESPONSE_TIMEOUT		(500 * HZ / 1000)
+#define XILLY_ETERNITY_TIMEOUT		(10 * HZ)
 
 #define BUF_SIZE_ORDER			4
 #define BUFNUM				8
@@ -596,11 +597,11 @@ static void wakeup_all(struct work_struct *work)
 			 * processed.
 			 */
 			chan->read_data_ok = 0;
-			wake_up_interruptible(&chan->in_fifo->waitq);
+			wake_up(&chan->in_fifo->waitq);
 		}
 
 		if (chan->out_ep)
-			wake_up_interruptible(&chan->out_ep->fifo.waitq);
+			wake_up(&chan->out_ep->fifo.waitq);
 
 		mutex_unlock(&chan->lock);
 
@@ -609,7 +610,7 @@ static void wakeup_all(struct work_struct *work)
 
 	mutex_unlock(&xdev->process_in_mutex);
 
-	wake_up_interruptible(&xdev->msg_ep->fifo.waitq);
+	wake_up(&xdev->msg_ep->fifo.waitq);
 
 	kref_put(&xdev->kref, cleanup_dev);
 }
@@ -872,7 +873,7 @@ static void try_queue_bulk_out(struct xillyusb_endpoint *ep)
 	mutex_unlock(&ep->ep_mutex);
 
 	if (do_wake)
-		wake_up_interruptible(&fifo->waitq);
+		wake_up(&fifo->waitq);
 }
 
 static void bulk_out_work(struct work_struct *work)
@@ -918,7 +919,7 @@ static int process_in_opcode(struct xillyusb_dev *xdev,
 
 		in_fifo = READ_ONCE(chan->in_fifo);
 		if (in_fifo)
-			wake_up_interruptible(&in_fifo->waitq);
+			wake_up(&in_fifo->waitq);
 		break;
 
 	case OPCODE_REACHED_CHECKPOINT:
@@ -1018,7 +1019,7 @@ static int process_bulk_in(struct xillybuffer *xb)
 			return -EIO;
 		}
 
-		wake_up_interruptible(&fifo->waitq);
+		wake_up(&fifo->waitq);
 
 		dwconsume = (bytes + 3) >> 2;
 		dws -= dwconsume;
@@ -1089,18 +1090,10 @@ static int xillyusb_send_opcode(struct xillyusb_dev *xdev,
 
 	mutex_lock(&xdev->msg_mutex);
 
-	/*
-	 * The wait queue is woken with the interruptible variant, so the
-	 * wait function matches, however returning because of an interrupt
-	 * will mess things up considerably, in particular when the caller is
-	 * the release method. And the xdev->error part prevents being stuck
-	 * forever in the event of a bizarre hardware bug: Pull the USB plug.
-	 */
-
-	while (wait_event_interruptible(fifo->waitq,
-					fifo->fill <= (fifo->size - 8) ||
-					xdev->error))
-		; /* Empty loop */
+	if (!wait_event_timeout(fifo->waitq,
+				fifo->fill <= (fifo->size - 8) ||
+				xdev->error, XILLY_ETERNITY_TIMEOUT))
+		report_io_error(xdev, -EIO);
 
 	if (xdev->error) {
 		rc = xdev->error;
@@ -1719,18 +1712,12 @@ static int xillyusb_release(struct inode *inode, struct file *filp)
 		 * If rc_read is nonzero, xdev->error indicates a global
 		 * device error. The error is reported later, so that
 		 * resources are freed.
-		 *
-		 * Looping on wait_event_interruptible() kinda breaks the idea
-		 * of being interruptible, and this should have been
-		 * wait_event(). Only it's being waken with
-		 * wake_up_interruptible() for the sake of other uses. If
-		 * there's a global device error, chan->read_data_ok is
-		 * deasserted and the wait queue is awaken, so this is covered.
 		 */
 
-		while (wait_event_interruptible(in_fifo->waitq,
-						!chan->read_data_ok))
-			; /* Empty loop */
+		if (!wait_event_timeout(in_fifo->waitq,
+					!chan->read_data_ok,
+					XILLY_ETERNITY_TIMEOUT))
+			report_io_error(xdev, -EIO);
 
 		safely_assign_in_fifo(chan, NULL);
 		fifo_mem_release(in_fifo);
@@ -2240,9 +2227,9 @@ static void xillyusb_disconnect(struct usb_interface *interface)
 	 * but somehow the message wasn't sent. Should never happen.
 	 */
 
-	rc = wait_event_interruptible_timeout(fifo->waitq,
-					      msg_ep->drained || xdev->error,
-					      XILLY_RESPONSE_TIMEOUT);
+	rc = wait_event_timeout(fifo->waitq,
+				msg_ep->drained || xdev->error,
+				XILLY_RESPONSE_TIMEOUT);
 
 	if (!rc)
 		dev_err(&interface->dev,
-- 
2.17.1

[PATCH 7/8] char: xillybus: Avoid indefinite sleep of work item

[Eli Billauer]

xillybus_write() may sleep indefinitely with rd_mutex taken while
waiting for a DMA buffer to become available for writing. As the work
item implementing automatic flush also takes this mutex, it may sleep
indefinitely as well.

Thanks to the Concurrency Managed Workqueue (cmwq), there has been no
visible problem with work items sleeping forever, but this shouldn't be
allowed by design.

This commit makes the autoflush work item return immediately if
xillybus_write() is invoked in parallel, and record that fact with the
help of a new struct member, autoflush_skipped. This way, the work item
doesn't attempt to take rd_mutex, avoiding sleeping indefinitely.
When an autoflush has been skipped this way, xillybus_write() compensates
for this with a direct call to xillybus_myflush().

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260724094302.50761-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillybus.h      |  3 +
 drivers/char/xillybus/xillybus_core.c | 88 +++++++++++++++++++++------
 2 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/drivers/char/xillybus/xillybus.h b/drivers/char/xillybus/xillybus.h
index 98c7ac4dd1f9..cd3cde833ce3 100644
--- a/drivers/char/xillybus/xillybus.h
+++ b/drivers/char/xillybus/xillybus.h
@@ -77,6 +77,9 @@ struct xilly_channel {
 	int rd_full;
 	spinlock_t rd_spinlock;
 	struct mutex rd_mutex;
+	struct mutex autoflush_mutex;
+	int dont_autoflush;
+	bool autoflush_skipped;
 	wait_queue_head_t rd_wait;
 	int rd_ref_count;
 	int rd_allow_partial;
diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index 7189c6d97e7d..ab8ac78b28dc 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -84,10 +84,11 @@ static struct workqueue_struct *xillybus_wq;
  * Blocking on the wait queues is allowed with mutexes held, but not with
  * spinlocks.
  *
- * Only interruptible blocking is allowed on mutexes and wait queues.
+ * Only interruptible blocking is allowed on mutexes and wait queues,
+ * except for autoflush_mutex.
  *
  * All in all, the locking order goes (with skips allowed, of course):
- * wr_mutex -> rd_mutex -> register_mutex ->
+ * autoflush_mutex -> wr_mutex -> rd_mutex -> register_mutex ->
  * allow_isr_lock -> wr_spinlock -> rd_spinlock
  */
 
@@ -522,6 +523,9 @@ static int xilly_setupchannels(struct xilly_endpoint *ep,
 		spin_lock_init(&channel->rd_spinlock);
 		mutex_init(&channel->wr_mutex);
 		mutex_init(&channel->rd_mutex);
+		mutex_init(&channel->autoflush_mutex);
+		channel->dont_autoflush = 0;
+		channel->autoflush_skipped = false;
 		init_waitqueue_head(&channel->rd_wait);
 		init_waitqueue_head(&channel->wr_wait);
 		init_waitqueue_head(&channel->wr_ready_wait);
@@ -1194,7 +1198,10 @@ static int xillybus_myflush(struct xilly_channel *channel, long timeout)
 		/*
 		 * Indefinite sleep with mutex taken. With data waiting for
 		 * flushing user should not be surprised if open() for write
-		 * sleeps.
+		 * sleeps. Note that timeout == 0 is used only with
+		 * synchronous channels (with rd_synchronous true), and
+		 * the autoflush work item is never queued for such, so
+		 * no risk of work item sleeping indefinitely.
 		 */
 		if (timeout == 0)
 			wait_event_interruptible(channel->rd_wait,
@@ -1241,6 +1248,13 @@ static void xillybus_autoflush(struct work_struct *work)
 		workitem, struct xilly_channel, rd_workitem);
 	int rc;
 
+	guard(mutex)(&channel->autoflush_mutex);
+
+	if (channel->dont_autoflush) {
+		channel->autoflush_skipped = true;
+		return;
+	}
+
 	rc = xillybus_myflush(channel, -1);
 	if (rc == -EINTR)
 		dev_warn(channel->endpoint->dev,
@@ -1250,8 +1264,8 @@ static void xillybus_autoflush(struct work_struct *work)
 			"Autoflush failed under weird circumstances.\n");
 }
 
-static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
-			      size_t count, loff_t *f_pos)
+static ssize_t __xillybus_write(struct file *filp, const char __user *userbuf,
+				size_t count)
 {
 	ssize_t rc;
 	unsigned long flags;
@@ -1264,9 +1278,6 @@ static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
 	int howmany = 0, bufpos = 0, bufidx = 0, bufferdone = 0;
 	int end_offset_plus1 = 0;
 
-	if (channel->endpoint->fatal_error)
-		return -EIO;
-
 	rc = mutex_lock_interruptible(&channel->rd_mutex);
 	if (rc)
 		return rc;
@@ -1409,12 +1420,6 @@ static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
 				if (channel->endpoint->fatal_error)
 					return -EIO;
 
-				if (!channel->rd_synchronous)
-					queue_delayed_work(
-						xillybus_wq,
-						&channel->rd_workitem,
-						XILLY_RX_TIMEOUT);
-
 				return rc;
 			}
 		}
@@ -1454,11 +1459,6 @@ static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
 
 	mutex_unlock(&channel->rd_mutex);
 
-	if (!channel->rd_synchronous)
-		queue_delayed_work(xillybus_wq,
-				   &channel->rd_workitem,
-				   XILLY_RX_TIMEOUT);
-
 	if (channel->endpoint->fatal_error)
 		return -EIO;
 
@@ -1475,6 +1475,56 @@ static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
 	return bytes_done;
 }
 
+/*
+ * Prevent the autoflush work item from sleeping indefinitely by having
+ * dont_autoflush non-zero while __xillybus_write() is invoked, as this
+ * function may sleep indefinitely with rd_mutex taken.
+ * The work item terminates immediately when dont_autoflush is non-zero,
+ * and announces this by setting autoflush_skipped. Perform autoflush before
+ * returning to compensate for a skipped autoflush if necessary.
+ */
+
+static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
+			      size_t count, loff_t *f_pos)
+{
+	struct xilly_channel *channel = filp->private_data;
+
+	ssize_t retval;
+	bool compensate = false;
+
+	if (channel->endpoint->fatal_error)
+		return -EIO;
+
+	/*
+	 * On synchronous channels, flush is performed after every
+	 * write operation. Autoflush is therefore irrelevant.
+	 */
+	if (channel->rd_synchronous)
+		return __xillybus_write(filp, userbuf, count);
+
+	scoped_guard(mutex, &channel->autoflush_mutex)
+		channel->dont_autoflush++;
+
+	retval = __xillybus_write(filp, userbuf, count);
+
+	scoped_guard(mutex, &channel->autoflush_mutex) {
+		channel->dont_autoflush--;
+
+		if (channel->autoflush_skipped) {
+			compensate = true;
+			channel->autoflush_skipped = false;
+		}
+	}
+
+	if (compensate)
+		xillybus_myflush(channel, -1);
+
+	queue_delayed_work(xillybus_wq, &channel->rd_workitem,
+			   XILLY_RX_TIMEOUT);
+
+	return retval;
+}
+
 static int xillybus_open(struct inode *inode, struct file *filp)
 {
 	int rc;
-- 
2.17.1

[PATCH 8/8] char: xillybus: Cancel delayed work items before teardown

[Eli Billauer]

xillybus_core queues only delayed work items in its workqueue. Use
cancel_delayed_work_sync() instead of flush_workqueue() to clean up
queued work items, as the latter does not wait for or cancel delayed
work items whose timers have not yet expired.

Assisted-by: Sashiko-0.2.5:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260630092330.43534-1-eli.billauer@gmail.com
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillybus_core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index ab8ac78b28dc..644447502d09 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -1926,9 +1926,10 @@ struct xilly_endpoint *xillybus_init_endpoint(struct device *dev)
 }
 EXPORT_SYMBOL(xillybus_init_endpoint);
 
-static int xilly_quiesce(struct xilly_endpoint *endpoint)
+static int xilly_quiesce(struct xilly_endpoint *endpoint, int to_cancel)
 {
 	long t;
+	int i;
 
 	endpoint->idtlen = -1;
 	endpoint->num_channels = 0;
@@ -1944,6 +1945,9 @@ static int xilly_quiesce(struct xilly_endpoint *endpoint)
 
 	xilly_allow_isr(endpoint, false);
 
+	for (i = 1; i <= to_cancel; i++)
+		cancel_delayed_work_sync(&endpoint->channels[i]->rd_workitem);
+
 	if (t <= 0) {
 		dev_err(endpoint->dev,
 			"Failed to quiesce the device on exit.\n");
@@ -1956,6 +1960,7 @@ int xillybus_endpoint_discovery(struct xilly_endpoint *endpoint)
 {
 	int rc;
 	long t;
+	int quiesce_channels = 0;
 
 	void *bootstrap_resources;
 	int idtbuffersize = (1 << PAGE_SHIFT);
@@ -2058,6 +2063,7 @@ int xillybus_endpoint_discovery(struct xilly_endpoint *endpoint)
 		goto failed_idt;
 
 	xilly_allow_isr(endpoint, true);
+	quiesce_channels = endpoint->num_channels;
 
 	rc = xillybus_init_chrdev(dev, &xillybus_fops,
 				  endpoint->owner, endpoint,
@@ -2074,8 +2080,7 @@ int xillybus_endpoint_discovery(struct xilly_endpoint *endpoint)
 	return 0;
 
 failed_idt:
-	xilly_quiesce(endpoint);
-	flush_workqueue(xillybus_wq);
+	xilly_quiesce(endpoint, quiesce_channels);
 
 	return rc;
 }
@@ -2085,13 +2090,7 @@ void xillybus_endpoint_remove(struct xilly_endpoint *endpoint)
 {
 	xillybus_cleanup_chrdev(endpoint, endpoint->dev);
 
-	xilly_quiesce(endpoint);
-
-	/*
-	 * Flushing is done upon endpoint release to prevent access to memory
-	 * just about to be released. This makes the quiesce complete.
-	 */
-	flush_workqueue(xillybus_wq);
+	xilly_quiesce(endpoint, endpoint->num_channels);
 }
 EXPORT_SYMBOL(xillybus_endpoint_remove);
 
@@ -2106,7 +2105,7 @@ static int __init xillybus_init(void)
 
 static void __exit xillybus_exit(void)
 {
-	/* flush_workqueue() was called for each endpoint released */
+	/* Work items have been canceled for each endpoint separately */
 	destroy_workqueue(xillybus_wq);
 }
 
-- 
2.17.1