Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work
@ 2025-09-15 15:19 Fiona Ebner
  2025-09-15 15:19 ` [PATCH 1/2] " Fiona Ebner
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Fiona Ebner @ 2025-09-15 15:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: samba-technical, linux-cifs, bharathsm, tom, sprasad,
	ronniesahlberg, pc, sfrench

When io_uring is used in the same task as CIFS, there might be
unnecessary reconnects, causing issues in user-space applications
like QEMU with a log like:

> CIFS: VFS: \\10.10.100.81 Error -512 sending data on socket to server

Certain io_uring completions might be added to task_work with
notify_method being TWA_SIGNAL and thus TIF_NOTIFY_SIGNAL is set for
the task.

In __smb_send_rqst(), signals are masked before calling
smb_send_kvec(), but the masking does not apply to TIF_NOTIFY_SIGNAL.

If sk_stream_wait_memory() is reached via sock_sendmsg() while
TIF_NOTIFY_SIGNAL is set, signal_pending(current) will evaluate to
true there, and -EINTR will be propagated all the way from
sk_stream_wait_memory() to sock_sendmsg() in smb_send_kvec().
Afterwards, __smb_send_rqst() will see that not everything was written
and reconnect.


A reproducer exposing the issue using QEMU:
#!/bin/bash
target=$1
dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=100
qemu-img create -f raw $target 100M
./qemu-system-x86_64 --qmp stdio \
--blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw,file.aio=io_uring \
--blockdev raw,node-name=node1,file.driver=file,file.filename=$target,file.aio=native,file.cache.direct=on \
<<EOF
{"execute": "qmp_capabilities"}
{"execute": "blockdev-mirror", "arguments": { "job-id": "mirror0", "device": "node0", "target": "node1", "sync": "full" } }
EOF

Another reproducer is having a QEMU virtual machine with one disk
using io_uring and one disk on CIFS and doing IO to both disks at the
same time.

I also got a reproducer based on liburing's examples/io_uring-cp.c
which I can send along if you are interested in it.


Fiona Ebner (2):
  smb: client: transport: avoid reconnects triggered by pending task
    work
  smb: client: transport: minor indentation style fix

 fs/smb/client/transport.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.47.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] smb: client: transport: avoid reconnects triggered by pending task work
  2025-09-15 15:19 [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
@ 2025-09-15 15:19 ` Fiona Ebner
  2025-10-02  3:19   ` Steve French
  2025-09-15 15:19 ` [PATCH 2/2] smb: client: transport: minor indentation style fix Fiona Ebner
  2025-09-30 14:08 ` [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
  2 siblings, 1 reply; 5+ messages in thread
From: Fiona Ebner @ 2025-09-15 15:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: samba-technical, linux-cifs, bharathsm, tom, sprasad,
	ronniesahlberg, pc, sfrench

When io_uring is used in the same task as CIFS, there might be
unnecessary reconnects, causing issues in user-space applications
like QEMU with a log like:

> CIFS: VFS: \\10.10.100.81 Error -512 sending data on socket to server

Certain io_uring completions might be added to task_work with
notify_method being TWA_SIGNAL and thus TIF_NOTIFY_SIGNAL is set for
the task.

In __smb_send_rqst(), signals are masked before calling
smb_send_kvec(), but the masking does not apply to TIF_NOTIFY_SIGNAL.

If sk_stream_wait_memory() is reached via sock_sendmsg() while
TIF_NOTIFY_SIGNAL is set, signal_pending(current) will evaluate to
true there, and -EINTR will be propagated all the way from
sk_stream_wait_memory() to sock_sendmsg() in smb_send_kvec().
Afterwards, __smb_send_rqst() will see that not everything was written
and reconnect.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 fs/smb/client/transport.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index a61ba7f3fb86..940e90107134 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -22,6 +22,7 @@
 #include <linux/mempool.h>
 #include <linux/sched/signal.h>
 #include <linux/task_io_accounting_ops.h>
+#include <linux/task_work.h>
 #include "cifspdu.h"
 #include "cifsglob.h"
 #include "cifsproto.h"
@@ -173,9 +174,16 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg,
 		 * send a packet.  In most cases if we fail to send
 		 * after the retries we will kill the socket and
 		 * reconnect which may clear the network problem.
+		 *
+		 * Even if regular signals are masked, EINTR might be
+		 * propagated from sk_stream_wait_memory() to here when
+		 * TIF_NOTIFY_SIGNAL is used for task work. For example,
+		 * certain io_uring completions will use that. Treat
+		 * having EINTR with pending task work the same as EAGAIN
+		 * to avoid unnecessary reconnects.
 		 */
 		rc = sock_sendmsg(ssocket, smb_msg);
-		if (rc == -EAGAIN) {
+		if (rc == -EAGAIN || unlikely(rc == -EINTR && task_work_pending(current))) {
 			retries++;
 			if (retries >= 14 ||
 			    (!server->noblocksnd && (retries > 2))) {
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] smb: client: transport: minor indentation style fix
  2025-09-15 15:19 [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
  2025-09-15 15:19 ` [PATCH 1/2] " Fiona Ebner
@ 2025-09-15 15:19 ` Fiona Ebner
  2025-09-30 14:08 ` [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
  2 siblings, 0 replies; 5+ messages in thread
From: Fiona Ebner @ 2025-09-15 15:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: samba-technical, linux-cifs, bharathsm, tom, sprasad,
	ronniesahlberg, pc, sfrench

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 fs/smb/client/transport.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 940e90107134..051cd9dbba13 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -331,8 +331,7 @@ int __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
 				break;
 			total_len += sent;
 		}
