| Message ID | 145da5ab094bcc7d3331385e8813074922c2a13c6.1697486714.git.nabijaczleweli@nabijaczleweli.xyz (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | splice(file<>pipe) I/O on file as-if O_NONBLOCK | expand |
Please add correct tag, for this patch, IIUC, it should be a fix, and you need add [PATCH net]. On Tue, Dec 12, 2023 at 11:12:47AM +0100, Ahelenia Ziemia'nska wrote: > Otherwise we risk sleeping with the pipe locked for indeterminate > lengths of time. > > Link: https://lore.kernel.org/linux-fsdevel/qk6hjuam54khlaikf2ssom6custxf5is2ekkaequf4hvode3ls@zgf7j5j4ubvw/t/#u Fixes line is needed. > Signed-off-by: Ahelenia Ziemia'nska <nabijaczleweli@nabijaczleweli.xyz> > --- > net/smc/af_smc.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index bacdd971615e..89473305f629 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -3243,12 +3243,8 @@ static ssize_t smc_splice_read(struct socket *sock, loff_t *ppos, > rc = -ESPIPE; > goto out; > } > - if (flags & SPLICE_F_NONBLOCK) > - flags = MSG_DONTWAIT; > - else > - flags = 0; > SMC_STAT_INC(smc, splice_cnt); > - rc = smc_rx_recvmsg(smc, NULL, pipe, len, flags); > + rc = smc_rx_recvmsg(smc, NULL, pipe, len, MSG_DONTWAIT); > } > out: > release_sock(sk); > -- > 2.39.2
On Wed, 13 Dec 2023 09:48:47 +0800 Tony Lu wrote: > Please add correct tag, for this patch, IIUC, it should be a fix, and > you need add [PATCH net]. I was wondering who's expected to take this. We (netdev/net maintainers) didn't even get CCed on all the patches in the series. My sense is that this is more of a VFS change, so Al / Christian may be better suited to take this? Let's figure that out before we get another repost.
> Let's figure that out before we get another repost.
I'm just waiting for Jens to review it as he had comments on this
before.
On 12/14/23 3:50 AM, Christian Brauner wrote: >> Let's figure that out before we get another repost. > > I'm just waiting for Jens to review it as he had comments on this > before. Well, I do wish the CC list had been setup a bit more deliberately. Especially as this is a resend, and I didn't even know about any of this before Christian pointed me this way the other day. Checking lore, I can't even see all the patches. So while it may be annoying, I do think it may be a good idea to resend the series so I can take a closer look as well. I do think it's interesting and I'd love to have it work in a non-blocking fashion, both solving the issue of splice holding the pipe lock while doing IO, and also then being able to eliminate the pipe_clear_nowait() hack hopefully.
On Thu, 14 Dec 2023 09:57:32 -0700 Jens Axboe wrote: > On 12/14/23 3:50 AM, Christian Brauner wrote: > >> Let's figure that out before we get another repost. > > > > I'm just waiting for Jens to review it as he had comments on this > > before. > > Well, I do wish the CC list had been setup a bit more deliberately. > Especially as this is a resend, and I didn't even know about any of this > before Christian pointed me this way the other day. > > Checking lore, I can't even see all the patches. So while it may be > annoying, I do think it may be a good idea to resend the series so I can > take a closer look as well. So to summarize - for the repost please make sure to CC Jens, Christian, Al, linux-fsdevel@vger.kernel.org on *all* patches. No need to add "net" to subject prefix, or CC net on all. > I do think it's interesting and I'd love to > have it work in a non-blocking fashion, both solving the issue of splice > holding the pipe lock while doing IO, and also then being able to > eliminate the pipe_clear_nowait() hack hopefully.
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index bacdd971615e..89473305f629 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -3243,12 +3243,8 @@ static ssize_t smc_splice_read(struct socket *sock, loff_t *ppos, rc = -ESPIPE; goto out; } - if (flags & SPLICE_F_NONBLOCK) - flags = MSG_DONTWAIT; - else - flags = 0; SMC_STAT_INC(smc, splice_cnt); - rc = smc_rx_recvmsg(smc, NULL, pipe, len, flags); + rc = smc_rx_recvmsg(smc, NULL, pipe, len, MSG_DONTWAIT); } out: release_sock(sk);
Otherwise we risk sleeping with the pipe locked for indeterminate lengths of time. Link: https://lore.kernel.org/linux-fsdevel/qk6hjuam54khlaikf2ssom6custxf5is2ekkaequf4hvode3ls@zgf7j5j4ubvw/t/#u Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> --- net/smc/af_smc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)