From afb7ab979ce78e5e2850881b01628c4628e3b790 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Apr 2015 11:19:22 -0700 Subject: [PATCH 1/2] lib: memcache. Add a new talloc cache type - SHARE_MODE_LOCK_CACHE. Signed-off-by: Jeremy Allison --- lib/util/memcache.c | 1 + lib/util/memcache.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/util/memcache.c b/lib/util/memcache.c index 2f2e77c..9e9a208 100644 --- a/lib/util/memcache.c +++ b/lib/util/memcache.c @@ -53,6 +53,7 @@ static bool memcache_is_talloc(enum memcache_number n) case GETPWNAM_CACHE: case PDB_GETPWSID_CACHE: case SINGLETON_CACHE_TALLOC: + case SHARE_MODE_LOCK_CACHE: result = true; break; default: diff --git a/lib/util/memcache.h b/lib/util/memcache.h index 97490b9..2602fb7 100644 --- a/lib/util/memcache.h +++ b/lib/util/memcache.h @@ -40,7 +40,8 @@ enum memcache_number { PDB_GETPWSID_CACHE, /* talloc */ SINGLETON_CACHE_TALLOC, /* talloc */ SINGLETON_CACHE, - SMB1_SEARCH_OFFSET_MAP + SMB1_SEARCH_OFFSET_MAP, + SHARE_MODE_LOCK_CACHE /* talloc */ }; /* -- 2.2.0.rc0.207.ga3a616c From 63519e808a66cf926ced7e9ad12fc6694d61e51a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 15 Apr 2015 10:36:00 -0700 Subject: [PATCH 2/2] s3: locking: Add a memcache based lock cache. Based on an idea by Volker to optimize cpu usage when parsing struct share_mode_data entries. Add a 64-bit sequence number to the share mode entry, and after the entry is stored back in the db, cache the in-memory version using talloc reparenting into the memcache. The memcache key used is the same struct file_id used as the key into the locking db. On read, check if the locking db version sequence number matches the in-memory cache, and if so reparent the memcache version back onto the required memory context. Saves all the ndr decoding when multiple accesses to the same lock entry happen in succession. Design also improved by Metze and Ira. Signed-off-by: Jeremy Allison --- source3/librpc/idl/open_files.idl | 2 + source3/locking/share_mode_lock.c | 208 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 201 insertions(+), 9 deletions(-) diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl index 0a9d5fd..6f74340 100644 --- a/source3/librpc/idl/open_files.idl +++ b/source3/librpc/idl/open_files.idl @@ -72,6 +72,7 @@ interface open_files } delete_token; typedef [public] struct { + hyper sequence_number; [string,charset(UTF8)] char *servicepath; [string,charset(UTF8)] char *base_name; [string,charset(UTF8)] char *stream_name; @@ -86,6 +87,7 @@ interface open_files [skip] boolean8 fresh; [skip] boolean8 modified; [ignore] db_record *record; + [ignore] file_id id; /* In memory key used to lookup cache. */ } share_mode_data; /* these are 0x30 (48) characters */ diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c index 138950f..01e6241 100644 --- a/source3/locking/share_mode_lock.c +++ b/source3/locking/share_mode_lock.c @@ -48,6 +48,7 @@ #include "../librpc/gen_ndr/ndr_open_files.h" #include "source3/lib/dbwrap/dbwrap_watch.h" #include "locking/leases_db.h" +#include "../lib/util/memcache.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_LOCKING @@ -121,26 +122,192 @@ static TDB_DATA locking_key(const struct file_id *id) } /******************************************************************* + Share mode cache utility functions that store/delete/retrieve + entries from memcache. + + For now share the statcache (global cache) memory space. If + a lock record gets orphaned (which shouldn't happen as we're + using the same locking_key data as lookup) it will eventually + fall out of the cache via the normal LRU trim mechanism. If + necessary we can always make this a separate (smaller) cache. +******************************************************************/ + +static const DATA_BLOB memcache_key(const struct file_id *id) +{ + return data_blob_const((const void *)id, sizeof(*id)); +} + +static void share_mode_memcache_delete(struct share_mode_data *d) +{ + const DATA_BLOB key = memcache_key(&d->id); + + DEBUG(10,("deleting entry for file %s seq 0x%llu key %s\n", + d->base_name, + (unsigned long long) d->sequence_number, + file_id_string(talloc_tos(), &d->id))); + + memcache_delete(NULL, + SHARE_MODE_LOCK_CACHE, + key); +} + +static void share_mode_memcache_store(struct share_mode_data *d) +{ + const DATA_BLOB key = memcache_key(&d->id); + + DEBUG(10,("stored entry for file %s seq 0x%llu key %s\n", + d->base_name, + (unsigned long long) d->sequence_number, + file_id_string(talloc_tos(), &d->id))); + + /* Ensure everything stored in the cache is pristine. */ + d->modified = false; + d->fresh = false; + + /* + * Ensure the memory going into the cache + * doesn't have a destructor so it can be + * cleanly freed by share_mode_memcache_delete(). + */ + talloc_set_destructor(d, NULL); + + /* Cache will own d after this call. */ + memcache_add_talloc(NULL, + SHARE_MODE_LOCK_CACHE, + key, + &d); +} + +/* + * NB. We use ndr_pull_hyper on a stack-created + * struct ndr_pull with no talloc allowed, as we + * need this to be really fast as an ndr-peek into + * the first 8 bytes of the blob. + */ + +static enum ndr_err_code get_blob_sequence_number(DATA_BLOB *blob, + uint64_t *pseq) +{ + struct ndr_pull ndr; + memset(&ndr, '\0', sizeof(struct ndr_pull)); + ndr.data = blob->data; + ndr.data_size = blob->length; + NDR_CHECK(ndr_pull_hyper(&ndr, NDR_SCALARS, pseq)); + return NDR_ERR_SUCCESS; +} + +static int share_mode_data_nofree_destructor(struct share_mode_data *d) +{ + return -1; +} + +static struct share_mode_data *share_mode_memcache_fetch(TALLOC_CTX *mem_ctx, + const TDB_DATA id_key, + DATA_BLOB *blob) +{ + enum ndr_err_code ndr_err; + struct share_mode_data *d; + uint64_t sequence_number; + void *ptr; + struct file_id id; + DATA_BLOB key; + + /* Ensure this is a locking_key record. */ + if (id_key.dsize != sizeof(id)) { + return NULL; + } + + memcpy(&id, id_key.dptr, id_key.dsize); + key = memcache_key(&id); + + ptr = memcache_lookup_talloc(NULL, + SHARE_MODE_LOCK_CACHE, + key); + if (ptr == NULL) { + DEBUG(10,("failed to find entry for key %s\n", + file_id_string(mem_ctx, &id))); + return NULL; + } + /* sequence number key is at start of blob. */ + ndr_err = get_blob_sequence_number(blob, &sequence_number); + if (ndr_err != NDR_ERR_SUCCESS) { + /* Bad blob. Remove entry. */ + DEBUG(10,("bad blob %u key %s\n", + (unsigned int)ndr_err, + file_id_string(mem_ctx, &id))); + memcache_delete(NULL, + SHARE_MODE_LOCK_CACHE, + key); + return NULL; + } + + d = (struct share_mode_data *)ptr; + if (d->sequence_number != sequence_number) { + DEBUG(10,("seq changed (cached 0x%llu) (new 0x%llu) " + "for key %s\n", + (unsigned long long)d->sequence_number, + (unsigned long long)sequence_number, + file_id_string(mem_ctx, &id))); + /* Cache out of date. Remove entry. */ + memcache_delete(NULL, + SHARE_MODE_LOCK_CACHE, + key); + return NULL; + } + + /* Move onto mem_ctx. */ + d = talloc_move(mem_ctx, &ptr); + + /* + * Now we own d, prevent the cache from freeing it + * when we delete the entry. + */ + talloc_set_destructor(d, share_mode_data_nofree_destructor); + + /* Remove from the cache. We own it now. */ + memcache_delete(NULL, + SHARE_MODE_LOCK_CACHE, + key); + + /* And reset the destructor to none. */ + talloc_set_destructor(d, NULL); + + DEBUG(10,("fetched entry for file %s seq 0x%llu key %s\n", + d->base_name, + (unsigned long long)d->sequence_number, + file_id_string(mem_ctx, &id))); + + return d; +} + +/******************************************************************* Get all share mode entries for a dev/inode pair. ********************************************************************/ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx, - const TDB_DATA dbuf) + const TDB_DATA key, + const TDB_DATA dbuf) { struct share_mode_data *d; enum ndr_err_code ndr_err; uint32_t i; DATA_BLOB blob; + blob.data = dbuf.dptr; + blob.length = dbuf.dsize; + + /* See if we already have a cached copy of this key. */ + d = share_mode_memcache_fetch(mem_ctx, key, &blob); + if (d != NULL) { + return d; + } + d = talloc(mem_ctx, struct share_mode_data); if (d == NULL) { DEBUG(0, ("talloc failed\n")); goto fail; } - blob.data = dbuf.dptr; - blob.length = dbuf.dsize; - ndr_err = ndr_pull_struct_blob_all( &blob, d, d, (ndr_pull_flags_fn_t)ndr_pull_share_mode_data); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { @@ -195,6 +362,11 @@ static TDB_DATA unparse_share_modes(struct share_mode_data *d) NDR_PRINT_DEBUG(share_mode_data, d); } + share_mode_memcache_delete(d); + + /* Update the sequence number. */ + d->sequence_number += 1; + remove_stale_share_mode_entries(d); if (d->num_share_modes == 0) { @@ -246,7 +418,11 @@ static int share_mode_data_destructor(struct share_mode_data *d) smb_panic(errmsg); } } - goto done; + /* + * Nothing to store in cache - allow the normal + * release of record lock and memory free. + */ + return 0; } status = dbwrap_record_store(d->record, data, TDB_REPLACE); @@ -262,9 +438,19 @@ static int share_mode_data_destructor(struct share_mode_data *d) smb_panic(errmsg); } - done: + /* + * Release the record lock before putting in the cache. + */ + TALLOC_FREE(d->record); - return 0; + /* + * Reparent d into the in-memory cache so it can be reused if the + * sequence number matches. See parse_share_modes() + * for details. + */ + + share_mode_memcache_store(d); + return -1; } /******************************************************************* @@ -288,6 +474,9 @@ static struct share_mode_data *fresh_share_mode_lock( if (d == NULL) { goto fail; } + /* New record - new sequence number. */ + generate_random_buffer((uint8_t *)&d->sequence_number, 8); + d->base_name = talloc_strdup(d, smb_fname->base_name); if (d->base_name == NULL) { goto fail; @@ -340,7 +529,7 @@ static struct share_mode_lock *get_share_mode_lock_internal( d = fresh_share_mode_lock(mem_ctx, servicepath, smb_fname, old_write_time); } else { - d = parse_share_modes(mem_ctx, value); + d = parse_share_modes(mem_ctx, key, value); } if (d == NULL) { @@ -349,6 +538,7 @@ static struct share_mode_lock *get_share_mode_lock_internal( TALLOC_FREE(rec); return NULL; } + d->id = id; d->record = talloc_move(d, &rec); talloc_set_destructor(d, share_mode_data_destructor); @@ -428,7 +618,7 @@ static void fetch_share_mode_unlocked_parser( struct share_mode_lock *lck = talloc_get_type_abort( private_data, struct share_mode_lock); - lck->data = parse_share_modes(lck, data); + lck->data = parse_share_modes(lck, key, data); } /******************************************************************* -- 2.2.0.rc0.207.ga3a616c