[linux-cifs-client] cifs: make cifs_writepages use longer timeout
diff mbox

Message ID 1238686320-23533-1-git-send-email-jlayton@redhat.com
State New, archived
Headers show

Commit Message

Jeff Layton April 2, 2009, 3:32 p.m. UTC
We've had a lot of changes to the socket sending and buffered write code
in recent months and I've just noticed some breakage due to it. The
"bigfile2" connectathon test does writes at 2G and 4G offsets in a file.
When running this test against a windows server, this is timing out
after 45s and throwing a -EAGAIN error back to userspace.

The following patch "fixes" it, but I'm not entirely thrilled with it.
It would be nice to detect that we are writing past the EOF and only
increase the timeout in that situation. The problem is that by the time
we're writing back pages i_size has already been updated.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Steve French April 2, 2009, 6:15 p.m. UTC | #1
When file is extended in write we could save the (previous) offset in
cifs_file struct ...?  And then increase the saved offset to match the
last write (past previous end of file).   Whenever we attempt a write
more than a few megabytes past the saved range, we can use long op?
Thoughts?

On Thu, Apr 2, 2009 at 10:32 AM, Jeff Layton <jlayton@redhat.com> wrote:
> We've had a lot of changes to the socket sending and buffered write code
> in recent months and I've just noticed some breakage due to it. The
> "bigfile2" connectathon test does writes at 2G and 4G offsets in a file.
> When running this test against a windows server, this is timing out
> after 45s and throwing a -EAGAIN error back to userspace.
>
> The following patch "fixes" it, but I'm not entirely thrilled with it.
> It would be nice to detect that we are writing past the EOF and only
> increase the timeout in that situation. The problem is that by the time
> we're writing back pages i_size has already been updated.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/file.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 81747ac..e86e7b5 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1532,7 +1532,7 @@ retry:
>                                                   open_file->netfid,
>                                                   bytes_to_write, offset,
>                                                   &bytes_written, iov, n_iov,
> -                                                  CIFS_LONG_OP);
> +                                                  CIFS_VLONG_OP);
>                                atomic_dec(&open_file->wrtPending);
>                                if (rc || bytes_written < bytes_to_write) {
>                                        cERROR(1, ("Write2 ret %d, wrote %d",
> --
> 1.5.5.6
>
>
Jeff Layton April 2, 2009, 6:28 p.m. UTC | #2
On Thu, 2 Apr 2009 13:15:39 -0500
Steve French <smfrench@gmail.com> wrote:

> When file is extended in write we could save the (previous) offset in
> cifs_file struct ...?  And then increase the saved offset to match the
> last write (past previous end of file).   Whenever we attempt a write
> more than a few megabytes past the saved range, we can use long op?
> Thoughts?
> 

Something like that might work. Basically I think we need to track the
current size as reported by the server separate from the i_size.

It's probably best suited to be in the CIFS_I struct rather than the
open file info. After a successful write past that value we'd update it.
We'd also update it when we get new FILE_*_INFO from the server.

Then we can use that value to decide how to time the op. At some
threshold we'll switch from a LONG to VLONG op.

I'll see what I can come up with...
Steve French April 2, 2009, 6:34 p.m. UTC | #3
On Thu, Apr 2, 2009 at 1:28 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Thu, 2 Apr 2009 13:15:39 -0500
> Steve French <smfrench@gmail.com> wrote:
>
>> When file is extended in write we could save the (previous) offset in
>> cifs_file struct ...?  And then increase the saved offset to match the
>> last write (past previous end of file).   Whenever we attempt a write
>> more than a few megabytes past the saved range, we can use long op?
>> Thoughts?
>>
>
> Something like that might work. Basically I think we need to track the
> current size as reported by the server separate from the i_size.
>
> It's probably best suited to be in the CIFS_I struct rather than the
> open file info. After a successful write past that value we'd update it.

Yes ... CIFS_I (not cifs_file) is better  :)

Patch
diff mbox

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 81747ac..e86e7b5 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1532,7 +1532,7 @@  retry:
 						   open_file->netfid,
 						   bytes_to_write, offset,
 						   &bytes_written, iov, n_iov,
-						   CIFS_LONG_OP);
+						   CIFS_VLONG_OP);
 				atomic_dec(&open_file->wrtPending);
 				if (rc || bytes_written < bytes_to_write) {
 					cERROR(1, ("Write2 ret %d, wrote %d",