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. 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> > -- Thanks, Steve -- 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
