generic_file_splice_read() goes into an infinite loop if it races with
truncation. I've found this with fsx-linux on NFS over fuse.
Perhaps the whole while() loop is bogus, but I can't tell from a
cursory glance at __generic_file_splice_read() if it will return zero
only on EOF, or it can do that for other reasons as well. In the
latter case the loop is obviously needed.
This simplistic patch fixes the issue for me.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/splice.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
Index: linux/fs/splice.c
===================================================================
--- linux.orig/fs/splice.c 2008-04-02 13:34:58.000000000 +0200
+++ linux/fs/splice.c 2008-04-09 17:35:06.000000000 +0200
@@ -481,19 +481,20 @@ ssize_t generic_file_splice_read(struct
{
ssize_t spliced;
int ret;
- loff_t isize, left;
-
- isize = i_size_read(in->f_mapping->host);
- if (unlikely(*ppos >= isize))
- return 0;
-
- left = isize - *ppos;
- if (unlikely(left < len))
- len = left;
ret = 0;
spliced = 0;
while (len && !spliced) {
+ loff_t isize, left;
+
+ isize = i_size_read(in->f_mapping->host);
+ if (unlikely(*ppos >= isize))
+ return 0;
+
+ left = isize - *ppos;
+ if (unlikely(left < len))
+ len = left;
+
ret = __generic_file_splice_read(in, ppos, pipe, len, flags);
if (ret < 0)
--
SGVsbPMgTWlrbPNzIQoKZXogYSBwYXRjaCBzevxrc+lnZXMgYSAyLjYuMjIueSBrZXJuZWxoZXo/ ICgyLjYuMjIueS1iYSBlbHP1IHLhbul66XNyZQpodW5rIG7pbGv8bCBob3p64WFkaGF08ykKdmFn eSB24XJqYW0gbWVnLCBhbWlnIGJla2Vy/GwgYSBtYWlubGluZSBrZXJuZWxiZT8KCk9uIDQvOS8w OCwgTWlrbG9zIFN6ZXJlZGkgPG1pa2xvc0BzemVyZWRpLmh1PiB3cm90ZToKPiBnZW5lcmljX2Zp bGVfc3BsaWNlX3JlYWQoKSBnb2VzIGludG8gYW4gaW5maW5pdGUgbG9vcCBpZiBpdCByYWNlcyB3 aXRoCj4gdHJ1bmNhdGlvbi4gIEkndmUgZm91bmQgdGhpcyB3aXRoIGZzeC1saW51eCBvbiBORlMg b3ZlciBmdXNlLgo+Cj4gUGVyaGFwcyB0aGUgd2hvbGUgd2hpbGUoKSBsb29wIGlzIGJvZ3VzLCBi dXQgSSBjYW4ndCB0ZWxsIGZyb20gYQo+IGN1cnNvcnkgZ2xhbmNlIGF0IF9fZ2VuZXJpY19maWxl X3NwbGljZV9yZWFkKCkgaWYgaXQgd2lsbCByZXR1cm4gemVybwo+IG9ubHkgb24gRU9GLCBvciBp dCBjYW4gZG8gdGhhdCBmb3Igb3RoZXIgcmVhc29ucyBhcyB3ZWxsLiAgSW4gdGhlCj4gbGF0dGVy IGNhc2UgdGhlIGxvb3AgaXMgb2J2aW91c2x5IG5lZWRlZC4KPgo+IFRoaXMgc2ltcGxpc3RpYyBw YXRjaCBmaXhlcyB0aGUgaXNzdWUgZm9yIG1lLgo+Cj4gU2lnbmVkLW9mZi1ieTogTWlrbG9zIFN6 ZXJlZGkgPG1zemVyZWRpQHN1c2UuY3o+Cj4gLS0tCj4gIGZzL3NwbGljZS5jIHwgICAxOSArKysr KysrKysrLS0tLS0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspLCA5IGRl bGV0aW9ucygtKQo+Cj4gSW5kZXg6IGxpbnV4L2ZzL3NwbGljZS5jCj4gPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQo+IC0t LSBsaW51eC5vcmlnL2ZzL3NwbGljZS5jCTIwMDgtMDQtMDIgMTM6MzQ6NTguMDAwMDAwMDAwICsw MjAwCj4gKysrIGxpbnV4L2ZzL3NwbGljZS5jCTIwMDgtMDQtMDkgMTc6MzU6MDYuMDAwMDAwMDAw ICswMjAwCj4gQEAgLTQ4MSwxOSArNDgxLDIwIEBAIHNzaXplX3QgZ2VuZXJpY19maWxlX3NwbGlj ZV9yZWFkKHN0cnVjdAo+ICB7Cj4gIAlzc2l6ZV90IHNwbGljZWQ7Cj4gIAlpbnQgcmV0Owo+IC0J bG9mZl90IGlzaXplLCBsZWZ0Owo+IC0KPiAtCWlzaXplID0gaV9zaXplX3JlYWQoaW4tPmZfbWFw cGluZy0+aG9zdCk7Cj4gLQlpZiAodW5saWtlbHkoKnBwb3MgPj0gaXNpemUpKQo+IC0JCXJldHVy biAwOwo+IC0KPiAtCWxlZnQgPSBpc2l6ZSAtICpwcG9zOwo+IC0JaWYgKHVubGlrZWx5KGxlZnQg PCBsZW4pKQo+IC0JCWxlbiA9IGxlZnQ7Cj4KPiAgCXJldCA9IDA7Cj4gIAlzcGxpY2VkID0gMDsK PiAgCXdoaWxlIChsZW4gJiYgIXNwbGljZWQpIHsKPiArCQlsb2ZmX3QgaXNpemUsIGxlZnQ7Cj4g Kwo+ICsJCWlzaXplID0gaV9zaXplX3JlYWQoaW4tPmZfbWFwcGluZy0+aG9zdCk7Cj4gKwkJaWYg KHVubGlrZWx5KCpw ...
On Wed, 09 Apr 2008 17:57:56 +0200 We found suspicious-looking code in generic_file_splice_read() back in February. See http://lkml.org/lkml/2008/2/29/443. I suspect that patch (if it works) will address the truncate lockup as well - it zaps the loop entirely. Unfortunately Allard never got back to us (probably because he's running 2.6.24 which has a quite different generic_file_splice_read()) and the patch didn't get anywhere. Here it is again. It needs a changelog. Nobody has tested this at all, to my knowledge. It's going to take some serious and sudden effort to get these bugs fixed for 2.6.25. fs/splice.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff -puN fs/splice.c~generic_file_splice_read-fix-lockups fs/splice.c --- a/fs/splice.c~generic_file_splice_read-fix-lockups +++ a/fs/splice.c @@ -370,8 +370,10 @@ __generic_file_splice_read(struct file * * for an in-flight io page */ if (flags & SPLICE_F_NONBLOCK) { - if (TestSetPageLocked(page)) + if (TestSetPageLocked(page)) { + error = -EAGAIN; break; + } } else lock_page(page); @@ -479,9 +481,8 @@ ssize_t generic_file_splice_read(struct struct pipe_inode_info *pipe, size_t len, unsigned int flags) { - ssize_t spliced; - int ret; loff_t isize, left; + int ret; isize = i_size_read(in->f_mapping->host); if (unlikely(*ppos >= isize)) @@ -491,29 +492,9 @@ ssize_t generic_file_splice_read(struct if (unlikely(left < len)) len = left; - ret = 0; - spliced = 0; - while (len && !spliced) { - ret = __generic_file_splice_read(in, ppos, pipe, len, flags); - - if (ret < 0) - break; - else if (!ret) { - if (spliced) - break; - if (flags & SPLICE_F_NONBLOCK) { - ret = -EAGAIN; - break; - } - } - + ret = __generic_file_splice_read(in, ppos, pipe, len, flags); + if (ret > 0) *ppos += ret; - len -= ret; - spliced += ret; - } - - if ...
Patch looks sane, and fixes the fsx-linux/NFS/fuse lockup, as expected. Thanks, Miklos --
Hmm strange, I was pretty sure I pushed my patch back then. I'll double The original reporter did not, however others did. -- Jens Axboe --
Unfortunately, we cannot test the patch on the server that triggered the bug in 2.6.24.2, because it's too critical for operation. On other similar servers the bug hasn't been encountered and we were unable to reproduce it, so it must be some combination of load and typical usage (NFS server). We are now running 2.6.22.x, which does fine. I hope you understand our hesitation of running patched, known-bugged kernels on our fileservers :) Thanks for all the bughunting here. There is a maintenance window soon, I'll try to put the original patch on the agenda. Regards, Allard Hoeve --
