lkml.org 
[lkml]   [2018]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 35/41] btrfs: Fix memory barriers usage with device stats counters
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Nikolay Borisov <nborisov@suse.com>

    commit 9deae9689231964972a94bb56a79b669f9d47ac1 upstream.

    Commit addc3fa74e5b ("Btrfs: Fix the problem that the dirty flag of dev
    stats is cleared") reworked the way device stats changes are tracked. A
    new atomic dev_stats_ccnt counter was introduced which is incremented
    every time any of the device stats counters are changed. This serves as
    a flag whether there are any pending stats changes. However, this patch
    only partially implemented the correct memory barriers necessary:

    - It only ordered the stores to the counters but not the reads e.g.
    btrfs_run_dev_stats
    - It completely omitted any comments documenting the intended design and
    how the memory barriers pair with each-other

    This patch provides the necessary comments as well as adds a missing
    smp_rmb in btrfs_run_dev_stats. Furthermore since dev_stats_cnt is only
    a snapshot at best there was no point in reading the counter twice -
    once in btrfs_dev_stats_dirty and then again when assigning stats_cnt.
    Just collapse both reads into 1.

    Fixes: addc3fa74e5b ("Btrfs: Fix the problem that the dirty flag of dev stats is cleared")
    Signed-off-by: Nikolay Borisov <nborisov@suse.com>
    Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    Reviewed-by: David Sterba <dsterba@suse.com>
    Signed-off-by: David Sterba <dsterba@suse.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/btrfs/volumes.c | 18 ++++++++++++++++--
    fs/btrfs/volumes.h | 12 ++++++++++++
    2 files changed, 28 insertions(+), 2 deletions(-)

    --- a/fs/btrfs/volumes.c
    +++ b/fs/btrfs/volumes.c
    @@ -7082,10 +7082,24 @@ int btrfs_run_dev_stats(struct btrfs_tra

    mutex_lock(&fs_devices->device_list_mutex);
    list_for_each_entry(device, &fs_devices->devices, dev_list) {
    - if (!device->dev_stats_valid || !btrfs_dev_stats_dirty(device))
    + stats_cnt = atomic_read(&device->dev_stats_ccnt);
    + if (!device->dev_stats_valid || stats_cnt == 0)
    continue;

    - stats_cnt = atomic_read(&device->dev_stats_ccnt);
    +
    + /*
    + * There is a LOAD-LOAD control dependency between the value of
    + * dev_stats_ccnt and updating the on-disk values which requires
    + * reading the in-memory counters. Such control dependencies
    + * require explicit read memory barriers.
    + *
    + * This memory barriers pairs with smp_mb__before_atomic in
    + * btrfs_dev_stat_inc/btrfs_dev_stat_set and with the full
    + * barrier implied by atomic_xchg in
    + * btrfs_dev_stats_read_and_reset
    + */
    + smp_rmb();
    +
    ret = update_dev_stat_item(trans, fs_info, device);
    if (!ret)
    atomic_sub(stats_cnt, &device->dev_stats_ccnt);
    --- a/fs/btrfs/volumes.h
    +++ b/fs/btrfs/volumes.h
    @@ -498,6 +498,12 @@ static inline void btrfs_dev_stat_inc(st
    int index)
    {
    atomic_inc(dev->dev_stat_values + index);
    + /*
    + * This memory barrier orders stores updating statistics before stores
    + * updating dev_stats_ccnt.
    + *
    + * It pairs with smp_rmb() in btrfs_run_dev_stats().
    + */
    smp_mb__before_atomic();
    atomic_inc(&dev->dev_stats_ccnt);
    }
    @@ -523,6 +529,12 @@ static inline void btrfs_dev_stat_set(st
    int index, unsigned long val)
    {
    atomic_set(dev->dev_stat_values + index, val);
    + /*
    + * This memory barrier orders stores updating statistics before stores
    + * updating dev_stats_ccnt.
    + *
    + * It pairs with smp_rmb() in btrfs_run_dev_stats().
    + */
    smp_mb__before_atomic();
    atomic_inc(&dev->dev_stats_ccnt);
    }

    \
     
     \ /
      Last update: 2018-03-19 19:54    [W:4.083 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site