[PATCHES BUG 13446] smbd: Cache dfree information based on query path
Jeremy Allison
jra at samba.org
Mon May 21 19:46:33 UTC 2018
On Fri, May 18, 2018 at 08:54:26PM -0700, Christof Schmitt via samba-technical wrote:
> Forgot to include one patch to flush the cache on service reload. See
> the last patch in this series.
Couple of comments. The key for the cache is incorrect, you're
only using fname->base_name - this can easily be duplicated across
different shares (for example, multiple shares all with a "TEMP"
sub directory).
The cache key needs to be the absolute pathname, not the share
relative one.
Secondly, removing:
- struct dfree_cached_info *dfree_info;
from the typedef struct connection_struct { is an ABI
change for the VFS. Many VFS modules indirect into
connection_struct *conn which will break if introduced
as a back-pot for a patch.
So this change is OK for master (but you need a comment in
source3/include/vfs.h to say you're changing the VFS interface
ABI number, or at least logging the fact you're changing it
as the number is already incremented for 4.9.0), but
this part of the patch can't be back-ported for any
released version.
Jeremy.
> From dfb466ef2d647c5436d32379e0f36f6779255a44 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 16 May 2018 13:05:36 -0700
> Subject: [PATCH 1/4] memcache: Add new cache type for dfree information
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> lib/util/memcache.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/util/memcache.h b/lib/util/memcache.h
> index 670cbd6..4331c2f 100644
> --- a/lib/util/memcache.h
> +++ b/lib/util/memcache.h
> @@ -45,7 +45,8 @@ enum memcache_number {
> SINGLETON_CACHE,
> SMB1_SEARCH_OFFSET_MAP,
> SHARE_MODE_LOCK_CACHE, /* talloc */
> - VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC /* talloc */
> + VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC, /* talloc */
> + DFREE_CACHE,
> };
>
> /*
> --
> 1.8.3.1
>
>
> From 9ff637d587f6e30376ce93236163a0dfe2ba710f Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 16 May 2018 13:17:52 -0700
> Subject: [PATCH 2/4] smbd: Cache dfree information based on query path
>
> Sub directories in a SMB share can have different free space information
> (e.g. when a different file system is mounted there). Caching the dfree
> information per SMB share will return invalid data. Address this by
> switching to memcache and store the cached data based on the query path.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/smbd/dfree.c | 39 +++++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c
> index a702d08..841d537 100644
> --- a/source3/smbd/dfree.c
> +++ b/source3/smbd/dfree.c
> @@ -21,6 +21,7 @@
> #include "smbd/smbd.h"
> #include "smbd/globals.h"
> #include "lib/util_file.h"
> +#include "lib/util/memcache.h"
>
> /****************************************************************************
> Normalise for DOS usage.
> @@ -167,21 +168,33 @@ dfree_done:
>
> /****************************************************************************
> Potentially returned cached dfree info.
> +
> + Depending on the file system layout and file system features, the free space
> + information can be different for different sub directories underneath a SMB
> + share. Store the cache information in memcache using the query path as the
> + key to accomodate this.
> ****************************************************************************/
>
> uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> uint64_t *bsize, uint64_t *dfree, uint64_t *dsize)
> {
> int dfree_cache_time = lp_dfree_cache_time(SNUM(conn));
> - struct dfree_cached_info *dfc = conn->dfree_info;
> + struct dfree_cached_info *dfc = NULL, dfc_new;
> uint64_t dfree_ret;
> + DATA_BLOB key, value;
> + bool found;
>
> if (!dfree_cache_time) {
> return sys_disk_free(conn, fname, bsize, dfree, dsize);
> }
>
> + key = data_blob_const(fname->base_name, strlen(fname->base_name));
> + found = memcache_lookup(smbd_memcache(), DFREE_CACHE, key, &value);
> + dfc = found ? (struct dfree_cached_info *)value.data : NULL;
> +
> if (dfc && (conn->lastused - dfc->last_dfree_time < dfree_cache_time)) {
> - /* Return cached info. */
> + DBG_DEBUG("Returning cached dfree info for %s\n",
> + fname->base_name);
> *bsize = dfc->bsize;
> *dfree = dfc->dfree;
> *dsize = dfc->dsize;
> @@ -195,20 +208,14 @@ uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> return dfree_ret;
> }
>
> - /* No cached info or time to refresh. */
> - if (!dfc) {
> - dfc = talloc(conn, struct dfree_cached_info);
> - if (!dfc) {
> - return dfree_ret;
> - }
> - conn->dfree_info = dfc;
> - }
> -
> - dfc->bsize = *bsize;
> - dfc->dfree = *dfree;
> - dfc->dsize = *dsize;
> - dfc->dfree_ret = dfree_ret;
> - dfc->last_dfree_time = conn->lastused;
> + dfc_new.bsize = *bsize;
> + dfc_new.dfree = *dfree;
> + dfc_new.dsize = *dsize;
> + dfc_new.dfree_ret = dfree_ret;
> + dfc_new.last_dfree_time = conn->lastused;
> + DBG_DEBUG("Creating cached dfree entry for %s\n", fname->base_name);
> + memcache_add(smbd_memcache(), DFREE_CACHE,
> + key, data_blob_const(&dfc_new, sizeof(dfc_new)));
>
> return dfree_ret;
> }
> --
> 1.8.3.1
>
>
> From 066de08f5ae5cbd5cb68c3b595867fde8ee0ea73 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 16 May 2018 13:25:54 -0700
> Subject: [PATCH 3/4] smbd: Move dfree_info struct
>
> As the struct is no longer used as part of connection_struct, move it to
> dfree.c
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/include/vfs.h | 9 ---------
> source3/smbd/dfree.c | 8 ++++++++
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/source3/include/vfs.h b/source3/include/vfs.h
> index cc7948a..cdf6fb3 100644
> --- a/source3/include/vfs.h
> +++ b/source3/include/vfs.h
> @@ -409,14 +409,6 @@ typedef struct {
> bool is_wild;
> } name_compare_entry;
>
> -struct dfree_cached_info {
> - time_t last_dfree_time;
> - uint64_t dfree_ret;
> - uint64_t bsize;
> - uint64_t dfree;
> - uint64_t dsize;
> -};
> -
> struct share_params {
> int service;
> };
> @@ -479,7 +471,6 @@ typedef struct connection_struct {
> name_compare_entry *veto_list; /* Per-share list of files to veto (never show). */
> name_compare_entry *veto_oplock_list; /* Per-share list of files to refuse oplocks on. */
> name_compare_entry *aio_write_behind_list; /* Per-share list of files to use aio write behind on. */
> - struct dfree_cached_info *dfree_info;
> struct trans_state *pending_trans;
>
> struct rpc_pipe_client *spoolss_pipe;
> diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c
> index 841d537..8863e58 100644
> --- a/source3/smbd/dfree.c
> +++ b/source3/smbd/dfree.c
> @@ -175,6 +175,14 @@ dfree_done:
> key to accomodate this.
> ****************************************************************************/
>
> +struct dfree_cached_info {
> + time_t last_dfree_time;
> + uint64_t dfree_ret;
> + uint64_t bsize;
> + uint64_t dfree;
> + uint64_t dsize;
> +};
> +
> uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> uint64_t *bsize, uint64_t *dfree, uint64_t *dsize)
> {
> --
> 1.8.3.1
>
>
> From 26e6a2e857a714518a980ce6919e0e9b4d2cd4be Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Fri, 18 May 2018 20:51:58 -0700
> Subject: [PATCH 4/4] smbd: Flush dfree memcache on service reload
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/smbd/dfree.c | 5 +++++
> source3/smbd/proto.h | 1 +
> source3/smbd/server_reload.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c
> index 8863e58..98aca4e 100644
> --- a/source3/smbd/dfree.c
> +++ b/source3/smbd/dfree.c
> @@ -227,3 +227,8 @@ uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
>
> return dfree_ret;
> }
> +
> +void flush_dfree_cache(void)
> +{
> + memcache_flush(smbd_memcache(), DFREE_CACHE);
> +}
> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index 778561c..399be83 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -173,6 +173,7 @@ uint64_t sys_disk_free(connection_struct *conn, struct smb_filename *fname,
> uint64_t *bsize, uint64_t *dfree, uint64_t *dsize);
> uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> uint64_t *bsize, uint64_t *dfree, uint64_t *dsize);
> +void flush_dfree_cache(void);
>
> /* The following definitions come from smbd/dir.c */
>
> diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
> index c93c077..9b62096 100644
> --- a/source3/smbd/server_reload.c
> +++ b/source3/smbd/server_reload.c
> @@ -164,6 +164,7 @@ bool reload_services(struct smbd_server_connection *sconn,
>
> mangle_reset_cache();
> reset_stat_cache();
> + flush_dfree_cache();
>
> /* this forces service parameters to be flushed */
> set_current_service(NULL,0,True);
> --
> 1.8.3.1
>
More information about the samba-technical
mailing list