On Wed, 9 Nov 2011 14:55:36 -0600 Steve French <smfrench@xxxxxxxxx> wrote: > How much of a performance hit does this make? I would expect that > dropping the writesize by more than 50% to most non-Samba servers to > work around a bug in Solaris seems extreme. If this hurts our > performance measurably, it may be more pragmatic to limit the wsize > change to a subset of these (e.g. based on server type) - seems more > fair to not punish other servers for a Solaris bug. > I'm not sure how big a hit it will be, but it won't be 0. As always, it depends on workload. The problem is, I'm not sure we can reliably detect when the server can't handle larger writes like this. Most servers are fine with larger writes, but when it's not then the result is failed writeback. Given that a lot of userspace code doesn't check the return code on close(), then this manifests itself as "silent" data corruption. So, while I don't like it, I think we're basically of constrained by Windows' behavior here. We really have to ensure that the defaults work for everyone, even if they're not what we'd consider ideal. Note too that this just sets the default wsize to 65536 in this situation. There's nothing that prevents someone from setting it higher if they have a server that can handle larger writes. > On Wed, Nov 9, 2011 at 12:41 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Wed, 9 Nov 2011 13:37:23 -0500 > > Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > >> We've had some reports of servers (namely, the Solaris in-kernel CIFS > >> server) that don't deal properly with writes that are "too large" even > >> though they set CAP_LARGE_WRITE_ANDX. Change the default to better > >> mirror what windows clients do. > >> > >> Cc: Pavel Shilovsky <piastry@xxxxxxxxxxx> > >> Reported-by: Nick Davis <phireph0x@xxxxxxxxx> > >> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > >> --- > >> fs/cifs/connect.c | 23 +++++++++++++++++++---- > >> 1 files changed, 19 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > >> index d6a972d..bf82f88 100644 > >> --- a/fs/cifs/connect.c > >> +++ b/fs/cifs/connect.c > >> @@ -2912,18 +2912,33 @@ void cifs_setup_cifs_sb(struct smb_vol *pvolume_info, > >> #define CIFS_DEFAULT_IOSIZE (1024 * 1024) > >> > >> /* > >> - * Windows only supports a max of 60k reads. Default to that when posix > >> - * extensions aren't in force. > >> + * Windows only supports a max of 60kb reads and 65535 byte writes. Default to > >> + * those values when posix extensions aren't in force. In actuality here, we > >> + * use 65536 to allow for a write that is a multiple of 4k. Most servers seem > >> + * to be ok with the extra byte even though Windows doesn't send writes that > >> + * are that large. > >> + * > >> + * Citation: > >> + * > >> + * http://blogs.msdn.com/b/openspecification/archive/2009/04/10/smb-maximum-transmit-buffer-size-and-performance-tuning.aspx > >> */ > >> #define CIFS_DEFAULT_NON_POSIX_RSIZE (60 * 1024) > >> +#define CIFS_DEFAULT_NON_POSIX_WSIZE (65536) > >> > >> static unsigned int > >> cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *pvolume_info) > >> { > >> __u64 unix_cap = le64_to_cpu(tcon->fsUnixInfo.Capability); > >> struct TCP_Server_Info *server = tcon->ses->server; > >> - unsigned int wsize = pvolume_info->wsize ? pvolume_info->wsize : > >> - CIFS_DEFAULT_IOSIZE; > >> + unsigned int wsize; > >> + > >> + /* start with specified wsize, or default */ > >> + if (pvolume_info->wsize) > >> + wsize = pvolume_info->wsize; > >> + else if (tcon->unix_ext && (unix_cap & CIFS_UNIX_LARGE_WRITE_CAP)) > >> + wsize = CIFS_DEFAULT_IOSIZE; > >> + else > >> + wsize = CIFS_DEFAULT_NON_POSIX_WSIZE; > >> > >> /* can server support 24-bit write sizes? (via UNIX extensions) */ > >> if (!tcon->unix_ext || !(unix_cap & CIFS_UNIX_LARGE_WRITE_CAP)) > > > > > > I should also mention that I suspect that this will fix this bug > > without needing extra options: > > > > https://bugzilla.samba.org/show_bug.cgi?id=8559 > > > > If it looks acceptable, this might be suitable for stable too. > > -- > > Jeff Layton <jlayton@xxxxxxxxxx> > > > > > -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