-
-}
+	}
 
 unmask:
 	sigprocmask(SIG_SETMASK, &oldmask, NULL);
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work
  2025-09-15 15:19 [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
  2025-09-15 15:19 ` [PATCH 1/2] " Fiona Ebner
  2025-09-15 15:19 ` [PATCH 2/2] smb: client: transport: minor indentation style fix Fiona Ebner
@ 2025-09-30 14:08 ` Fiona Ebner
  2 siblings, 0 replies; 5+ messages in thread
From: Fiona Ebner @ 2025-09-30 14:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-cifs, bharathsm, tom, sprasad, ronniesahlberg, pc, sfrench

Ping

Am 15.09.25 um 5:19 PM schrieb Fiona Ebner:
> When io_uring is used in the same task as CIFS, there might be
> unnecessary reconnects, causing issues in user-space applications
> like QEMU with a log like:
> 
>> CIFS: VFS: \\10.10.100.81 Error -512 sending data on socket to server
> 
> Certain io_uring completions might be added to task_work with
> notify_method being TWA_SIGNAL and thus TIF_NOTIFY_SIGNAL is set for
> the task.
> 
> In __smb_send_rqst(), signals are masked before calling
> smb_send_kvec(), but the masking does not apply to TIF_NOTIFY_SIGNAL.
> 
> If sk_stream_wait_memory() is reached via sock_sendmsg() while
> TIF_NOTIFY_SIGNAL is set, signal_pending(current) will evaluate to
> true there, and -EINTR will be propagated all the way from
> sk_stream_wait_memory() to sock_sendmsg() in smb_send_kvec().
> Afterwards, __smb_send_rqst() will see that not everything was written
> and reconnect.
> 
> 
> A reproducer exposing the issue using QEMU:
> #!/bin/bash
> target=$1
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=100
> qemu-img create -f raw $target 100M
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw,file.aio=io_uring \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=$target,file.aio=native,file.cache.direct=on \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-mirror", "arguments": { "job-id": "mirror0", "device": "node0", "target": "node1", "sync": "full" } }
> EOF
> 
> Another reproducer is having a QEMU virtual machine with one disk
> using io_uring and one disk on CIFS and doing IO to both disks at the
> same time.
> 
> I also got a reproducer based on liburing's examples/io_uring-cp.c
> which I can send along if you are interested in it.
> 
> 
> Fiona Ebner (2):
>   smb: client: transport: avoid reconnects triggered by pending task
>     work
>   smb: client: transport: minor indentation style fix
> 
>  fs/smb/client/transport.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] smb: client: transport: avoid reconnects triggered by pending task work
  2025-09-15 15:19 ` [PATCH 1/2] " Fiona Ebner
@ 2025-10-02  3:19   ` Steve French
  0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2025-10-02  3:19 UTC (permalink / raw)
  To: Fiona Ebner
  Cc: linux-kernel, samba-technical, linux-cifs, bharathsm, tom,
	sprasad, ronniesahlberg, pc, David Howells

I have tentatively merged these two into cifs-2.6.git for-next pending
testing, but additional review/testing would be helpful.


On Mon, Sep 15, 2025 at 10:23 AM Fiona Ebner <f.ebner@proxmox.com> wrote:
>
> When io_uring is used in the same task as CIFS, there might be
> unnecessary reconnects, causing issues in user-space applications
> like QEMU with a log like:
>
> > CIFS: VFS: \\10.10.100.81 Error -512 sending data on socket to server
>
> Certain io_uring completions might be added to task_work with
> notify_method being TWA_SIGNAL and thus TIF_NOTIFY_SIGNAL is set for
> the task.
>
> In __smb_send_rqst(), signals are masked before calling
> smb_send_kvec(), but the masking does not apply to TIF_NOTIFY_SIGNAL.
>
> If sk_stream_wait_memory() is reached via sock_sendmsg() while
> TIF_NOTIFY_SIGNAL is set, signal_pending(current) will evaluate to
> true there, and -EINTR will be propagated all the way from
> sk_stream_wait_memory() to sock_sendmsg() in smb_send_kvec().
> Afterwards, __smb_send_rqst() will see that not everything was written
> and reconnect.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  fs/smb/client/transport.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
> index a61ba7f3fb86..940e90107134 100644
> --- a/fs/smb/client/transport.c
> +++ b/fs/smb/client/transport.c
> @@ -22,6 +22,7 @@
>  #include <linux/mempool.h>
>  #include <linux/sched/signal.h>
>  #include <linux/task_io_accounting_ops.h>
> +#include <linux/task_work.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
>  #include "cifsproto.h"
> @@ -173,9 +174,16 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg,
>                  * send a packet.  In most cases if we fail to send
>                  * after the retries we will kill the socket and
>                  * reconnect which may clear the network problem.
> +                *
> +                * Even if regular signals are masked, EINTR might be
> +                * propagated from sk_stream_wait_memory() to here when
> +                * TIF_NOTIFY_SIGNAL is used for task work. For example,
> +                * certain io_uring completions will use that. Treat
> +                * having EINTR with pending task work the same as EAGAIN
> +                * to avoid unnecessary reconnects.
>                  */
>                 rc = sock_sendmsg(ssocket, smb_msg);
> -               if (rc == -EAGAIN) {
> +               if (rc == -EAGAIN || unlikely(rc == -EINTR && task_work_pending(current))) {
>                         retries++;
>                         if (retries >= 14 ||
>                             (!server->noblocksnd && (retries > 2))) {
> --
> 2.47.2
>
>
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-02  3:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 15:19 [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner
2025-09-15 15:19 ` [PATCH 1/2] " Fiona Ebner
2025-10-02  3:19   ` Steve French
2025-09-15 15:19 ` [PATCH 2/2] smb: client: transport: minor indentation style fix Fiona Ebner
2025-09-30 14:08 ` [PATCH 0/2] smb: client: transport: avoid reconnects triggered by pending task work Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox