Jeremy Allison
2008-Dec-23 19:15 UTC
[Samba] [jra@samba.org: Patch to improve Samba write speeds on Linux ext3 with 3.2.x]
----- Forwarded message from Jeremy Allison <jra@samba.org> -----
From: Jeremy Allison <jra@samba.org>
To: samba-technical@samba.org
Cc: jra@samba.org
Subject: Patch to improve Samba write speeds on Linux ext3 with 3.2.x
Reply-To: Jeremy Allison <jra@samba.org>
Hi all,
Intel recently did some research on Samba write speeds,
tested in Linux using ext3 resulting in the following
very interesting paper :
http://software.intel.com/en-us/articles/windows-client-cifs-behavior-can-slow-linux-nas-performance
This problem is only seen on Linux using the ext3
filesystem, not Linux with XFS or ext4. However Linux
with ext3 is one of our most popular platforms currently,
as it's what the major distros ship as default.
They subsequently sent me a paper including some simple
patches to Samba, which I've converted into a test patch
which should apply to any recent 3.2.x build.
I'd be interested to see if people can test this patch
to see if it improves Windows client write speed to Samba
in real-world cases (like the kind the people on this
list deal with every day :-).
I'm including this patch as an attachment. It is currently
experimental, and I need some feedback before we decide
if it's worth including in the mainline.
It alters the parameter "strict allocate" from a boolean
to a tri-state. To test, apply the patch and then add the
line :
strict allocate = partial
to the [global] section of your smb.conf and restart
smbd. I have one report so far of increased write performance
in IOZone benchmark tests from Windows, but I'd love
to get more data. Remember, this should only affect
write performance, not read.
The fix is simple enough that I could back-port it
to 3.0.x also if enough people demand it to test
(although I'm travelling between now + Sunday so
will only have intermittent email access).
Let me know how your tests go,
Thanks all !
Jeremy.
diff --git a/source/include/local.h b/source/include/local.h
index c125ded..b0fd7cb 100644
--- a/source/include/local.h
+++ b/source/include/local.h
@@ -253,4 +253,8 @@
/* Windows minimum lock resolution timeout in ms */
#define WINDOWS_MINIMUM_LOCK_TIMEOUT_MS 200
+/* When strict allocate = partial, define the limit on how far
+ * ahead we will write. */
+#define STRICT_ALLOCATE_PARTIAL_LIMIT 0x200000
+
#endif
diff --git a/source/include/smb.h b/source/include/smb.h
index bfada11..f1b6033 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -1966,4 +1966,10 @@ struct smb_extended_info {
char samba_version_string[SAMBA_EXTENDED_INFO_VERSION_STRING_LENGTH];
};
+enum smb_strict_allocate_options {
+ STRICT_ALLOCATE_OFF=0,
+ STRICT_ALLOCATE_ON=1,
+ STRICT_ALLOCATE_PARTIAL=2
+};
+
#endif /* _SMB_H */
diff --git a/source/modules/vfs_default.c b/source/modules/vfs_default.c
index 88c7237..8a724ad 100644
--- a/source/modules/vfs_default.c
+++ b/source/modules/vfs_default.c
@@ -745,10 +745,14 @@ static int vfswrap_ftruncate(vfs_handle_struct *handle,
files_struct *fsp, SMB_O
SMB_STRUCT_STAT st;
char c = 0;
SMB_OFF_T currpos;
+ enum smb_strict_allocate_options sa_options =
lp_strict_allocate(SNUM(fsp->conn));
START_PROFILE(syscall_ftruncate);
- if (lp_strict_allocate(SNUM(fsp->conn))) {
+ /* Only use allocation truncate if strict allocate
+ * is set "on", not off or partial.
+ */
+ if (sa_options == STRICT_ALLOCATE_ON) {
result = strict_allocate_ftruncate(handle, fsp, len);
END_PROFILE(syscall_ftruncate);
return result;
diff --git a/source/param/loadparm.c b/source/param/loadparm.c
index 81a308e..f3878c2 100644
--- a/source/param/loadparm.c
+++ b/source/param/loadparm.c
@@ -445,7 +445,7 @@ struct service {
bool bWidelinks;
bool bSymlinks;
bool bSyncAlways;
- bool bStrictAllocate;
+ int iStrictAllocate;
bool bStrictSync;
char magic_char;
struct bitmap *copymap;
@@ -588,7 +588,7 @@ static struct service sDefault = {
True, /* bWidelinks */
True, /* bSymlinks */
False, /* bSyncAlways */
- False, /* bStrictAllocate */
+ STRICT_ALLOCATE_OFF, /* iStrictAllocate */
False, /* bStrictSync */
'~', /* magic char */
NULL, /* copymap */
@@ -861,6 +861,21 @@ static const struct enum_list enum_config_backend[] = {
{-1, NULL}
};
+static const struct enum_list enum_strict_allocate[] = {
+ {STRICT_ALLOCATE_OFF, "No"},
+ {STRICT_ALLOCATE_OFF, "False"},
+ {STRICT_ALLOCATE_OFF, "0"},
+ {STRICT_ALLOCATE_OFF, "Off"},
+ {STRICT_ALLOCATE_OFF, "disabled"},
+ {STRICT_ALLOCATE_ON, "Yes"},
+ {STRICT_ALLOCATE_ON, "True"},
+ {STRICT_ALLOCATE_ON, "1"},
+ {STRICT_ALLOCATE_ON, "On"},
+ {STRICT_ALLOCATE_ON, "enabled"},
+ {STRICT_ALLOCATE_PARTIAL, "partial"},
+ {-1, NULL}
+};
+
/* Note: We do not initialise the defaults union - it is not allowed in ANSI C
*
* The FLAG_HIDE is explicit. Paramters set this way do NOT appear in any edit
@@ -2394,11 +2409,11 @@ static struct parm_struct parm_table[] = {
},
{
.label = "strict allocate",
- .type = P_BOOL,
+ .type = P_ENUM,
.p_class = P_LOCAL,
- .ptr = &sDefault.bStrictAllocate,
+ .ptr = &sDefault.iStrictAllocate,
.special = NULL,
- .enum_list = NULL,
+ .enum_list = enum_strict_allocate,
.flags = FLAG_ADVANCED | FLAG_SHARE,
},
{
@@ -5274,7 +5289,7 @@ FN_LOCAL_PARM_BOOL(lp_manglednames, bMangledNames)
FN_LOCAL_BOOL(lp_widelinks, bWidelinks)
FN_LOCAL_BOOL(lp_symlinks, bSymlinks)
FN_LOCAL_BOOL(lp_syncalways, bSyncAlways)
-FN_LOCAL_BOOL(lp_strict_allocate, bStrictAllocate)
+FN_LOCAL_INTEGER(lp_strict_allocate, iStrictAllocate)
FN_LOCAL_BOOL(lp_strict_sync, bStrictSync)
FN_LOCAL_BOOL(lp_map_system, bMap_system)
FN_LOCAL_BOOL(lp_delete_readonly, bDeleteReadonly)
diff --git a/source/smbd/fileio.c b/source/smbd/fileio.c
index 60aeeef..e23f391 100644
--- a/source/smbd/fileio.c
+++ b/source/smbd/fileio.c
@@ -127,9 +127,10 @@ static ssize_t real_write_file(struct smb_request *req,
if (pos == -1) {
ret = vfs_write_data(req, fsp, data, n);
} else {
+ enum smb_strict_allocate_options sa_options =
lp_strict_allocate(SNUM(fsp->conn));
fsp->fh->pos = pos;
- if (pos && lp_strict_allocate(SNUM(fsp->conn))) {
- if (vfs_fill_sparse(fsp, pos) == -1) {
+ if (pos && (sa_options != STRICT_ALLOCATE_OFF)) {
+ if (vfs_fill_sparse(fsp, pos, sa_options) == -1) {
return -1;
}
}
diff --git a/source/smbd/vfs.c b/source/smbd/vfs.c
index 6cf156c..e8c0ebb 100644
--- a/source/smbd/vfs.c
+++ b/source/smbd/vfs.c
@@ -545,8 +545,9 @@ int vfs_allocate_file_space(files_struct *fsp, SMB_BIG_UINT
len)
/* Grow - we need to test if we have enough space. */
- if (!lp_strict_allocate(SNUM(fsp->conn)))
+ if (lp_strict_allocate(SNUM(fsp->conn)) == STRICT_ALLOCATE_OFF) {
return 0;
+ }
len -= st.st_size;
len /= 1024; /* Len is now number of 1k blocks needed. */
@@ -600,7 +601,7 @@ int vfs_set_filelen(files_struct *fsp, SMB_OFF_T len)
static char *sparse_buf;
#define SPARSE_BUF_WRITE_SIZE (32*1024)
-int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len)
+int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len, enum
smb_strict_allocate_options sa_options)
{
int ret;
SMB_STRUCT_STAT st;
@@ -619,6 +620,14 @@ int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len)
return 0;
}
+ /* If strict allocate is set to "partial", ignore all allocate
+ * retquests over the STRICT_ALLOCATE_PARTIAL_LIMIT. */
+
+ if ((sa_options == STRICT_ALLOCATE_PARTIAL) &&
+ (len - st.st_size > STRICT_ALLOCATE_PARTIAL_LIMIT)) {
+ return 0;
+ }
+
DEBUG(10,("vfs_fill_sparse: write zeros in file %s from len %.0f to len
%.0f (%.0f bytes)\n",
fsp->fsp_name, (double)st.st_size, (double)len, (double)(len -
st.st_size)));
----- End forwarded message -----
-------------- next part --------------
diff --git a/source/include/local.h b/source/include/local.h
index c125ded..b0fd7cb 100644
--- a/source/include/local.h
+++ b/source/include/local.h
@@ -253,4 +253,8 @@
/* Windows minimum lock resolution timeout in ms */
#define WINDOWS_MINIMUM_LOCK_TIMEOUT_MS 200
+/* When strict allocate = partial, define the limit on how far
+ * ahead we will write. */
+#define STRICT_ALLOCATE_PARTIAL_LIMIT 0x200000
+
#endif
diff --git a/source/include/smb.h b/source/include/smb.h
index bfada11..f1b6033 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -1966,4 +1966,10 @@ struct smb_extended_info {
char samba_version_string[SAMBA_EXTENDED_INFO_VERSION_STRING_LENGTH];
};
+enum smb_strict_allocate_options {
+ STRICT_ALLOCATE_OFF=0,
+ STRICT_ALLOCATE_ON=1,
+ STRICT_ALLOCATE_PARTIAL=2
+};
+
#endif /* _SMB_H */
diff --git a/source/modules/vfs_default.c b/source/modules/vfs_default.c
index 88c7237..8a724ad 100644
--- a/source/modules/vfs_default.c
+++ b/source/modules/vfs_default.c
@@ -745,10 +745,14 @@ static int vfswrap_ftruncate(vfs_handle_struct *handle,
files_struct *fsp, SMB_O
SMB_STRUCT_STAT st;
char c = 0;
SMB_OFF_T currpos;
+ enum smb_strict_allocate_options sa_options =
lp_strict_allocate(SNUM(fsp->conn));
START_PROFILE(syscall_ftruncate);
- if (lp_strict_allocate(SNUM(fsp->conn))) {
+ /* Only use allocation truncate if strict allocate
+ * is set "on", not off or partial.
+ */
+ if (sa_options == STRICT_ALLOCATE_ON) {
result = strict_allocate_ftruncate(handle, fsp, len);
END_PROFILE(syscall_ftruncate);
return result;
diff --git a/source/param/loadparm.c b/source/param/loadparm.c
index 81a308e..f3878c2 100644
--- a/source/param/loadparm.c
+++ b/source/param/loadparm.c
@@ -445,7 +445,7 @@ struct service {
bool bWidelinks;
bool bSymlinks;
bool bSyncAlways;
- bool bStrictAllocate;
+ int iStrictAllocate;
bool bStrictSync;
char magic_char;
struct bitmap *copymap;
@@ -588,7 +588,7 @@ static struct service sDefault = {
True, /* bWidelinks */
True, /* bSymlinks */
False, /* bSyncAlways */
- False, /* bStrictAllocate */
+ STRICT_ALLOCATE_OFF, /* iStrictAllocate */
False, /* bStrictSync */
'~', /* magic char */
NULL, /* copymap */
@@ -861,6 +861,21 @@ static const struct enum_list enum_config_backend[] = {
{-1, NULL}
};
+static const struct enum_list enum_strict_allocate[] = {
+ {STRICT_ALLOCATE_OFF, "No"},
+ {STRICT_ALLOCATE_OFF, "False"},
+ {STRICT_ALLOCATE_OFF, "0"},
+ {STRICT_ALLOCATE_OFF, "Off"},
+ {STRICT_ALLOCATE_OFF, "disabled"},
+ {STRICT_ALLOCATE_ON, "Yes"},
+ {STRICT_ALLOCATE_ON, "True"},
+ {STRICT_ALLOCATE_ON, "1"},
+ {STRICT_ALLOCATE_ON, "On"},
+ {STRICT_ALLOCATE_ON, "enabled"},
+ {STRICT_ALLOCATE_PARTIAL, "partial"},
+ {-1, NULL}
+};
+
/* Note: We do not initialise the defaults union - it is not allowed in ANSI C
*
* The FLAG_HIDE is explicit. Paramters set this way do NOT appear in any edit
@@ -2394,11 +2409,11 @@ static struct parm_struct parm_table[] = {
},
{
.label = "strict allocate",
- .type = P_BOOL,
+ .type = P_ENUM,
.p_class = P_LOCAL,
- .ptr = &sDefault.bStrictAllocate,
+ .ptr = &sDefault.iStrictAllocate,
.special = NULL,
- .enum_list = NULL,
+ .enum_list = enum_strict_allocate,
.flags = FLAG_ADVANCED | FLAG_SHARE,
},
{
@@ -5274,7 +5289,7 @@ FN_LOCAL_PARM_BOOL(lp_manglednames, bMangledNames)
FN_LOCAL_BOOL(lp_widelinks, bWidelinks)
FN_LOCAL_BOOL(lp_symlinks, bSymlinks)
FN_LOCAL_BOOL(lp_syncalways, bSyncAlways)
-FN_LOCAL_BOOL(lp_strict_allocate, bStrictAllocate)
+FN_LOCAL_INTEGER(lp_strict_allocate, iStrictAllocate)
FN_LOCAL_BOOL(lp_strict_sync, bStrictSync)
FN_LOCAL_BOOL(lp_map_system, bMap_system)
FN_LOCAL_BOOL(lp_delete_readonly, bDeleteReadonly)
diff --git a/source/smbd/fileio.c b/source/smbd/fileio.c
index 60aeeef..e23f391 100644
--- a/source/smbd/fileio.c
+++ b/source/smbd/fileio.c
@@ -127,9 +127,10 @@ static ssize_t real_write_file(struct smb_request *req,
if (pos == -1) {
ret = vfs_write_data(req, fsp, data, n);
} else {
+ enum smb_strict_allocate_options sa_options =
lp_strict_allocate(SNUM(fsp->conn));
fsp->fh->pos = pos;
- if (pos && lp_strict_allocate(SNUM(fsp->conn))) {
- if (vfs_fill_sparse(fsp, pos) == -1) {
+ if (pos && (sa_options != STRICT_ALLOCATE_OFF)) {
+ if (vfs_fill_sparse(fsp, pos, sa_options) == -1) {
return -1;
}
}
diff --git a/source/smbd/vfs.c b/source/smbd/vfs.c
index 6cf156c..e8c0ebb 100644
--- a/source/smbd/vfs.c
+++ b/source/smbd/vfs.c
@@ -545,8 +545,9 @@ int vfs_allocate_file_space(files_struct *fsp, SMB_BIG_UINT
len)
/* Grow - we need to test if we have enough space. */
- if (!lp_strict_allocate(SNUM(fsp->conn)))
+ if (lp_strict_allocate(SNUM(fsp->conn)) == STRICT_ALLOCATE_OFF) {
return 0;
+ }
len -= st.st_size;
len /= 1024; /* Len is now number of 1k blocks needed. */
@@ -600,7 +601,7 @@ int vfs_set_filelen(files_struct *fsp, SMB_OFF_T len)
static char *sparse_buf;
#define SPARSE_BUF_WRITE_SIZE (32*1024)
-int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len)
+int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len, enum
smb_strict_allocate_options sa_options)
{
int ret;
SMB_STRUCT_STAT st;
@@ -619,6 +620,14 @@ int vfs_fill_sparse(files_struct *fsp, SMB_OFF_T len)
return 0;
}
+ /* If strict allocate is set to "partial", ignore all allocate
+ * retquests over the STRICT_ALLOCATE_PARTIAL_LIMIT. */
+
+ if ((sa_options == STRICT_ALLOCATE_PARTIAL) &&
+ (len - st.st_size > STRICT_ALLOCATE_PARTIAL_LIMIT)) {
+ return 0;
+ }
+
DEBUG(10,("vfs_fill_sparse: write zeros in file %s from len %.0f to len
%.0f (%.0f bytes)\n",
fsp->fsp_name, (double)st.st_size, (double)len, (double)(len -
st.st_size)));
Apparently Analagous Threads
Wisdom of the Ancients
