diff --git a/drivers/block/blksnap/chunk.c b/drivers/block/blksnap/chunk.c index 83222863d348..73113c714ac1 100644 --- a/drivers/block/blksnap/chunk.c +++ b/drivers/block/blksnap/chunk.c @@ -6,7 +6,6 @@ #include #include "chunk.h" #include "diff_buffer.h" -#include "diff_area.h" #include "diff_storage.h" #include "params.h" @@ -27,35 +26,30 @@ static inline sector_t chunk_sector(struct chunk *chunk) << (chunk->diff_area->chunk_shift - SECTOR_SHIFT); } -void chunk_diff_buffer_release(struct chunk *chunk) -{ - if (unlikely(!chunk->diff_buffer)) - return; - - diff_buffer_release(chunk->diff_area, chunk->diff_buffer); - chunk->diff_buffer = NULL; -} - void chunk_store_failed(struct chunk *chunk, int error) { - struct diff_area *diff_area = chunk->diff_area; + struct diff_area *diff_area = diff_area_get(chunk->diff_area); WARN_ON_ONCE(chunk->state != CHUNK_ST_NEW && chunk->state != CHUNK_ST_IN_MEMORY); chunk->state = CHUNK_ST_FAILED; - chunk_diff_buffer_release(chunk); + if (likely(chunk->diff_buffer)) { + diff_buffer_release(diff_area, chunk->diff_buffer); + chunk->diff_buffer = NULL; + } diff_storage_free_region(chunk->diff_region); chunk->diff_region = NULL; - up(&chunk->lock); + chunk_up(chunk); if (error) diff_area_set_corrupted(diff_area, error); + diff_area_put(diff_area); }; static void chunk_schedule_storing(struct chunk *chunk) { - struct diff_area *diff_area = chunk->diff_area; + struct diff_area *diff_area = diff_area_get(chunk->diff_area); int queue_count; WARN_ON_ONCE(chunk->state != CHUNK_ST_NEW && @@ -67,11 +61,12 @@ static void chunk_schedule_storing(struct chunk *chunk) queue_count = atomic_inc_return(&diff_area->store_queue_count); spin_unlock(&diff_area->store_queue_lock); - up(&chunk->lock); + chunk_up(chunk); /* Initiate the queue clearing process */ if (queue_count > get_chunk_maximum_in_queue()) queue_work(system_wq, &diff_area->store_queue_work); + diff_area_put(diff_area); } void chunk_copy_bio(struct chunk *chunk, struct bio *bio, @@ -193,7 +188,7 @@ static void notify_load_and_schedule_io(struct work_struct *work) continue; } if (chunk->state == CHUNK_ST_FAILED) { - up(&chunk->lock); + chunk_up(chunk); continue; } @@ -217,7 +212,7 @@ static void notify_load_and_postpone_io(struct work_struct *work) continue; } if (chunk->state == CHUNK_ST_FAILED) { - up(&chunk->lock); + chunk_up(chunk); continue; } @@ -243,38 +238,17 @@ static void chunk_notify_store(struct work_struct *work) WARN_ON_ONCE(chunk->state != CHUNK_ST_IN_MEMORY); chunk->state = CHUNK_ST_STORED; - chunk_diff_buffer_release(chunk); - up(&chunk->lock); + if (chunk->diff_buffer) { + diff_buffer_release(chunk->diff_area, + chunk->diff_buffer); + chunk->diff_buffer = NULL; + } + chunk_up(chunk); } bio_put(&cbio->bio); } -struct chunk *chunk_alloc(struct diff_area *diff_area, unsigned long number) -{ - struct chunk *chunk; - - chunk = kzalloc(sizeof(struct chunk), GFP_KERNEL); - if (!chunk) - return NULL; - - INIT_LIST_HEAD(&chunk->link); - sema_init(&chunk->lock, 1); - chunk->diff_area = diff_area; - chunk->number = number; - chunk->state = CHUNK_ST_NEW; - return chunk; -} - -void chunk_free(struct chunk *chunk) -{ - if (unlikely(!chunk)) - return; - chunk_diff_buffer_release(chunk); - diff_storage_free_region(chunk->diff_region); - kfree(chunk); -} - static void chunk_io_endio(struct bio *bio) { struct chunk_bio *cbio = container_of(bio, struct chunk_bio, bio); diff --git a/drivers/block/blksnap/chunk.h b/drivers/block/blksnap/chunk.h index f68bf4f0572f..661cca20b867 100644 --- a/drivers/block/blksnap/chunk.h +++ b/drivers/block/blksnap/chunk.h @@ -7,6 +7,7 @@ #include #include #include +#include "diff_area.h" struct diff_area; struct diff_region; @@ -41,8 +42,6 @@ enum chunk_st { * * @link: * The list header allows to create queue of chunks. - * @diff_area: - * Pointer to the difference area - the storage of changes for a specific device. * @number: * Sequential number of the chunk. * @sector_count: @@ -51,6 +50,10 @@ enum chunk_st { * @lock: * Binary semaphore. Syncs access to the chunks fields: state, * diff_buffer and diff_region. + * @diff_area: + * Pointer to the difference area - the difference storage area for a + * specific device. This field is only available when the chunk is locked. + * Allows to protect the difference area from early release. * @state: * Defines the state of a chunk. * @diff_buffer: @@ -74,22 +77,26 @@ enum chunk_st { */ struct chunk { struct list_head link; - struct diff_area *diff_area; - unsigned long number; sector_t sector_count; struct semaphore lock; + struct diff_area *diff_area; enum chunk_st state; struct diff_buffer *diff_buffer; struct diff_region *diff_region; }; -struct chunk *chunk_alloc(struct diff_area *diff_area, unsigned long number); -void chunk_free(struct chunk *chunk); +static inline void chunk_up(struct chunk *chunk) +{ + struct diff_area *diff_area = chunk->diff_area; + + chunk->diff_area = NULL; + up(&chunk->lock); + diff_area_put(diff_area); +}; -void chunk_diff_buffer_release(struct chunk *chunk); void chunk_store_failed(struct chunk *chunk, int error); void chunk_copy_bio(struct chunk *chunk, struct bio *bio, diff --git a/drivers/block/blksnap/diff_area.c b/drivers/block/blksnap/diff_area.c index 4c6386d0ef8d..5a3f4d74a92f 100644 --- a/drivers/block/blksnap/diff_area.c +++ b/drivers/block/blksnap/diff_area.c @@ -7,7 +7,6 @@ #include #include #include "chunk.h" -#include "diff_area.h" #include "diff_buffer.h" #include "diff_storage.h" #include "params.h" @@ -25,14 +24,14 @@ static inline sector_t chunk_sector(struct chunk *chunk) << (chunk->diff_area->chunk_shift - SECTOR_SHIFT); } -static inline void recalculate_last_chunk_size(struct chunk *chunk) +static inline sector_t last_chunk_size(sector_t sector_count, sector_t capacity) { - sector_t capacity; + sector_t capacity_rounded = round_down(capacity, sector_count); + + if (capacity > capacity_rounded) + sector_count = capacity - capacity_rounded; - capacity = bdev_nr_sectors(chunk->diff_area->orig_bdev); - if (capacity > round_down(capacity, chunk->sector_count)) - chunk->sector_count = - capacity - round_down(capacity, chunk->sector_count); + return sector_count; } static inline unsigned long long count_by_shift(sector_t capacity, @@ -43,6 +42,35 @@ static inline unsigned long long count_by_shift(sector_t capacity, return round_up(capacity, (1ull << shift_sector)) >> shift_sector; } +static inline struct chunk *chunk_alloc(unsigned long number) +{ + struct chunk *chunk; + + chunk = kzalloc(sizeof(struct chunk), GFP_KERNEL); + if (!chunk) + return NULL; + + INIT_LIST_HEAD(&chunk->link); + sema_init(&chunk->lock, 1); + chunk->diff_area = NULL; + chunk->number = number; + chunk->state = CHUNK_ST_NEW; + return chunk; +}; + +static inline void chunk_free(struct diff_area *diff_area, struct chunk *chunk) +{ + if (unlikely(!chunk)) + return; + + down(&chunk->lock); + if (chunk->diff_buffer) + diff_buffer_release(diff_area, chunk->diff_buffer); + diff_storage_free_region(chunk->diff_region); + up(&chunk->lock); + kfree(chunk); +} + static void diff_area_calculate_chunk_size(struct diff_area *diff_area) { unsigned long long count; @@ -79,16 +107,18 @@ static void diff_area_calculate_chunk_size(struct diff_area *diff_area) MINOR(diff_area->orig_bdev->bd_dev)); } -void diff_area_free(struct diff_area *diff_area) +void diff_area_free(struct kref *kref) { unsigned long inx = 0; struct chunk *chunk; + struct diff_area *diff_area = + container_of(kref, struct diff_area, kref); might_sleep(); flush_work(&diff_area->store_queue_work); xa_for_each(&diff_area->chunk_map, inx, chunk) - chunk_free(chunk); + chunk_free(diff_area, chunk); xa_destroy(&diff_area->chunk_map); if (diff_area->orig_bdev) { @@ -112,6 +142,7 @@ static inline bool diff_area_store_one(struct diff_area *diff_area) chunk = iter; atomic_dec(&diff_area->store_queue_count); list_del_init(&chunk->link); + chunk->diff_area = diff_area_get(diff_area); break; } /* @@ -129,7 +160,7 @@ static inline bool diff_area_store_one(struct diff_area *diff_area) * There cannot be a chunk in the store queue whose buffer has * not been read into memory. */ - up(&chunk->lock); + chunk_up(chunk); pr_warn("Cannot release empty buffer for chunk #%ld", chunk->number); return true; @@ -193,6 +224,7 @@ struct diff_area *diff_area_new(dev_t dev_id, struct diff_storage *diff_storage) return ERR_PTR(-ENOMEM); } + kref_init(&diff_area->kref); diff_area->orig_bdev = bdev; diff_area->diff_storage = diff_storage; @@ -225,7 +257,7 @@ struct diff_area *diff_area_new(dev_t dev_id, struct diff_storage *diff_storage) * independently of each other, provided that different chunks are used. */ for (number = 0; number < diff_area->chunk_count; number++) { - chunk = chunk_alloc(diff_area, number); + chunk = chunk_alloc(number); if (!chunk) { pr_err("Failed allocate chunk\n"); ret = -ENOMEM; @@ -237,7 +269,7 @@ struct diff_area *diff_area_new(dev_t dev_id, struct diff_storage *diff_storage) GFP_KERNEL); if (ret) { pr_err("Failed insert chunk to chunk map\n"); - chunk_free(chunk); + chunk_free(diff_area, chunk); break; } } @@ -252,7 +284,8 @@ struct diff_area *diff_area_new(dev_t dev_id, struct diff_storage *diff_storage) return ERR_PTR(ret); } - recalculate_last_chunk_size(chunk); + chunk->sector_count = last_chunk_size(chunk->sector_count, + bdev_nr_sectors(diff_area->orig_bdev)); return diff_area; } @@ -298,6 +331,7 @@ bool diff_area_cow(struct bio *bio, struct diff_area *diff_area, if (unlikely(ret)) goto fail; } + chunk->diff_area = diff_area_get(diff_area); len = chunk_limit(chunk, iter); if (chunk->state == CHUNK_ST_NEW) { @@ -308,7 +342,7 @@ bool diff_area_cow(struct bio *bio, struct diff_area *diff_area, * impossible to process the I/O write unit with * the NOWAIT flag. */ - up(&chunk->lock); + chunk_up(chunk); ret = -EAGAIN; goto fail; } @@ -318,7 +352,7 @@ bool diff_area_cow(struct bio *bio, struct diff_area *diff_area, */ ret = chunk_load_and_postpone_io(chunk, &chunk_bio); if (ret) { - up(&chunk->lock); + chunk_up(chunk); goto fail; } list_add_tail(&chunk->link, &chunks); @@ -330,7 +364,7 @@ bool diff_area_cow(struct bio *bio, struct diff_area *diff_area, * - stored into the diff storage * In this case, we do not change the chunk. */ - up(&chunk->lock); + chunk_up(chunk); } bio_advance_iter_single(bio, iter, len); } @@ -374,6 +408,7 @@ bool diff_area_submit_chunk(struct diff_area *diff_area, struct bio *bio) if (down_killable(&chunk->lock)) return false; + chunk->diff_area = diff_area_get(diff_area); if (unlikely(chunk->state == CHUNK_ST_FAILED)) { pr_err("Chunk #%ld corrupted\n", chunk->number); @@ -381,7 +416,7 @@ bool diff_area_submit_chunk(struct diff_area *diff_area, struct bio *bio) bio->bi_iter.bi_sector, (1Ull << diff_area->chunk_shift), diff_area->chunk_count); - up(&chunk->lock); + chunk_up(chunk); return false; } if (chunk->state == CHUNK_ST_IN_MEMORY) { @@ -390,7 +425,7 @@ bool diff_area_submit_chunk(struct diff_area *diff_area, struct bio *bio) * copy to the in-memory chunk for write operation. */ chunk_copy_bio(chunk, bio, &bio->bi_iter); - up(&chunk->lock); + chunk_up(chunk); return true; } if ((chunk->state == CHUNK_ST_STORED) || !op_is_write(bio_op(bio))) { @@ -398,7 +433,7 @@ bool diff_area_submit_chunk(struct diff_area *diff_area, struct bio *bio) * Read data from the chunk on difference storage. */ chunk_clone_bio(chunk, bio); - up(&chunk->lock); + chunk_up(chunk); return true; } /* @@ -407,7 +442,7 @@ bool diff_area_submit_chunk(struct diff_area *diff_area, struct bio *bio) * in-memory chunk. */ if (chunk_load_and_schedule_io(chunk, bio)) { - up(&chunk->lock); + chunk_up(chunk); return false; } return true; diff --git a/drivers/block/blksnap/diff_area.h b/drivers/block/blksnap/diff_area.h index 61b609b66990..cb3e09809c0f 100644 --- a/drivers/block/blksnap/diff_area.h +++ b/drivers/block/blksnap/diff_area.h @@ -87,6 +87,7 @@ struct chunk; * */ struct diff_area { + struct kref kref; struct block_device *orig_bdev; struct diff_storage *diff_storage; @@ -112,7 +113,16 @@ struct diff_area { struct diff_area *diff_area_new(dev_t dev_id, struct diff_storage *diff_storage); -void diff_area_free(struct diff_area *diff_area); +void diff_area_free(struct kref *kref); +static inline struct diff_area *diff_area_get(struct diff_area *diff_area) +{ + kref_get(&diff_area->kref); + return diff_area; +}; +static inline void diff_area_put(struct diff_area *diff_area) +{ + kref_put(&diff_area->kref, diff_area_free); +}; void diff_area_set_corrupted(struct diff_area *diff_area, int err_code); static inline bool diff_area_is_corrupted(struct diff_area *diff_area) diff --git a/drivers/block/blksnap/snapimage.c b/drivers/block/blksnap/snapimage.c index 328abb376780..6bffdb9e1ac7 100644 --- a/drivers/block/blksnap/snapimage.c +++ b/drivers/block/blksnap/snapimage.c @@ -11,7 +11,6 @@ #include #include "snapimage.h" #include "tracker.h" -#include "diff_area.h" #include "chunk.h" #include "cbt_map.h" @@ -26,6 +25,11 @@ static void snapimage_submit_bio(struct bio *bio) struct tracker *tracker = bio->bi_bdev->bd_disk->private_data; struct diff_area *diff_area = tracker->diff_area; + /* + * The diff_area is not blocked from releasing now, because + * snapimage_free() is calling before diff_area_put() in + * tracker_release_snapshot(). + */ if (diff_area_is_corrupted(diff_area)) { bio_io_error(bio); return; diff --git a/drivers/block/blksnap/snapshot.c b/drivers/block/blksnap/snapshot.c index 2f2108bb23b6..1f28ff59d762 100644 --- a/drivers/block/blksnap/snapshot.c +++ b/drivers/block/blksnap/snapshot.c @@ -278,7 +278,14 @@ static int snapshot_take_trackers(struct snapshot *snapshot) MAJOR(tracker->dev_id), MINOR(tracker->dev_id)); } fail: - + if (ret) { + list_for_each_entry(tracker, &snapshot->trackers, link) { + if (tracker->diff_area) { + diff_area_put(tracker->diff_area); + tracker->diff_area = NULL; + } + } + } up_write(&snapshot->rw_lock); return ret; } diff --git a/drivers/block/blksnap/tracker.c b/drivers/block/blksnap/tracker.c index 3f6586b86f24..6d2c77e4c90f 100644 --- a/drivers/block/blksnap/tracker.c +++ b/drivers/block/blksnap/tracker.c @@ -45,8 +45,14 @@ static bool tracker_submit_bio(struct bio *bio) copy_iter.bi_sector -= bio->bi_bdev->bd_start_sect; if (cbt_map_set(tracker->cbt_map, copy_iter.bi_sector, count) || - !atomic_read(&tracker->snapshot_is_taken) || - diff_area_is_corrupted(tracker->diff_area)) + !atomic_read(&tracker->snapshot_is_taken)) + return false; + /* + * The diff_area is not blocked from releasing now, because + * changing the value of the snapshot_is_taken is performed when + * the block device queue is frozen in tracker_release_snapshot(). + */ + if (diff_area_is_corrupted(tracker->diff_area)) return false; return diff_area_cow(bio, tracker->diff_area, ©_iter); @@ -287,22 +293,24 @@ int tracker_take_snapshot(struct tracker *tracker) void tracker_release_snapshot(struct tracker *tracker) { - if (tracker->diff_area) { - blk_mq_freeze_queue(tracker->diff_area->orig_bdev->bd_queue); - - pr_debug("Tracker for device [%u:%u] release snapshot\n", - MAJOR(tracker->dev_id), MINOR(tracker->dev_id)); + struct diff_area *diff_area = tracker->diff_area; - atomic_set(&tracker->snapshot_is_taken, false); + if (unlikely(!diff_area)) + return; - blk_mq_unfreeze_queue(tracker->diff_area->orig_bdev->bd_queue); - } snapimage_free(tracker); - if (tracker->diff_area) { - diff_area_free(tracker->diff_area); - tracker->diff_area = NULL; - } + blk_mq_freeze_queue(diff_area->orig_bdev->bd_queue); + + pr_debug("Tracker for device [%u:%u] release snapshot\n", + MAJOR(tracker->dev_id), MINOR(tracker->dev_id)); + + atomic_set(&tracker->snapshot_is_taken, false); + tracker->diff_area = NULL; + + blk_mq_unfreeze_queue(diff_area->orig_bdev->bd_queue); + + diff_area_put(diff_area); } int __init tracker_init(void)