The branch, master has been updated
via 3fea05e01f8 smbd: Remove write cache
from a0aaf5c3345 lib: Remove unused file_id_string()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 3fea05e01f845588eb0de63af435bfec670593be
Author: Volker Lendecke <v...@samba.org>
Date: Mon May 27 11:24:14 2019 +0200
smbd: Remove write cache
Since this was written, our write path has changed significantly. In
particular we have gained very flexible support for async I/O, with the
linux io_uring in the pipeline. Caching stuff in main memory and then
doing a blocking pwrite nowadays does not belong into the core smbd
code. If someone wants it back, it should be doable in a VFS module.
Removes: "write cache size" parameter.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Wed Nov 13 00:20:55 UTC 2019 on sn-devel-184
-----------------------------------------------------------------------
Summary of changes:
WHATSNEW.txt | 18 +
docs-xml/smbdotconf/tuning/writecachesize.xml | 33 --
source3/include/smb.h | 15 -
source3/include/smbprofile.h | 21 -
source3/include/vfs.h | 2 +-
source3/param/loadparm.c | 1 -
source3/smbd/aio.c | 20 +-
source3/smbd/close.c | 24 -
source3/smbd/fileio.c | 767 +-------------------------
source3/smbd/globals.c | 3 -
source3/smbd/globals.h | 3 -
source3/smbd/open.c | 2 -
source3/smbd/oplock.c | 6 -
source3/smbd/proto.h | 3 -
source3/smbd/reply.c | 6 -
source3/smbd/smb2_flush.c | 7 -
source3/smbd/smb2_read.c | 1 -
source3/smbd/vfs.c | 13 +-
18 files changed, 26 insertions(+), 919 deletions(-)
delete mode 100644 docs-xml/smbdotconf/tuning/writecachesize.xml
Changeset truncated at 500 lines:
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index dccb44dbd27..cc43b29b3d1 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -56,6 +56,23 @@ applies.
REMOVED FEATURES
================
+The smb.conf parameter "write cache size" has been removed.
+
+Since the in-memory write caching code was written, our write path has
+changed significantly. In particular we have gained very flexible
+support for async I/O, with the new linux io_uring interface in
+development. The old write cache concept which cached data in main
+memory followed by a blocking pwrite no longer gives any improvement
+on modern systems, and may make performance worse on memory-contrained
+systems, so this functionality should not be enabled in core smbd
+code.
+
+In addition, it complicated the write code, which is a performance
+critical code path.
+
+If required for specialist purposes, it can be recreated as a VFS
+module.
+
BIND9_FLATFILE deprecated
-------------------------
@@ -77,6 +94,7 @@ smb.conf changes
nfs4:acedup Changed default merge
rndc command Removed
+ write cache size Removed
KNOWN ISSUES
============
diff --git a/docs-xml/smbdotconf/tuning/writecachesize.xml
b/docs-xml/smbdotconf/tuning/writecachesize.xml
deleted file mode 100644
index 484b35398bf..00000000000
--- a/docs-xml/smbdotconf/tuning/writecachesize.xml
+++ /dev/null
@@ -1,33 +0,0 @@
-<samba:parameter name="write cache size"
- context="S"
- type="bytes"
- xmlns:samba="http://www.samba.org/samba/DTD/samba-doc";>
-<description>
- <para>If this integer parameter is set to non-zero value,
- Samba will create an in-memory cache for each oplocked file
- (it does <emphasis>not</emphasis> do this for
- non-oplocked files). All writes that the client does not request
- to be flushed directly to disk will be stored in this cache if possible.
- The cache is flushed onto disk when a write comes in whose offset
- would not fit into the cache or when the file is closed by the client.
- Reads for the file are also served from this cache if the data is stored
- within it.</para>
-
- <para>This cache allows Samba to batch client writes into a more
- efficient write size for RAID disks (i.e. writes may be tuned to
- be the RAID stripe size) and can improve performance on systems
- where the disk subsystem is a bottleneck but there is free
- memory for userspace programs.</para>
-
- <para>The integer parameter specifies the size of this cache
- (per oplocked file) in bytes.</para>
-
- <para>Note that the write cache won't be used for file handles with a smb2
write lease.</para>
-</description>
-
-<related>aio read size</related>
-<related>aio write size</related>
-<related>smb2 leases</related>
-<value type="default">0</value>
-<value type="example">262144<comment> for a 256k cache size per
file</comment></value>
-</samba:parameter>
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 012ed485494..162cdcc1a32 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -749,19 +749,4 @@ struct smb_extended_info {
char samba_version_string[SAMBA_EXTENDED_INFO_VERSION_STRING_LENGTH];
};
-/*
- * Reasons for cache flush.
- */
-
-enum flush_reason_enum {
- SAMBA_SEEK_FLUSH,
- SAMBA_READ_FLUSH,
- SAMBA_WRITE_FLUSH,
- SAMBA_READRAW_FLUSH,
- SAMBA_OPLOCK_RELEASE_FLUSH,
- SAMBA_CLOSE_FLUSH,
- SAMBA_SYNC_FLUSH,
- SAMBA_SIZECHANGE_FLUSH,
-};
-
#endif /* _SMB_H */
diff --git a/source3/include/smbprofile.h b/source3/include/smbprofile.h
index a003a1d9df0..b771c26c81b 100644
--- a/source3/include/smbprofile.h
+++ b/source3/include/smbprofile.h
@@ -108,27 +108,6 @@ struct tevent_context;
SMBPROFILE_STATS_COUNT(statcache_hits) \
SMBPROFILE_STATS_SECTION_END \
\
- SMBPROFILE_STATS_SECTION_START(writecache, "Write Cache") \
- SMBPROFILE_STATS_COUNT(writecache_allocations) \
- SMBPROFILE_STATS_COUNT(writecache_deallocations) \
- SMBPROFILE_STATS_COUNT(writecache_cached_reads) \
- SMBPROFILE_STATS_COUNT(writecache_total_writes) \
- SMBPROFILE_STATS_COUNT(writecache_init_writes) \
- SMBPROFILE_STATS_COUNT(writecache_abutted_writes) \
- SMBPROFILE_STATS_COUNT(writecache_non_oplock_writes) \
- SMBPROFILE_STATS_COUNT(writecache_direct_writes) \
- SMBPROFILE_STATS_COUNT(writecache_cached_writes) \
- SMBPROFILE_STATS_COUNT(writecache_perfect_writes) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_seek) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_read) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_readraw) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_write) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_oplock) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_close) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_sync) \
- SMBPROFILE_STATS_COUNT(writecache_flush_reason_sizechange) \
- SMBPROFILE_STATS_SECTION_END \
- \
SMBPROFILE_STATS_SECTION_START(SMB, "SMB Calls") \
SMBPROFILE_STATS_BASIC(SMBmkdir) \
SMBPROFILE_STATS_BASIC(SMBrmdir) \
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 802eb8d0292..a6c57c6bcbc 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -286,6 +286,7 @@
/* Version 42 - Remove SMB_VFS_RMDIR.
Use SMB_VFS_UNLINKAT(.., AT_REMOVEDIR) instead. */
/* Version 42 - Remove SMB_VFS_CHOWN */
+/* Version 42 - Remove struct write_cache *wcp from files_struct */
#define SMB_VFS_INTERFACE_VERSION 42
@@ -347,7 +348,6 @@ typedef struct files_struct {
uint64_t initial_allocation_size; /* Faked up initial allocation on
disk. */
uint16_t file_pid;
uint64_t vuid; /* SMB2 compat */
- struct write_cache *wcp;
struct timeval open_time;
uint32_t access_mask; /* NTCreateX access bits
(FILE_READ_DATA etc.) */
bool kernel_share_modes_taken;
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 433762eedfb..31fa229d5ff 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -160,7 +160,6 @@ static const struct loadparm_service _sDefault =
.min_print_space = 0,
.max_print_jobs = 1000,
.max_reported_print_jobs = 0,
- .write_cache_size = 0,
.create_mask = 0744,
.force_create_mode = 0,
.directory_mask = 0755,
diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c
index 8ac3ef7278e..0f824f5aa1f 100644
--- a/source3/smbd/aio.c
+++ b/source3/smbd/aio.c
@@ -172,9 +172,8 @@ NTSTATUS schedule_aio_read_and_X(connection_struct *conn,
return NT_STATUS_RETRY;
}
- /* Only do this on non-chained and non-chaining reads not using the
- * write cache. */
- if (req_is_in_chain(smbreq) || (lp_write_cache_size(SNUM(conn)) != 0))
{
+ /* Only do this on non-chained and non-chaining reads */
+ if (req_is_in_chain(smbreq)) {
return NT_STATUS_RETRY;
}
@@ -428,9 +427,8 @@ NTSTATUS schedule_aio_write_and_X(connection_struct *conn,
return NT_STATUS_RETRY;
}
- /* Only do this on non-chained and non-chaining writes not using the
- * write cache. */
- if (req_is_in_chain(smbreq) || (lp_write_cache_size(SNUM(conn)) != 0))
{
+ /* Only do this on non-chained and non-chaining writes */
+ if (req_is_in_chain(smbreq)) {
return NT_STATUS_RETRY;
}
@@ -673,11 +671,6 @@ NTSTATUS schedule_smb2_aio_read(connection_struct *conn,
return NT_STATUS_RETRY;
}
- /* Only do this on reads not using the write cache. */
- if (lp_write_cache_size(SNUM(conn)) != 0) {
- return NT_STATUS_RETRY;
- }
-
if (smbd_smb2_is_compound(smbreq->smb2req)) {
return NT_STATUS_RETRY;
}
@@ -813,11 +806,6 @@ NTSTATUS schedule_aio_smb2_write(connection_struct *conn,
return NT_STATUS_RETRY;
}
- /* Only do this on writes not using the write cache. */
- if (lp_write_cache_size(SNUM(conn)) != 0) {
- return NT_STATUS_RETRY;
- }
-
if (smbd_smb2_is_compound(smbreq->smb2req)) {
return NT_STATUS_RETRY;
}
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index e62480d69da..180a5da735b 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -140,24 +140,6 @@ static NTSTATUS check_magic(struct files_struct *fsp)
return status;
}
-/****************************************************************************
- Common code to close a file or a directory.
-****************************************************************************/
-
-static NTSTATUS close_filestruct(files_struct *fsp)
-{
- NTSTATUS status = NT_STATUS_OK;
-
- if (fsp->fh->fd != -1) {
- if(flush_write_cache(fsp, SAMBA_CLOSE_FLUSH) == -1) {
- status = map_nt_error_from_unix(errno);
- }
- delete_write_cache(fsp);
- }
-
- return status;
-}
-
/****************************************************************************
Delete all streams
****************************************************************************/
@@ -715,9 +697,6 @@ static NTSTATUS close_normal_file(struct smb_request *req,
files_struct *fsp,
* error here, we must remember this.
*/
- tmp = close_filestruct(fsp);
- status = ntstatus_keeperror(status, tmp);
-
if (NT_STATUS_IS_OK(status) && fsp->op != NULL) {
is_durable = fsp->op->global->durable;
}
@@ -1182,7 +1161,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
if (lck == NULL) {
DEBUG(0, ("close_directory: Could not get share mode lock for "
"%s\n", fsp_str_dbg(fsp)));
- close_filestruct(fsp);
file_free(req, fsp);
return NT_STATUS_INVALID_PARAMETER;
}
@@ -1242,7 +1220,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(5, ("delete_all_streams failed: %s\n",
nt_errstr(status)));
- close_filestruct(fsp);
file_free(req, fsp);
return status;
}
@@ -1287,7 +1264,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
/*
* Do the code common to files and directories.
*/
- close_filestruct(fsp);
file_free(req, fsp);
if (NT_STATUS_IS_OK(status) && !NT_STATUS_IS_OK(status1)) {
diff --git a/source3/smbd/fileio.c b/source3/smbd/fileio.c
index 067ce5a9ad4..b03f86d49d6 100644
--- a/source3/smbd/fileio.c
+++ b/source3/smbd/fileio.c
@@ -25,39 +25,6 @@
#include "smbd/globals.h"
#include "smbprofile.h"
-struct write_cache {
- off_t file_size;
- off_t offset;
- size_t alloc_size;
- size_t data_size;
- char *data;
-};
-
-static bool setup_write_cache(files_struct *, off_t);
-
-/****************************************************************************
- Read from write cache if we can.
-****************************************************************************/
-
-static bool read_from_write_cache(files_struct *fsp,char *data,off_t
pos,size_t n)
-{
- struct write_cache *wcp = fsp->wcp;
-
- if(!wcp) {
- return False;
- }
-
- if( n > wcp->data_size || pos < wcp->offset || pos + n > wcp->offset +
wcp->data_size) {
- return False;
- }
-
- memcpy(data, wcp->data + (pos - wcp->offset), n);
-
- DO_PROFILE_INC(writecache_cached_reads);
-
- return True;
-}
-
/****************************************************************************
Read from a file.
****************************************************************************/
@@ -72,18 +39,6 @@ ssize_t read_file(files_struct *fsp,char *data,off_t
pos,size_t n)
return -1;
}
- /*
- * Serve from write cache if we can.
- */
-
- if(read_from_write_cache(fsp, data, pos, n)) {
- fsp->fh->pos = pos + n;
- fsp->fh->position_information = fsp->fh->pos;
- return n;
- }
-
- flush_write_cache(fsp, SAMBA_READ_FLUSH);
-
fsp->fh->pos = pos;
if (n > 0) {
@@ -140,26 +95,6 @@ static ssize_t real_write_file(struct smb_request *req,
return ret;
}
-/****************************************************************************
- File size cache change.
- Updates size on disk but doesn't flush the cache.
-****************************************************************************/
-
-static int wcp_file_size_change(files_struct *fsp)
-{
- int ret;
- struct write_cache *wcp = fsp->wcp;
-
- wcp->file_size = wcp->offset + wcp->data_size;
- ret = SMB_VFS_FTRUNCATE(fsp, wcp->file_size);
- if (ret == -1) {
- DEBUG(0,("wcp_file_size_change (%s): ftruncate of size %.0f "
- "error %s\n", fsp_str_dbg(fsp),
- (double)wcp->file_size, strerror(errno)));
- }
- return ret;
-}
-
void fsp_flush_write_time_update(struct files_struct *fsp)
{
/*
@@ -314,9 +249,7 @@ ssize_t write_file(struct smb_request *req,
off_t pos,
size_t n)
{
- struct write_cache *wcp = fsp->wcp;
ssize_t total_written = 0;
- int write_path = -1;
if (fsp->print_file) {
uint32_t t;
@@ -335,30 +268,8 @@ ssize_t write_file(struct smb_request *req,
return -1;
}
- /*
- * If this is the first write and we have an exclusive oplock
- * then setup the write cache.
- */
-
- if (!fsp->modified &&
- EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type) &&
- (wcp == NULL)) {
- /*
- * Note: no write cache with leases!
- * as the handles would have to share the write cache
- * that's possible but an improvement for another day...
- */
- setup_write_cache(fsp, fsp->fsp_name->st.st_ex_size);
- wcp = fsp->wcp;
- }
-
mark_file_modified(fsp);
- DO_PROFILE_INC(writecache_total_writes);
- if (!fsp->oplock_type) {
- DO_PROFILE_INC(writecache_non_oplock_writes);
- }
-
/*
* If this file is level II oplocked then we need
* to grab the shared memory lock and inform all
@@ -371,681 +282,10 @@ ssize_t write_file(struct smb_request *req,
contend_level2_oplocks_begin(fsp, LEVEL2_CONTEND_WRITE);
contend_level2_oplocks_end(fsp, LEVEL2_CONTEND_WRITE);
- if (wcp && req->unread_bytes) {
- /* If we're using receivefile don't
- * deal with a write cache.
- */
- flush_write_cache(fsp, SAMBA_WRITE_FLUSH);
- delete_write_cache(fsp);
- wcp = NULL;
- }
-
- if(!wcp) {
- DO_PROFILE_INC(writecache_direct_writes);
- total_written = real_write_file(req, fsp, data, pos, n);
- return total_written;
- }
-
- DEBUG(9,("write_file (%s)(fd=%d pos=%.0f size=%u) wcp->offset=%.0f "
- "wcp->data_size=%u\n", fsp_str_dbg(fsp), fsp->fh->fd,
- (double)pos, (unsigned int)n, (double)wcp->offset,
- (unsigned int)wcp->data_size));
-
- fsp->fh->pos = pos + n;
-
- if ((n == 1) && (data[0] == '\0') && (pos > wcp->file_size)) {
- int ret;
-
- /*
- * This is a 1-byte write of a 0 beyond the EOF and
- * thus implicitly also beyond the current active
- * write cache, the typical file-extending (and
- * allocating, but we're using the write cache here)
- * write done by Windows. We just have to ftruncate
- * the file and rely on posix semantics to return
- * zeros for non-written file data that is within the
- * file length.
- *
- * We can not use wcp_file_size_change here because we
- * might have an existing write cache, and
- * wcp_file_size_change assumes a change to just the
- * end of the current write cache.
- */
-
- wcp->file_size = pos + 1;
- ret = SMB_VFS_FTRUNCATE(fsp, wcp->file_size);
- if (ret == -1) {
- DEBUG(0, ("wcp_file_size_change (%s): ftruncate of "
- "size %.0f error %s\n", fsp_str_dbg(fsp),
- (double)wcp->file_size, strerror(errno)));
- return -1;
- }
- return 1;
- }
-
-
- /*
- * If we have active cache and it isn't contiguous then we flush.
- * NOTE: There is a small problem with running out of disk ....
- */
-
- if (wcp->data_size) {
- bool cache_flush_needed = False;
-
- if ((pos >= wcp->offset) &&
- (pos <= wcp->offset + wcp->data_size)) {
-
- /* ASCII art.... JRA.
-
- +--------------+-----
- | Cached data | Rest of allocated cache buffer....
- +--------------+-----
-
- +-------------------+
- | Data to write |
- +-------------------+
-
- */
-
- /*
- * Start of write overlaps or abutts the existing data.
- */
-
- size_t data_used;
-
- data_used = MIN((wcp->alloc_size - (pos - wcp->offset)),
- n);
-
- memcpy(wcp->data + (pos - wcp->offset), data,
- data_used);
--
Samba Shared Repository
