linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergei Shtepa <sergei.shtepa@veeam.com>
To: Donald Buczek <buczek@molgen.mpg.de>, <axboe@kernel.dk>,
	<hch@infradead.org>, <corbet@lwn.net>, <snitzer@kernel.org>
Cc: <viro@zeniv.linux.org.uk>, <brauner@kernel.org>,
	<willy@infradead.org>, <kch@nvidia.com>,
	<martin.petersen@oracle.com>, <vkoul@kernel.org>,
	<ming.lei@redhat.com>, <gregkh@linuxfoundation.org>,
	<linux-block@vger.kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v3 03/11] documentation: Block Devices Snapshots Module
Date: Tue, 18 Apr 2023 12:31:16 +0200	[thread overview]
Message-ID: <a1854604-cec1-abd5-1d49-6cf6a19ee7a1@veeam.com> (raw)
In-Reply-To: <86068780-bab3-2fc2-3f6f-1868be119b38@veeam.com>

[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]



On 4/14/23 14:34, Sergei Shtepa wrote:
> Subject:
> Re: [PATCH v3 03/11] documentation: Block Devices Snapshots Module
> From:
> Sergei Shtepa <sergei.shtepa@veeam.com>
> Date:
> 4/14/23, 14:34
> 
> To:
> Donald Buczek <buczek@molgen.mpg.de>, axboe@kernel.dk, hch@infradead.org, corbet@lwn.net, snitzer@kernel.org
> CC:
> viro@zeniv.linux.org.uk, brauner@kernel.org, willy@infradead.org, kch@nvidia.com, martin.petersen@oracle.com, vkoul@kernel.org, ming.lei@redhat.com, gregkh@linuxfoundation.org, linux-block@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
> 
> 
> 
> On 4/12/23 21:38, Donald Buczek wrote:
>> Subject:
>> Re: [PATCH v3 03/11] documentation: Block Devices Snapshots Module
>> From:
>> Donald Buczek <buczek@molgen.mpg.de>
>> Date:
>> 4/12/23, 21:38
>>
>> To:
>> Sergei Shtepa <sergei.shtepa@veeam.com>, axboe@kernel.dk, hch@infradead.org, corbet@lwn.net, snitzer@kernel.org
>> CC:
>> viro@zeniv.linux.org.uk, brauner@kernel.org, willy@infradead.org, kch@nvidia.com, martin.petersen@oracle.com, vkoul@kernel.org, ming.lei@redhat.com, gregkh@linuxfoundation.org, linux-block@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
>>
>>
>> I think, you can trigger all kind of user-after-free when userspace deletes a snapshot image or the snapshot image and the tracker while the disk device snapshot image is kept alive (mounted or just opened) and doing I/O.
>>
>> Here is what I did to provoke that:
>>
>> root@dose:~# s=$(blksnap snapshot_create -d /dev/vdb)
>> root@dose:~# blksnap snapshot_appendstorage -i $s -f /scratch/local/test.dat
>> device path: '/dev/block/253:2'
>> allocate range: ofs=11264624 cnt=2097152
>> root@dose:~# blksnap snapshot_take -i $s
>> root@dose:~# mount /dev/blksnap-image_253\:16 /mnt
>> root@dose:~# dd if=/dev/zero of=/mnt/x.x &
>> [1] 2514
>> root@dose:~# blksnap snapshot_destroy -i $s
>> dd: writing to '/mnt/x.x': No space left on device
>> 1996041+0 records in
>> 1996040+0 records out
>> 1021972480 bytes (1.0 GB, 975 MiB) copied, 8.48923 s, 120 MB/s
>> [1]+  Exit 1                  dd if=/dev/zero of=/mnt/x.x
>>
> Thanks!
> I am very glad that the blksnap tool turned out to be useful in the review.
> This snapshot deletion scenario is not the most typical, but of course it is
> quite possible.
> I will need to solve this problem and add such a scenario to the test suite.
> 

Hi!

I have redesign the logic of ownership of the diff_area structure.
See patch in attach or commit.
Link: https://github.com/SergeiShtepa/linux/commit/7e927c381dcd2b2293be8315897a224d111b6f88
A test script for such a scenario has been added.
Link: https://github.com/veeam/blksnap/commit/fd0559dfedf094901d08bbf185fed288f0156433

I will be glad of any feedback.

[-- Attachment #2: fix_diff_area_ownership.patch --]
[-- Type: text/x-patch, Size: 16723 bytes --]

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 <linux/slab.h>
 #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 <linux/blkdev.h>
 #include <linux/rwsem.h>
 #include <linux/atomic.h>
+#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 <linux/build_bug.h>
 #include <uapi/linux/blksnap.h>
 #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 <uapi/linux/blksnap.h>
 #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, &copy_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)

  reply	other threads:[~2023-04-18 10:31 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04 14:08 [PATCH v3 00/11] blksnap - block devices snapshots module Sergei Shtepa
2023-04-04 14:08 ` [PATCH v3 01/11] documentation: Block Device Filtering Mechanism Sergei Shtepa
2023-04-10  5:04   ` Bagas Sanjaya
2023-04-12 12:39     ` Sergei Shtepa
2023-04-04 14:08 ` [PATCH v3 02/11] block: " Sergei Shtepa
2023-04-08 15:16   ` Donald Buczek
2023-04-11  6:23     ` Christoph Hellwig
2023-04-08 15:30   ` Donald Buczek
2023-04-11  6:25     ` Christoph Hellwig
2023-04-12 10:43       ` Sergei Shtepa
2023-04-12 19:59         ` Donald Buczek
2023-04-14 13:39           ` Sergei Shtepa
2023-04-16  6:06         ` Christoph Hellwig
2023-04-04 14:08 ` [PATCH v3 03/11] documentation: Block Devices Snapshots Module Sergei Shtepa
2023-04-10  5:01   ` Bagas Sanjaya
2023-04-12 19:38   ` Donald Buczek
2023-04-14 12:34     ` Sergei Shtepa
2023-04-18 10:31       ` Sergei Shtepa [this message]
2023-04-18 14:48         ` Donald Buczek
2023-04-19 13:05           ` Sergei Shtepa
2023-04-19 19:42             ` Donald Buczek
2023-04-20 14:44               ` Donald Buczek
2023-04-20 19:17                 ` Sergei Shtepa
2023-04-21 17:32                   ` Sergei Shtepa
2023-04-22 20:01                     ` Donald Buczek
2023-04-04 14:08 ` [PATCH v3 04/11] blksnap: header file of the module interface Sergei Shtepa
2023-04-04 14:08 ` [PATCH v3 05/11] blksnap: module management interface functions Sergei Shtepa
2023-04-04 14:08 ` [PATCH v3 06/11] blksnap: handling and tracking I/O units Sergei Shtepa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a1854604-cec1-abd5-1d49-6cf6a19ee7a1@veeam.com \
    --to=sergei.shtepa@veeam.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=buczek@molgen.mpg.de \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=kch@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=snitzer@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vkoul@kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).