[PATCH 0/2] dm log writes: Add support for filter bios based on its type

[PATCH 0/2] dm log writes: Add support for filter bios based on its type

[Qu Wenruo]

Current dm-log-writes will record all bios, no matter if the bios is
METADATA (normally what we care) or is DATA (normally we cares less for
the log-replay context).

This causes a lot of extra space required for log device. E.g write 10M,
sync, overwrite that 10M file, this would cause over 20M for log device
just for the data.

This patchset introduces two way to specify the dump type:
- dump_type=%s optional option for constructor
  The default dump_type is ALL, thus no behavior change.

- dump_type=%s message type to change type on the fly

Also to cooperate the new dump_type= option, always output the dump_type
for STATUSTYPE_TABLE.

A common use case would be:
 # dmsetup create log --table 0 $(blockdev --getsz $dev) log-writes $dev $log_dev
 # mkfs.btrfs -f /dev/mapper/log
 # dmsetup suspend log
 # dmsetup message log 0 dump_type=METADATA|FUA|FLUSH|DISCARD|MARK
 # dmsetup resume log
 # mount /dev/mapper/log <mnt>
 # <do some work load>
 # umount <mnt>
 # dmsetup remove log
 # <replay>

Now the log device will not record data writes, thus hugely reduce the
space requirement for log device, allowing more operations to be down
before hitting the space limit.

Currently btrfs check doesn't check data checksum by default, thus even
we have wrong data on-disk, we should be fine checking the metadata.

I'm not 100% sure if this applies to other filesystems, but as long as
metadata writes is marked correctly, other filesystems can also benifit
from this feature.

Qu Wenruo (2):
  dm log writes: Allow dm-log-writes to filter bios based on types to
    reduce log device space usage
  dm log writes: Introduce dump_type= message type to change dump_type
    on the fly

 drivers/md/dm-log-writes.c | 177 ++++++++++++++++++++++++++++++++++---
 1 file changed, 167 insertions(+), 10 deletions(-)

-- 
2.22.0

[PATCH 1/2] dm log writes: Allow dm-log-writes to filter bios based on types to reduce log device space usage

[Qu Wenruo]

Since dm-log-writes will record all writes, include data and metadata
writes, it can consume a lot of space.
However for a lot of filesystems, the data bio (without METADATA flag)
can be skipped for certain use case, thus we can skip them in
dm-log-writes to hugely reduce space usage.

This patch will introduce a new optional constructor parameter,
"dump_type=%s", for dm-log-writes.

The '%s' can be ALL, METADATA, FUA, FLUSH, DISCARD, MARK or the ORed
result of them.
The default dump_type will be 'ALL', so the behavior is not changed at
all.

But user can specify dump_type=METADATA|FUA|FLUSH|DISCARD|MARK to skip
data writes to save space on log device.

Currently the dump_type can only be speicified at contruction time.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 drivers/md/dm-log-writes.c | 146 +++++++++++++++++++++++++++++++++++--
 1 file changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c
index af94bbe77ce2..9edf0bdcae39 100644
--- a/drivers/md/dm-log-writes.c
+++ b/drivers/md/dm-log-writes.c
@@ -115,6 +115,7 @@ struct log_writes_c {
 	struct list_head logging_blocks;
 	wait_queue_head_t wait;
 	struct task_struct *log_kthread;
+	u32 dump_type;
 };
 
 struct pending_block {
@@ -503,15 +504,99 @@ static int log_writes_kthread(void *arg)
 	return 0;
 }
 
