[RESEND PATCH 0/5] Setting write hint in MD RAID

[RESEND PATCH 0/5] Setting write hint in MD RAID

[Mariusz Dabrowski]

This patchset adds support for write hints in MD driver. This is a new
feature for NVMe drives compliant to 1.3 specification and introduced to
Linux in kernel 4.13. Write hint has to be copied from bio containing user
data to bios sent to RAID members. Additionally, write hint can be set for
internal data like parity and PPL in RAID 5.

Setting write hint for parity is done with simple classification algorithm
which works for sequential IO workload. It tries to predict which parity
request are going to be overwritten in a moment and sets write hint for
them. This algorithm uses stripe cache to count updates of each parity
chunk. Parity request will be predicted as "soon-overwritten" if nubmer of
parity updates is smaller than number of data chunks in stripe.

For PPL there is no special algorithm. It is updated very frequently so we
can set write hint for each PPL write.

We have performed our internal tests which prove that setting write hint
for parity and PPL can significantly reduce write amplification.

Mariusz Dabrowski (5):
  raid1: copy write hint from master bio to behind bio
  raid5: copy write hint from origin bio to stripe
  raid5: sysfs attribute for write hint policy
  raid5: set write hint for parity
  raid5: set write hint for PPL

 Documentation/admin-guide/md.rst |  16 +++++
 drivers/md/raid1.c               |   2 +
 drivers/md/raid5-ppl.c           |   4 ++
 drivers/md/raid5.c               | 147 +++++++++++++++++++++++++++++++++++++--
 drivers/md/raid5.h               |  10 +++
 5 files changed, 175 insertions(+), 4 deletions(-)

-- 
2.16.1

[RESEND PATCH 1/5] raid1: copy write hint from master bio to behind bio

[Mariusz Dabrowski]

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 drivers/md/raid1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b2eae332e1a2..c512926a5193 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1092,6 +1092,8 @@ static void alloc_behind_master_bio(struct r1bio *r1_bio,
 		goto skip_copy;
 	}
 
+	behind_bio->bi_write_hint = bio->bi_write_hint;
+
 	while (i < vcnt && size) {
 		struct page *page;
 		int len = min_t(int, PAGE_SIZE, size);
-- 
2.16.1

[RESEND PATCH 2/5] raid5: copy write hint from origin bio to stripe

[Mariusz Dabrowski]

Store write hint from original bio in stripe head so it can be assigned to
bio sent to each RAID device.

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 drivers/md/raid5.c | 6 ++++++
 drivers/md/raid5.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 50d01144b805..bd99f1651b66 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1139,6 +1139,9 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 			bi->bi_io_vec[0].bv_len = STRIPE_SIZE;
 			bi->bi_io_vec[0].bv_offset = 0;
 			bi->bi_iter.bi_size = STRIPE_SIZE;
+			bi->bi_write_hint = sh->dev[i].write_hint;
+			if (!rrdev)
+				sh->dev[i].write_hint = 0;
 			/*
 			 * If this is discard request, set bi_vcnt 0. We don't
 			 * want to confuse SCSI because SCSI will replace payload
@@ -1190,6 +1193,8 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 			rbi->bi_io_vec[0].bv_len = STRIPE_SIZE;
 			rbi->bi_io_vec[0].bv_offset = 0;
 			rbi->bi_iter.bi_size = STRIPE_SIZE;
+			rbi->bi_write_hint = sh->dev[i].write_hint;
+			sh->dev[i].write_hint = 0;
 			/*
 			 * If this is discard request, set bi_vcnt 0. We don't
 			 * want to confuse SCSI because SCSI will replace payload
@@ -3203,6 +3208,7 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
 		(unsigned long long)sh->sector);
 
 	spin_lock_irq(&sh->stripe_lock);
+	sh->dev[dd_idx].write_hint = bi->bi_write_hint;
 	/* Don't allow new IO added to stripes in batch list */
 	if (sh->batch_head)
 		goto overlap;
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 2e6123825095..69686616d2ca 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -257,6 +257,7 @@ struct stripe_head {
 		sector_t	sector;			/* sector of this page */
 		unsigned long	flags;
 		u32		log_checksum;
+		u32		write_hint;
 	} dev[1]; /* allocated with extra space depending of RAID geometry */
 };
 
-- 
2.16.1

[RESEND PATCH 3/5] raid5: sysfs attribute for write hint policy

[Mariusz Dabrowski]

