On Tue, Sep 26, 2023 at 12:41:42PM +0200, Max Kellermann wrote: > On Tue, Sep 26, 2023 at 12:21 PM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > Hm, so the thing that is worrysome about this change is that this may > > cause regressions afaict as this is a pretty significant change from > > current behavior. > > Would you prefer a new flag for explicitly selecting "wait until at > least one byte was transferred, but don't wait further"? Because many I had thought about it but afaict it'd be rather annoying as one can get into that code from copy_file_range() as well so we'd need a new flag for that system call as well afaict. > applications need this behavior, and some (like nginx) have already > worked around the problem by limiting the maximum transaction size, > which I consider a bad workaround, because it leads to unnecessary > system calls and still doesn't really solve the latency problem. > > On the other hand, what exactly would the absence of this flag mean... > the old behavior, without my patch, can lead to partial transfers, and > the absence of the flag doesn't mean it can't happen; my patch tackles > just one corner case, but one that is important for me. > > We have been running this patch in production for nearly a year (and > will continue to do so until upstream kernels have a proper solution) > and never observed a problem, and I consider it safe, but I > acknowledge the risk that this may reveal obscure application bugs if > applied globally to all Linux kernels, so I understand your worries. I think hanging for an insane amount of time is indeed a problem and tweaking the code in this way might actually be useful but we'd need to let this soak for quite a while to see whether this causes any issues. @Jens, what do you think? Is this worth it?
