mirror of
https://github.com/torvalds/linux.git
synced 2026-09-11 20:13:02 +02:00
dm cache: fix issue with background work locking
dm cache used a rw_semaphore for background_work_lock. Write locks on
rw_semaphores have strict owner semantics, but there was no guarantee
that the process that locked background_work_lock was the same process
that unlocked it. This can be easily seen using a kernel compiled with
CONFIG_DEBUG_RWSEMS. Given a dm cache device <cache>, run: 'dmsetup
suspend <cache> && dmsetup resume <cache>'. This will trigger a kernel
warning:
DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) && !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE))
triggered by cache_resume(). To fix this, switch from a rw_semaphore to
a spinlock and a wait queue. dm cache already has a wait queue and
associated counter, migration_wait and nr_allocated_migrations, that was
getting woken up when background work was getting completed, but wasn't
actually used by anything. This is replaced by the background_work queue
and counter.
Fixes: b29d4986d0 ("dm cache: significant rework to leverage dm-bio-prison-v2")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Matthew Sakai <msakai@redhat.com>
Reviewed-by: Ming-Hung Tsai <mtsai@redhat.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
This commit is contained in:
parent
148845aa19
commit
bc9781c024
|
|
@ -340,8 +340,6 @@ struct cache {
|
|||
struct list_head invalidation_requests;
|
||||
|
||||
sector_t migration_threshold;
|
||||
wait_queue_head_t migration_wait;
|
||||
atomic_t nr_allocated_migrations;
|
||||
|
||||
/*
|
||||
* The number of in flight migrations that are performing
|
||||
|
|
@ -397,7 +395,11 @@ struct cache {
|
|||
bool loaded_mappings:1;
|
||||
bool loaded_discards:1;
|
||||
|
||||
struct rw_semaphore background_work_lock;
|
||||
/* background work management */
|
||||
bool background_work_allowed;
|
||||
unsigned background_work_nr;
|
||||
spinlock_t background_work_lock;
|
||||
wait_queue_head_t background_work_wait;
|
||||
|
||||
struct batcher committer;
|
||||
struct work_struct commit_ws;
|
||||
|
|
@ -488,19 +490,13 @@ static struct dm_cache_migration *alloc_migration(struct cache *cache)
|
|||
memset(mg, 0, sizeof(*mg));
|
||||
|
||||
mg->cache = cache;
|
||||
atomic_inc(&cache->nr_allocated_migrations);
|
||||
|
||||
return mg;
|
||||
}
|
||||
|
||||
static void free_migration(struct dm_cache_migration *mg)
|
||||
{
|
||||
struct cache *cache = mg->cache;
|
||||
|
||||
if (atomic_dec_and_test(&cache->nr_allocated_migrations))
|
||||
wake_up(&cache->migration_wait);
|
||||
|
||||
mempool_free(mg, &cache->migration_pool);
|
||||
mempool_free(mg, &mg->cache->migration_pool);
|
||||
}
|
||||
|
||||
/*----------------------------------------------------------------*/
|
||||
|
|
@ -1030,34 +1026,39 @@ static void calc_discard_block_range(struct cache *cache, struct bio *bio,
|
|||
|
||||
static void prevent_background_work(struct cache *cache)
|
||||
{
|
||||
lockdep_off();
|
||||
down_write(&cache->background_work_lock);
|
||||
lockdep_on();
|
||||
spin_lock_irq(&cache->background_work_lock);
|
||||
cache->background_work_allowed = false;
|
||||
wait_event_lock_irq(cache->background_work_wait,
|
||||
cache->background_work_nr == 0,
|
||||
cache->background_work_lock);
|
||||
spin_unlock_irq(&cache->background_work_lock);
|
||||
}
|
||||
|
||||
static void allow_background_work(struct cache *cache)
|
||||
{
|
||||
lockdep_off();
|
||||
up_write(&cache->background_work_lock);
|
||||
lockdep_on();
|
||||
spin_lock_irq(&cache->background_work_lock);
|
||||
cache->background_work_allowed = true;
|
||||
spin_unlock_irq(&cache->background_work_lock);
|
||||
}
|
||||
|
||||
static bool background_work_begin(struct cache *cache)
|
||||
{
|
||||
bool r;
|
||||
|
||||
lockdep_off();
|
||||
r = down_read_trylock(&cache->background_work_lock);
|
||||
lockdep_on();
|
||||
|
||||
spin_lock_irq(&cache->background_work_lock);
|
||||
r = cache->background_work_allowed;
|
||||
if (r)
|
||||
cache->background_work_nr++;
|
||||
spin_unlock_irq(&cache->background_work_lock);
|
||||
return r;
|
||||
}
|
||||
|
||||
static void background_work_end(struct cache *cache)
|
||||
{
|
||||
lockdep_off();
|
||||
up_read(&cache->background_work_lock);
|
||||
lockdep_on();
|
||||
spin_lock_irq(&cache->background_work_lock);
|
||||
if (--cache->background_work_nr == 0)
|
||||
wake_up(&cache->background_work_wait);
|
||||
spin_unlock_irq(&cache->background_work_lock);
|
||||
}
|
||||
|
||||
/*----------------------------------------------------------------*/
|
||||
|
|
@ -2507,9 +2508,7 @@ static int cache_create(struct cache_args *ca, struct cache **result)
|
|||
|
||||
spin_lock_init(&cache->lock);
|
||||
bio_list_init(&cache->deferred_bios);
|
||||
atomic_set(&cache->nr_allocated_migrations, 0);
|
||||
atomic_set(&cache->nr_io_migrations, 0);
|
||||
init_waitqueue_head(&cache->migration_wait);
|
||||
|
||||
r = -ENOMEM;
|
||||
atomic_set(&cache->nr_dirty, 0);
|
||||
|
|
@ -2592,8 +2591,10 @@ static int cache_create(struct cache_args *ca, struct cache **result)
|
|||
issue_op, cache, cache->wq);
|
||||
dm_iot_init(&cache->tracker);
|
||||
|
||||
init_rwsem(&cache->background_work_lock);
|
||||
prevent_background_work(cache);
|
||||
init_waitqueue_head(&cache->background_work_wait);
|
||||
spin_lock_init(&cache->background_work_lock);
|
||||
cache->background_work_allowed = false;
|
||||
cache->background_work_nr = 0;
|
||||
|
||||
*result = cache;
|
||||
return 0;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user