+#define string_type_to_bit(string)			\
+({							\
+	if (!strcasecmp(p, #string)) {			\
+		dump_type |= LOG_##string##_FLAG;	\
+		continue;				\
+	}						\
+})
+static int parse_dump_types(struct log_writes_c *lc, const char *string)
+{
+	char *orig;
+	char *opts;
+	char *p;
+	u32 dump_type = LOG_MARK_FLAG;
+	int ret = 0;
+
+	opts = kstrdup(string, GFP_KERNEL);
+	if (!opts)
+		return -ENOMEM;
+	orig = opts;
+
+	while ((p = strsep(&opts, "|")) != NULL) {
+		if (!*p)
+			continue;
+		if (!strcasecmp(p, "ALL")) {
+			dump_type = (u32)-1;
+			/* No need to check other flags */
+			break;
+		}
+		string_type_to_bit(METADATA);
+		string_type_to_bit(FUA);
+		string_type_to_bit(FLUSH);
+		string_type_to_bit(DISCARD);
+		string_type_to_bit(MARK);
+		ret = -EINVAL;
+		goto out;
+	}
+out:
+	kfree(orig);
+	if (!ret)
+		lc->dump_type = dump_type;
+	return ret;
+}
+#undef string_type_to_bit
+
+/* Must be large enough to contain "METADATA|FUA|FLUSH|DISCARD|MARK" */
+#define DUMP_TYPES_BUF_SIZE	256
+#define dump_type_to_string(name)				\
+({								\
+	if (lc->dump_type & LOG_##name##_FLAG) {		\
+		if (!first_word)				\
+			strcat(buf, "|");			\
+		strcat(buf, #name);				\
+		first_word = false;				\
+	}							\
+ })
+static void status_dump_types(struct log_writes_c *lc, char *buf)
+{
+	bool first_word = true;
+
+	buf[0] = '\0';
+
+	if (lc->dump_type == (u32)-1) {
+		strcat(buf, "ALL");
+		return;
+	}
+	dump_type_to_string(METADATA);
+	dump_type_to_string(FUA);
+	dump_type_to_string(FLUSH);
+	dump_type_to_string(DISCARD);
+	dump_type_to_string(MARK);
+	return;
+}
+#undef dump_type_to_string
+
 /*
  * Construct a log-writes mapping:
- * log-writes <dev_path> <log_dev_path>
+ * log-writes <dev_path> <log_dev_path> [<option feature> ...]
+ * option feature can be:
+ * - dump_type=<flags>
+ *   flags can be ALL, METADATA, FUA, FLUSH, DISCARD or any of them combined
+ *   with '|'.
+ *   Default value is ALL.
+ *
+ *   This will make log-writes only to record writes with certain type.
+ *   E.g dump_type=METADATA|FUA|FLUSH|DISCARD will only record metadata writes
+ *       and save log device space.
  */
 static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 {
 	struct log_writes_c *lc;
 	struct dm_arg_set as;
 	const char *devname, *logdevname;
+	const char *arg_name;
 	int ret;
 
 	as.argc = argc;
@@ -533,8 +618,10 @@ static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	init_waitqueue_head(&lc->wait);
 	atomic_set(&lc->io_blocks, 0);
 	atomic_set(&lc->pending_blocks, 0);
+	lc->dump_type = (u32)-1;
 
 	devname = dm_shift_arg(&as);
+	argc--;
 	ret = dm_get_device(ti, devname, dm_table_get_mode(ti->table), &lc->dev);
 	if (ret) {
 		ti->error = "Device lookup failed";
@@ -542,6 +629,7 @@ static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	}
 
 	logdevname = dm_shift_arg(&as);
+	argc--;
 	ret = dm_get_device(ti, logdevname, dm_table_get_mode(ti->table),
 			    &lc->logdev);
 	if (ret) {
@@ -550,15 +638,35 @@ static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 
+	while (argc) {
+		arg_name = dm_shift_arg(&as);
+		argc--;
+
+		if (!arg_name) {
+			ti->error = "Insufficient feature arguments";
+			goto free_all;
+		}
+		/* dump_type= */
+		if (!strncasecmp(arg_name, "dump_type=", strlen("dump_type="))) {
+			ret = parse_dump_types(lc,
+					arg_name + strlen("dump_type="));
+			if (ret < 0) {
+				ti->error = "Bad dump type";
+				goto free_all;
+			}
+			continue;
+		}
+		ti->error = "Unrecognised log-writes feature requested";
+		goto free_all;
+	}
+
 	lc->sectorsize = bdev_logical_block_size(lc->dev->bdev);
 	lc->sectorshift = ilog2(lc->sectorsize);
 	lc->log_kthread = kthread_run(log_writes_kthread, lc, "log-write");
 	if (IS_ERR(lc->log_kthread)) {
 		ret = PTR_ERR(lc->log_kthread);
 		ti->error = "Couldn't alloc kthread";
-		dm_put_device(ti, lc->dev);
-		dm_put_device(ti, lc->logdev);
-		goto bad;
+		goto free_all;
 	}
 
 	/*
@@ -579,6 +687,9 @@ static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	ti->private = lc;
 	return 0;
 
+free_all:
+	dm_put_device(ti, lc->dev);
+	dm_put_device(ti, lc->logdev);
 bad:
 	kfree(lc);
 	return ret;
@@ -589,6 +700,8 @@ static int log_mark(struct log_writes_c *lc, char *data)
 	struct pending_block *block;
 	size_t maxsize = lc->sectorsize - sizeof(struct log_write_entry);
 
+	if (!(lc->dump_type & LOG_MARK_FLAG))
+		goto wake_up;
 	block = kzalloc(sizeof(struct pending_block), GFP_KERNEL);
 	if (!block) {
 		DMERR("Error allocating pending block");
@@ -607,6 +720,7 @@ static int log_mark(struct log_writes_c *lc, char *data)
 	spin_lock_irq(&lc->blocks_lock);
 	list_add_tail(&block->list, &lc->logging_blocks);
 	spin_unlock_irq(&lc->blocks_lock);
+wake_up:
 	wake_up_process(lc->log_kthread);
 	return 0;
 }
@@ -643,6 +757,22 @@ static void normal_map_bio(struct dm_target *ti, struct bio *bio)
 	bio_set_dev(bio, lc->dev->bdev);
 }
 
+static bool need_record(struct log_writes_c *lc, struct bio *bio)
+{
+	if (lc->dump_type == (u32)-1)
+		return true;
+
+	if (lc->dump_type & LOG_METADATA_FLAG && (bio->bi_opf & REQ_META))
+		return true;
+	if (lc->dump_type & LOG_FUA_FLAG && (bio->bi_opf & REQ_FUA))
+		return true;
+	if (lc->dump_type & LOG_FLUSH_FLAG && (bio->bi_opf & REQ_PREFLUSH))
+		return true;
+	if (lc->dump_type & LOG_DISCARD_FLAG && (bio_op(bio) == REQ_PREFLUSH))
+		return true;
+	return false;
+}
+
 static int log_writes_map(struct dm_target *ti, struct bio *bio)
 {
 	struct log_writes_c *lc = ti->private;
@@ -673,6 +803,9 @@ static int log_writes_map(struct dm_target *ti, struct bio *bio)
 	if (!bio_sectors(bio) && !flush_bio)
 		goto map_bio;
 
+	/* Check against lc->dump_type */
+	if (!need_record(lc, bio))
+		goto map_bio;
 	/*
 	 * Discards will have bi_size set but there's no actual data, so just
 	 * allocate the size of the pending block.
@@ -803,6 +936,7 @@ static void log_writes_status(struct dm_target *ti, status_type_t type,
 {
 	unsigned sz = 0;
 	struct log_writes_c *lc = ti->private;
+	char dump_type_buf[DUMP_TYPES_BUF_SIZE];
 
 	switch (type) {
 	case STATUSTYPE_INFO:
@@ -813,7 +947,9 @@ static void log_writes_status(struct dm_target *ti, status_type_t type,
 		break;
 
 	case STATUSTYPE_TABLE:
-		DMEMIT("%s %s", lc->dev->name, lc->logdev->name);
+		status_dump_types(lc, dump_type_buf);
+		DMEMIT("%s %s dump_type=%s", lc->dev->name, lc->logdev->name,
+			dump_type_buf);
 		break;
 	}
 }
-- 
2.22.0

[PATCH 2/2] dm log writes: Introduce dump_type= message type to change dump_type on the fly

[Qu Wenruo]

The new message format is:
dump_type=<new dump type>

The parameter of dump_type= follows the same one of constructor.
This allows us to change dump_type on the fly, making the following use
case possible:
  # dmsetup create log --table 0 10485760 log-writes \
    /dev/tests/dest /dev/test/log dump_type=ALL
  # mkfs.btrfs -f /dev/mapper/log
  # dmsetup suspend log
  # dmsetup message log dm_dump_type=METADATA|FLUSH|FUA|DISCARD|MARK
  # mount /dev/mapper/log
  # <do some writes>
  # umount /dev/mapper/log

The log device will record the full mkfs bios (as user space write can't
generate bios with METADATA flag), then switch to only log METADATA FUA
FLUSH DISCARD writes.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 drivers/md/dm-log-writes.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c
index 9edf0bdcae39..80e872c7dcd3 100644
--- a/drivers/md/dm-log-writes.c
+++ b/drivers/md/dm-log-writes.c
@@ -980,7 +980,8 @@ static int log_writes_iterate_devices(struct dm_target *ti,
 
 /*
  * Messages supported:
- *   mark <mark data> - specify the marked data.
+ *   mark <mark data>	    - specify the marked data.
+ *   dump_type=<type flags> - change dump type on the fly, suspend recommended
  */
 static int log_writes_message(struct dm_target *ti, unsigned argc, char **argv,
 			      char *result, unsigned maxlen)
@@ -988,15 +989,35 @@ static int log_writes_message(struct dm_target *ti, unsigned argc, char **argv,
 	int r = -EINVAL;
 	struct log_writes_c *lc = ti->private;
 
-	if (argc != 2) {
-		DMWARN("Invalid log-writes message arguments, expect 2 arguments, got %d", argc);
+	if (argc < 1) {
+		DMWARN(
+"Invalid log-writes message arguments, expect at least one argument, got %d",
+			argc);
 		return r;
 	}
 
-	if (!strcasecmp(argv[0], "mark"))
+	if (!strcasecmp(argv[0], "mark")) {
+		if (argc != 2) {
+			DMWARN(
+"Invalid log-writes message arguments, expect 2 arguments for mark, got %d",
+				argc);
+			return r;
+		}
 		r = log_mark(lc, argv[1]);
-	else
+	} else if (!strncasecmp(argv[0], "dump_type=", strlen("dump_type="))) {
+		if (argc != 1) {
+			DMWARN(
+"Invalid log-writes message arguments, expect 1 argument for dump_type, got %d",
+				argc);
+			return r;
+		}
+		r = parse_dump_types(lc, argv[0] + strlen("dump_type="));
+		if (r < 0) {
+			ti->error = "Bad dump type";
+		}
+	} else {
 		DMWARN("Unrecognised log writes target message received: %s", argv[0]);
+	}
 
 	return r;
 }
-- 
2.22.0

Re: [PATCH 0/2] dm log writes: Add support for filter bios based on its type

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2157 bytes --]

Gentle ping?

This feature would be pretty useful if we want to really log heavy
operations on a relatively small log devices.

Thanks,
Qu

On 2019/6/19 下午4:03, Qu Wenruo wrote:
> Current dm-log-writes will record all bios, no matter if the bios is
> METADATA (normally what we care) or is DATA (normally we cares less for
> the log-replay context).
> 
> This causes a lot of extra space required for log device. E.g write 10M,
> sync, overwrite that 10M file, this would cause over 20M for log device
> just for the data.
> 
> This patchset introduces two way to specify the dump type:
> - dump_type=%s optional option for constructor
>   The default dump_type is ALL, thus no behavior change.
> 
> - dump_type=%s message type to change type on the fly
> 
> Also to cooperate the new dump_type= option, always output the dump_type
> for STATUSTYPE_TABLE.
> 
> A common use case would be:
>  # dmsetup create log --table 0 $(blockdev --getsz $dev) log-writes $dev $log_dev
>  # mkfs.btrfs -f /dev/mapper/log
>  # dmsetup suspend log
>  # dmsetup message log 0 dump_type=METADATA|FUA|FLUSH|DISCARD|MARK
>  # dmsetup resume log
>  # mount /dev/mapper/log <mnt>
>  # <do some work load>
>  # umount <mnt>
>  # dmsetup remove log
>  # <replay>
> 
> Now the log device will not record data writes, thus hugely reduce the
> space requirement for log device, allowing more operations to be down
> before hitting the space limit.
> 
> Currently btrfs check doesn't check data checksum by default, thus even
> we have wrong data on-disk, we should be fine checking the metadata.
> 
> I'm not 100% sure if this applies to other filesystems, but as long as
> metadata writes is marked correctly, other filesystems can also benifit
> from this feature.
> 
> Qu Wenruo (2):
>   dm log writes: Allow dm-log-writes to filter bios based on types to
>     reduce log device space usage
>   dm log writes: Introduce dump_type= message type to change dump_type
>     on the fly
> 
>  drivers/md/dm-log-writes.c | 177 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 167 insertions(+), 10 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]