Skip to content

Commit

Permalink
MDCACHE - Use atomics for readdir flags
Browse files Browse the repository at this point in the history
Although the write lock is always held when we return from
mdcache_populate_dir_chunk(), the readdir loop can terminate early, if
the client buffer doesn't have enough space.  This could result in
multiple readdirs processing the same chunk under the read lock, which
can deref an entry twice.

Switch to using atomics on this flag field.

Change-Id: Ibd59283b274422ee6f2477066389e6e43746fe6f
Signed-off-by: Daniel Gryniewicz <dang@redhat.com>
  • Loading branch information
dang authored and ffilz committed Feb 28, 2019
1 parent cd9ac94 commit 23c05a5
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
11 changes: 8 additions & 3 deletions src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c
Expand Up @@ -2288,6 +2288,8 @@ mdc_readdir_chunk_object(const char *name, struct fsal_obj_handle *sub_handle,
state->cur_chunk = chunk;
if (new_dir_entry->flags & DIR_ENTRY_REFFED) {
/* This was ref'd already; drop extra ref */
/* Note, we have the write lock here, so atomics
* are unnecessary */
mdcache_put(new_entry);
new_dir_entry->flags &= ~DIR_ENTRY_REFFED;
}
Expand Down Expand Up @@ -2326,7 +2328,8 @@ mdc_readdir_chunk_object(const char *name, struct fsal_obj_handle *sub_handle,
atomic_fetch_int32_t(&new_entry->lru.refcnt));

/* Note that this entry is ref'd, so that mdcache_readdir_chunked can
* un-ref it. Keep the ref from above for this purpose */
* un-ref it. Keep the ref from above for this purpose. We have the
* write lock, so atomics are unnecessary */
new_dir_entry->flags |= DIR_ENTRY_REFFED;


Expand Down Expand Up @@ -2953,6 +2956,7 @@ fsal_status_t mdcache_readdir_chunked(mdcache_entry_t *directory,
enum fsal_dir_result cb_result;
mdcache_entry_t *entry = NULL;
struct attrlist attrs;
uint32_t flags;

if (dirent->flags & DIR_ENTRY_FLAG_DELETED) {
/* Skip deleted entries */
Expand Down Expand Up @@ -3042,9 +3046,10 @@ fsal_status_t mdcache_readdir_chunked(mdcache_entry_t *directory,

/* If we get here, we got an entry, and took a ref on it. Put
* the saved ref from the mdc_readdir_chunk_object(), if any. */
if (dirent->flags & DIR_ENTRY_REFFED) {
flags = atomic_postclear_uint32_t_bits(&dirent->flags,
DIR_ENTRY_REFFED);
if (flags & DIR_ENTRY_REFFED) {
mdcache_put(entry);
dirent->flags &= ~DIR_ENTRY_REFFED;
}

if (reload_chunk && look_ck != 0 && dirent->ck !=
Expand Down
3 changes: 2 additions & 1 deletion src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h
Expand Up @@ -382,7 +382,8 @@ typedef struct mdcache_dir_entry__ {
uint64_t namehash;
/** Key of cache entry */
mdcache_key_t ckey;
/** Flags */
/** Flags
* Protected by write lock or atomics. */
uint32_t flags;
const char *name;
/** The NUL-terminated filename */
Expand Down

0 comments on commit 23c05a5

Please sign in to comment.