Added new sysfs attribute to change write hint classification for parity

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 drivers/md/raid5.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid5.h |  5 +++++
 2 files changed, 67 insertions(+)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index bd99f1651b66..088b97bbcbe2 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6631,6 +6631,67 @@ raid5_group_thread_cnt = __ATTR(group_thread_cnt, S_IRUGO | S_IWUSR,
 				raid5_show_group_thread_cnt,
 				raid5_store_group_thread_cnt);
 
+static ssize_t
+raid5_show_write_hint_policy(struct mddev *mddev, char *buf)
+{
+	char *sep = ",";
+	size_t len = 0;
+	unsigned long flags = 0;
+	struct r5conf *conf;
+
+	spin_lock(&mddev->lock);
+	conf = mddev->private;
+	if (conf) {
+		flags = conf->write_hint_flags;
+
+		if (!flags) {
+			len = sprintf(buf, "off");
+		} else {
+			if (test_bit(WH_POLICY_PARITY, &flags))
+				len += sprintf(buf+len, "parity%s", sep);
+			len -= strlen(sep);
+		}
+	}
+	spin_unlock(&mddev->lock);
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+static ssize_t
+raid5_store_write_hint_policy(struct mddev *mddev, const char *page, size_t len)
+{
+	struct r5conf *conf;
+	int err = 0;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	conf = mddev->private;
+	if (!conf) {
+		err = -ENODEV;
+	} else {
+		if (strncmp(page, "parity", 6) == 0)
+			set_bit(WH_POLICY_PARITY, &conf->write_hint_flags);
+		else if (strncmp(page, "-parity", 7) == 0)
+			clear_bit(WH_POLICY_PARITY, &conf->write_hint_flags);
+		else if (strncmp(page, "off", 3) == 0)
+			conf->write_hint_flags = 0;
+		else
+			err = -EINVAL;
+	}
+
+	mddev_unlock(mddev);
+
+	return err ?: len;
+}
+
+static struct md_sysfs_entry
+raid5_write_hint_policy = __ATTR(write_hint_policy, S_IRUGO | S_IWUSR,
+				 raid5_show_write_hint_policy,
+				 raid5_store_write_hint_policy);
+
 static struct attribute *raid5_attrs[] =  {
 	&raid5_stripecache_size.attr,
 	&raid5_stripecache_active.attr,
@@ -6639,6 +6700,7 @@ static struct attribute *raid5_attrs[] =  {
 	&raid5_skip_copy.attr,
 	&raid5_rmw_level.attr,
 	&r5c_journal_mode.attr,
+	&raid5_write_hint_policy.attr,
 	NULL,
 };
 static struct attribute_group raid5_attrs_group = {
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 69686616d2ca..e08071dc4202 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -195,6 +195,10 @@ enum reconstruct_states {
 	reconstruct_state_result,
 };
 
+enum write_hint_flags {
+	WH_POLICY_PARITY = 0,
+};
+
 struct stripe_head {
 	struct hlist_node	hash;
 	struct list_head	lru;	      /* inactive_list or handle_list */
@@ -565,6 +569,7 @@ struct r5conf {
 	int			raid_disks;
 	int			max_nr_stripes;
 	int			min_nr_stripes;
+	unsigned long		write_hint_flags;
 
 	/* reshape_progress is the leading edge of a 'reshape'
 	 * It has value MaxSector when no reshape is happening
-- 
2.16.1

[RESEND PATCH 4/5] raid5: set write hint for parity

[Mariusz Dabrowski]

During sequential workload, if no full-stripe write occurs, each parity
will be written data_disks (raid_disks-max_degraded) times. The lifetime
of the parity written before the data_disks update will be short, so
RWH_WRITE_LIFE_SHORT write hint can be assigned to those writes.

New counter has been added to the stripe_head struct to count number of
parity updates in a stripe. If this counter is equal to data_disks or this
is the full stripe write, a write hint from the original bio is assigned.
Otherwise RWH_WRITE_LIFE_SHORT write hint will be set.

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 drivers/md/raid5.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 drivers/md/raid5.h |  2 ++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 088b97bbcbe2..1c581b0bbc44 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -978,6 +978,36 @@ static void defer_issue_bios(struct r5conf *conf, sector_t sector,
 	dispatch_bio_list(&tmp);
 }
 
+static void raid5_set_parity_write_hint(struct bio *bio,
+					 struct stripe_head *sh,
+					 int *updates_cnt)
+{
+	enum rw_hint hint = WRITE_LIFE_NOT_SET;
+	struct r5conf *conf = sh->raid_conf;
+	int data_disks = conf->raid_disks - conf->max_degraded;
+
+	if (!test_bit(STRIPE_FULL_WRITE, &sh->state) &&
+	    (++*updates_cnt < data_disks)) {
+		hint = WRITE_LIFE_SHORT;
+	} else {
+		int i;
+
+		for (i = 0; i < sh->disks; i++) {
+			if ((i != sh->pd_idx) && (i != sh->qd_idx)) {
+				if (sh->dev[i].req.bi_write_hint > 0)
+					hint = sh->dev[i].req.bi_write_hint;
+				else if (sh->dev[i].write_hint > 0)
+					hint = sh->dev[i].write_hint;
+				if (hint)
+					break;
+			}
+		}
+
+		*updates_cnt = 0;
+	}
+	bio->bi_write_hint = hint;
+}
+
 static void
 raid5_end_read_request(struct bio *bi);
 static void
@@ -1003,6 +1037,9 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 		int replace_only = 0;
 		struct bio *bi, *rbi;
 		struct md_rdev *rdev, *rrdev = NULL;
+		int sh_syncing = s->syncing || s->expanding ||
+				 s->expanded || s->replacing;
+
 
 		sh = head_sh;
 		if (test_and_clear_bit(R5_Wantwrite, &sh->dev[i].flags)) {
@@ -1094,8 +1131,7 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 		}
 
 		if (rdev) {
-			if (s->syncing || s->expanding || s->expanded
-			    || s->replacing)
+			if (sh_syncing)
 				md_sync_acct(rdev->bdev, STRIPE_SECTORS);
 
 			set_bit(STRIPE_IO_STARTED, &sh->state);
@@ -1155,14 +1191,25 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 				trace_block_bio_remap(bi->bi_disk->queue,
 						      bi, disk_devt(conf->mddev->gendisk),
 						      sh->dev[i].sector);
+
+			if (op_is_write(op) && !sh_syncing &&
+			    test_bit(WH_POLICY_PARITY,
+				     &conf->write_hint_flags)) {
+				if (i == sh->pd_idx)
+					raid5_set_parity_write_hint(bi, sh,
+							&sh->p_updates);
+				else if (i == sh->qd_idx)
+					raid5_set_parity_write_hint(bi, sh,
+							&sh->q_updates);
+			}
+
 			if (should_defer && op_is_write(op))
 				bio_list_add(&pending_bios, bi);
 			else
 				generic_make_request(bi);
 		}
 		if (rrdev) {
-			if (s->syncing || s->expanding || s->expanded
-			    || s->replacing)
+			if (sh_syncing)
 				md_sync_acct(rrdev->bdev, STRIPE_SECTORS);
 
 			set_bit(STRIPE_IO_STARTED, &sh->state);
@@ -1205,6 +1252,18 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 				trace_block_bio_remap(rbi->bi_disk->queue,
 						      rbi, disk_devt(conf->mddev->gendisk),
 						      sh->dev[i].sector);
+
+			if (op_is_write(op) && !sh_syncing &&
+			    test_bit(WH_POLICY_PARITY,
+				     &conf->write_hint_flags)) {
+				if (i == sh->pd_idx)
+					raid5_set_parity_write_hint(rbi, sh,
+							&sh->p_updates);
+				else if (i == sh->qd_idx)
+					raid5_set_parity_write_hint(rbi, sh,
+							&sh->q_updates);
+			}
+
 			if (should_defer && op_is_write(op))
 				bio_list_add(&pending_bios, rbi);
 			else
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index e08071dc4202..4a5fa44ffecf 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -219,6 +219,8 @@ struct stripe_head {
 						  * this is only checked when stripe
 						  * has STRIPE_BATCH_READY
 						  */
+	int p_updates;
+	int q_updates;
 	enum check_states	check_state;
 	enum reconstruct_states reconstruct_state;
 	spinlock_t		stripe_lock;
-- 
2.16.1

[RESEND PATCH 5/5] raid5: set write hint for PPL

[Mariusz Dabrowski]

When the Partial Parity Log is enabled, each write overwrites the old PPL
entry on the drive so set WRITE_LIFE_SHORT write hint for all PPL writes.

Change write hint policy for parity to WRITE_LIFE_MEDIUM because its
lifetime for single write is longer than PPL. It might cause a slightly
higher write amplification, but data with different lifetime will not be
mixed.

Introduce new policy "sharing" to use same write hint for both PPL and
parity when the workload is predominantly sequential (similar lifetime
for parity and PPL data).

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 Documentation/admin-guide/md.rst | 16 ++++++++++++++++
 drivers/md/raid5-ppl.c           |  4 ++++
 drivers/md/raid5.c               | 18 +++++++++++++++++-
 drivers/md/raid5.h               |  2 ++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/md.rst b/Documentation/admin-guide/md.rst
index 84de718f24a4..b3adab6ed960 100644
--- a/Documentation/admin-guide/md.rst
+++ b/Documentation/admin-guide/md.rst
@@ -756,3 +756,19 @@ These currently include:
       The cache mode for raid5. raid5 could include an extra disk for
       caching. The mode can be "write-throuth" and "write-back". The
       default is "write-through".
+
+  write_hint_policy (raid4, raid5 and raid6 only)
+      Write lifetime hint policy for raid 4/5/6. This file lists all active
+      policies separated by commas. Policy can be changed by writing:
+
+        ``parity`` set write hint for all parity requests which will be
+                   overwritten in a moment.
+
+        ``ppl`` set write lifetime hint for all requests containing PPL data.
+
+        ``sharing`` set the same write hint for PPL and parity. If not set, use
+                    longer lifetime write hint for parity. Applicable only if
+                    ``parity`` and ``ppl`` policies are enabled.
+
+      To disable specific policy, use ``-`` prefix. To disable all policies use
+      ``off`` value.
\ No newline at end of file
diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 2764c2290062..d98855281254 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -424,6 +424,10 @@ static void ppl_log_endio(struct bio *bio)
 static void ppl_submit_iounit_bio(struct ppl_io_unit *io, struct bio *bio)
 {
 	char b[BDEVNAME_SIZE];
+	struct r5conf *conf = io->log->ppl_conf->mddev->private;
+
+	if (test_bit(WH_POLICY_PPL, &conf->write_hint_flags))
+		bio->bi_write_hint = WRITE_LIFE_SHORT;
 
 	pr_debug("%s: seq: %llu size: %u sector: %llu dev: %s\n",
 		 __func__, io->seq, bio->bi_iter.bi_size,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 1c581b0bbc44..2236103b7306 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -992,7 +992,11 @@ static void raid5_set_parity_write_hint(struct bio *bio,
 
 	if (!test_bit(STRIPE_FULL_WRITE, &sh->state) &&
 	    (++*updates_cnt < data_disks)) {
-		hint = WRITE_LIFE_SHORT;
+		if (test_bit(WH_POLICY_SHARING, &conf->write_hint_flags) &&
+		    test_bit(WH_POLICY_PPL, &conf->write_hint_flags))
+			hint = WRITE_LIFE_SHORT;
+		else
+			hint = WRITE_LIFE_MEDIUM;
 	} else {
 		int i;
 
@@ -6708,6 +6712,10 @@ raid5_show_write_hint_policy(struct mddev *mddev, char *buf)
 		} else {
 			if (test_bit(WH_POLICY_PARITY, &flags))
 				len += sprintf(buf+len, "parity%s", sep);
+			if (test_bit(WH_POLICY_PPL, &flags))
+				len += sprintf(buf+len, "ppl%s", sep);
+			if (test_bit(WH_POLICY_SHARING, &flags))
+				len += sprintf(buf+len, "sharing%s", sep);
 			len -= strlen(sep);
 		}
 	}
@@ -6735,6 +6743,14 @@ raid5_store_write_hint_policy(struct mddev *mddev, const char *page, size_t len)
 			set_bit(WH_POLICY_PARITY, &conf->write_hint_flags);
 		else if (strncmp(page, "-parity", 7) == 0)
 			clear_bit(WH_POLICY_PARITY, &conf->write_hint_flags);
+		else if (strncmp(page, "ppl", 3) == 0)
+			set_bit(WH_POLICY_PPL, &conf->write_hint_flags);
+		else if (strncmp(page, "-ppl", 4) == 0)
+			clear_bit(WH_POLICY_PPL, &conf->write_hint_flags);
+		else if (strncmp(page, "sharing", 7) == 0)
+			set_bit(WH_POLICY_SHARING, &conf->write_hint_flags);
+		else if (strncmp(page, "-sharing", 8) == 0)
+			clear_bit(WH_POLICY_SHARING, &conf->write_hint_flags);
 		else if (strncmp(page, "off", 3) == 0)
 			conf->write_hint_flags = 0;
 		else
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 4a5fa44ffecf..ca7233dacdf1 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -197,6 +197,8 @@ enum reconstruct_states {
 
 enum write_hint_flags {
 	WH_POLICY_PARITY = 0,
+	WH_POLICY_PPL,
+	WH_POLICY_SHARING,
 };
 
 struct stripe_head {
-- 
2.16.1