a caching layer for raid 5/6

Hi,

These are the patches to add a caching layer for raid5/6. It uses a SSD as a cache for a raid 5/6. It works like the similar way of a hardware raid controller. The purpose is to improve raid performance (reduce read-modify-write) and fix write hole issue. The main patch is patch 3 and the description has all details about the implementation.

This also requires mdadm utility changes, which Song will send out later.

Comments and suggestions are welcome!

Thanks,
Shaohua

Shaohua Li (4):
raid5: directly use mddev->queue
A caching layer for RAID5/6
raid5-cache: add some sysfs entries
md: don't allow resize/reshape with cache support

Song Liu (1):
MD: add a new disk role to present cache device

 drivers/md/Makefile            |    2 +-
 drivers/md/md.c                |   14 +-
 drivers/md/md.h                |    4 +
 drivers/md/raid5-cache.c       | 3252 ++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid5.c             |   90 +-
 drivers/md/raid5.h             |   16 +-
 include/uapi/linux/raid/md_p.h |   73 +
 7 files changed, 3431 insertions(+), 20 deletions(-)
 create mode 100644 drivers/md/raid5-cache.c

1.8.1

From: Song Liu <[email protected]>

Next patches will use a disk as raid5/6 caching. We need a new disk role to present the cache device

Not sure if we should bump up the MD superblock version for the disk role.

---
 drivers/md/md.c                | 14 +++++++++++++-
 drivers/md/md.h                |  4 ++++
 include/uapi/linux/raid/md_p.h |  1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

index d4f31e1..b6ece48 100644
@@ -1656,6 +1656,9 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
 		case 0xfffe: /* faulty */
 			set_bit(Faulty, &rdev->flags);
 			break;
+		case 0xfffd: /* cache device */
+			set_bit(WriteCache, &rdev->flags);
+			break;
 		default:
 			rdev->saved_raid_disk = role;
 			if ((le32_to_cpu(sb->feature_map) &

@@ -1811,6 +1814,8 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
 			sb->dev_roles[i] = cpu_to_le16(0xfffe);
 		else if (test_bit(In_sync, &rdev2->flags))
 			sb->dev_roles[i] = cpu_to_le16(rdev2->raid_disk);
+		else if (test_bit(WriteCache, &rdev2->flags))
+			sb->dev_roles[i] = cpu_to_le16(0xfffd);
 		else if (rdev2->raid_disk >= 0)
 			sb->dev_roles[i] = cpu_to_le16(rdev2->raid_disk);
 		else

@@ -5778,7 +5783,8 @@ static int get_disk_info(struct mddev *mddev, void __user * arg)
 		else if (test_bit(In_sync, &rdev->flags)) {
 			info.state |= (1<<MD_DISK_ACTIVE);
 			info.state |= (1<<MD_DISK_SYNC);
-		}
+		} else if (test_bit(WriteCache, &rdev->flags))
+			info.state |= (1<<MD_DISK_WRITECACHE);
 		if (test_bit(WriteMostly, &rdev->flags))
 			info.state |= (1<<MD_DISK_WRITEMOSTLY);
 	} else {

@@ -5893,6 +5899,8 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
 		else
 			clear_bit(WriteMostly, &rdev->flags);
 
+		if (info->state & (1<<MD_DISK_WRITECACHE))
+			set_bit(WriteCache, &rdev->flags);
 		/*
 		 * check whether the device shows up in other nodes
 		 */

@@ -7261,6 +7269,10 @@ static int md_seq_show(struct seq_file *seq, void *v)
 				seq_printf(seq, "(F)");
 				continue;
 			}
+			if (test_bit(WriteCache, &rdev->flags)) {
+				seq_printf(seq, "(C)");
+				continue;
+			}
 			if (rdev->raid_disk < 0)
 				seq_printf(seq, "(S)"); /* spare */
 			if (test_bit(Replacement, &rdev->flags))

index 4046a6c..6857592 100644
@@ -175,6 +175,10 @@ enum flag_bits {
 				 * This device is seen locally but not
 				 * by the whole cluster
 				 */
+	WriteCache,		/* This device is used as write cache.
+				 * Usually, this device should be faster
+				 * than other devices in the array
+				 */
 };
 
 #define BB_LEN_MASK	(0x00000000000001FFULL)

index 2ae6131..9d36b91 100644
@@ -89,6 +89,7 @@
 				   * read requests will only be sent here in
 				   * dire need
 				   */
+#define MD_DISK_WRITECACHE      18 /* disk is used as the write cache in RAID-5/6 */
 
 typedef struct mdp_device_descriptor_s {
 	__u32 number;		/* 0 Device number in the entire set	      */

1.8.1

When the cache layer flushes data from cache disk to raid disks, it will dipsatch IO to raid disks. At that time, we don't have a block device attached to the bio, so directly use mddev->queue. That should not impact IO dispatched to rdev, which has rdev block device attached.

---
 drivers/md/raid5.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

index 77dfd72..950c3c6 100644
@@ -223,7 +223,7 @@ static int raid6_idx_to_slot(int idx, struct stripe_head *sh,
 	return slot;
 }
 
-static void return_io(struct bio *return_bi)
+static void return_io(struct r5conf *conf, struct bio *return_bi)
 {
 	struct bio *bi = return_bi;
 	while (bi) {

@@ -231,8 +231,7 @@ static void return_io(struct bio *return_bi)
 		return_bi = bi->bi_next;
 		bi->bi_next = NULL;
 		bi->bi_iter.bi_size = 0;
-		trace_block_bio_complete(bdev_get_queue(bi->bi_bdev),
-					 bi, 0);
+		trace_block_bio_complete(conf->mddev->queue, bi, 0);
 		bio_endio(bi, 0);
 		bi = return_bi;
 	}

@@ -1193,7 +1192,7 @@ static void ops_complete_biofill(void *stripe_head_ref)
 	}
 	clear_bit(STRIPE_BIOFILL_RUN, &sh->state);
 
-	return_io(return_bi);
+	return_io(sh->raid_conf, return_bi);
 
 	set_bit(STRIPE_HANDLE, &sh->state);
 	release_stripe(sh);

@@ -4563,7 +4562,7 @@ static void handle_stripe(struct stripe_head *sh)
 			md_wakeup_thread(conf->mddev->thread);
 	}
 
-	return_io(s.return_bi);
+	return_io(conf, s.return_bi);
 
 	clear_bit_unlock(STRIPE_ACTIVE, &sh->state);
 }

@@ -5267,8 +5266,7 @@ static void make_request(struct mddev *mddev, struct bio * bi)
 		if ( rw == WRITE )
 			md_write_end(mddev);
 
-		trace_block_bio_complete(bdev_get_queue(bi->bi_bdev),
-					 bi, 0);
+		trace_block_bio_complete(mddev->queue, bi, 0);
 		bio_endio(bi, 0);
 	}
 }

1.8.1

Main goal of the caching layer is to aggregate IO write to hopefully make full stripe IO write and fix write hole issue. This might speed up read too, but it's not optimized for read, eg, we don't proactively cache data for read. The aggregation makes a lot of sense for workloads which sequentially write to several files with/without fsync. Such workloads are popular in today's datacenter.

Write IO data will write to a cache disk (SSD) first, then later the data will be flushed to raid disks.

The cache disk will be organized as a simple ring buffer log. For IO data, a tuple (raid_sector, io length, checksum, data) will be appended to the log; for raid 5/6 parity, a tuple (stripe_sector, parity length, checksum, parity data) will be appended to the log. We don't have on-disk index for the data appended to the log. So either we can rebuild an in-memory index at startup with scanning the whole log, or we can flush all data from cache disk to raid disks at shutdown so cache disk has no valid data. Current code chooses the second option, but this can be easily changed.

We have a simple meta data for above tuples. It's essentially a tuple (sequence, metadata length). Crash recovery or startup will scan the log to read the metadata and rebuild in-memory index. If metadata is broken at the head of the log, even metadata afterward is ok, the scanning will not work well. So we take some steps to mitigate the issue:
-meta data is written with WRITE_FUA. write IO return only happens after both data and metadata IO finish
-IO return happens in the order they are added in the log.
These will make sure write IO return happens after its meta and all previous meta IO finish, eg all related meta are persistent.

The in-memory index is a simple list of io_range (sequence, metadata sector, data sector, length). The list is orded by sequence. The first io_range entry's metadata sector is the tail of the log. There is also a struct to track io ranges within a stripe. All stripes will be organized as a radix tree.

All IO data will be copied to a memory pool for caching too until the data is flushed to raid disks. This is just to simplify the implementation, it's not mandated. In the future flush can do a data copy from cache disk to raid disks, so the memory pool can be discarded. If a stripe is flushed to raid disks, memory of the stripe can be reused.

We have two limited resources here, memory pool and cache disk space. If resource is tight, we will do reclaim. In either case, we will flush some data from cache disk to raid disks. However, we implement different strategies. For memory reclaim, we prefer reclaiming full stripe. For cache disk space reclaim, we prefer reclaiming io_range entry at the head of index list.

If cache disk has IO error, since all data are in memory pool, we will flush all data to raid disks and fallback to no-cache-disk mode. IO error of cache disk doesn't corrupt any data. After some time, we will try to use cache disk again if the disk is ok. The retry will stop after several rounds of failure.

We always do reclaim in stripe unit. Reclaim could create holes in the log, eg, some io_range in the middle is reclaimed, but io_range at the head remains. So the index list entries don't always have continuous sequence. But this doesn't matter, the first io_range is always the log tail. Superblock has a field pointing to the position of log tail. The hole can waste a lot of disk space though. In the future, we can implement a garbage collection to mitigate the issue, eg, copy data from the index tail to head.

In the process reclaim flush data to raid disk, stripe parity will be append to cache log. Parity is always appended after its corresponding data. After parity is in cache disk, a flush_start block is appended to log, which indicates stripe is flushing to raid disks. Data writing to raid disks only happens after all data and parity are already in cache disk. This will fix the write whole issue. After a stripe is flushed to raid disks, a flush_end block is appended to log, which indicates a stripe is settled down in raid disks.

Recovery relies on the flush_start and flush_end block. If recovery finds data and parity of a stripe, the flush start/end block will be used to determine which stage the stripe is in flush. If the stripe is listed in flush end block, the stripe is in raid disks, all data and parity of the stripe can be discarded. If the stripe isn't listed in flush start block, the stripe hasn't started flush to raid yet, its parity can be ignored. Otherwise, the stripe is in the middle of flushing to raid disks. Since we have both data and parity, the recovery just rewrite them to raid disks.

IO write code path:
1. copy bio data to stripe memory pages
2. append metadata and data to cache log
3. IO write endio

reclaim code path:
1. select stripe to reclaim
2. write all stripe data to raid disk
3. in raid5 ops_run_io, append metadata and parity data to cache log. ops_run_io doesn't write data/parity to raid disks at this time
4. flush cache disk and write flush_start block
5. ops_run_io continues. data/parity will be written to raid disks
6. flush all raid disks cache
7. write flush_end block
8. delete in-memory index of the stripe, and advance superblock log checkpoint

Recovery:
Crash in IO write code path doesn't need recovery. If data and checksum don't match, the data will be ignored so read will return old data. In reclaim code path, crash before step 4 doesn't need recovery as data/parity don't touch raid disk yet. Parity can be ignored too. crash after 7 doesn't need recovery too, as the stripe is fully flushed to raid disks. Crash between 4 and 7 need recovery. Data and parity in the log will be written to raid disks.

The performance issue will mainly in two areas. The FUA write to log device and reclaim performance. The FUA write isn't fast, especially SATA disables FUA support by default. But since SSD is fast enough, this might not a big issue. On the other hand, the FUA isn't a must-have. As long as we flush log disk cache for any FUA or FLUSH bio from upper layer, we don't need to use FUA write for log device. This might lose data but matches the behavior of hard disk. We can fix this later if the issue is a bottleneck.

The performance of the raid will largely be determined by reclaim speed at run time. Several stages of the reclaim process involves IO wait or disk cache flush, which significantly impact the raid disk utilization and performance. The flush_start and flush_end block make running multiple reclaim possible. Each reclaim only records stripes in the flush start/end block which it is reclaiming. Recovery can use the information to correctly determine stripe's flush stage. An example of 2 reclaimer:

xxx1 belongs to stripe1 and the same for stripe2

reclaim 1:  |data1|...|parity1|...|flush_start1|...|flush_end1|
reclaim 2:      |data2|..|parity2|...............|flush_start2|...|flush_end2|

the reclaims only record its own stripes in flush block. If, for exmaple, recovery finds flush_start1, it knows stripe1 is flushing to raid disk. Recovery will ignore stripe2, since stripe2 isn't in flush_start1.

Multiple reclaim will efficiently solve the performance issue. Current code hasn't add multiple reclaim yet, but certainly will be added soon.

---
 drivers/md/Makefile            |    2 +-
 drivers/md/raid5-cache.c       | 3007 ++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid5.c             |   69 +-
 drivers/md/raid5.h             |   15 +-
 include/uapi/linux/raid/md_p.h |   72 +
 5 files changed, 3153 insertions(+), 12 deletions(-)
 create mode 100644 drivers/md/raid5-cache.c

index dba4db5..aeb4330 100644
@@ -16,7 +16,7 @@ dm-cache-mq-y   += dm-cache-policy-mq.o
 dm-cache-cleaner-y += dm-cache-policy-cleaner.o
 dm-era-y	+= dm-era-target.o
 md-mod-y	+= md.o bitmap.o
-raid456-y	+= raid5.o
+raid456-y	+= raid5.o raid5-cache.o
 
 # Note: link order is important.  All raid personalities
 # and must come before md.o, as they each initialise 

index 7dc0dd8..e4e93bb 100644

1.8.1

Part of the diff is not shown here. View the raw message.

Just a few comments from looking over the code, this isn't a technical review of the concepts yet:

Add some sysfs entries.
-cache_memory. Control the cache memory size. -cache_reclaim_batch. Control how many stripes reclaim should run in one time.
-cache_memory_watermark. The background reclaim runs if cache memory hits the watermark and stops after hit 1.5x of the watermark. -cache_disk_watermark. The background reclaim runs if cache disk space hits the watermark and stops after hit 1.5x of the watermark.

---
 drivers/md/raid5-cache.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/md/raid5.c       |   3 +
 drivers/md/raid5.h       |   1 +
 3 files changed, 250 insertions(+), 1 deletion(-)

index 355a450..b2855ed 100644
@@ -2869,6 +2869,247 @@ static int r5c_shrink_cache_memory(struct r5c_cache *cache, unsigned long size)
 	return 0;
 }
 
+static ssize_t r5c_show_cache_memory(struct mddev *mddev, char *page)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+
+	return sprintf(page, "%lld\n", cache->max_pages << PAGE_SHIFT);
+}
+
+static ssize_t r5c_store_cache_memory(struct mddev *mddev, const char *page,
+	size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+	unsigned long new;
+	LIST_HEAD(page_list);
+	u64 i;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+	if (kstrtoul(page, 0, &new))
+		return -EINVAL;
+	new >>= PAGE_SHIFT;
+
+	if (new > cache->max_pages) {
+		i = cache->max_pages;
+		while (i < new) {
+			struct page *page = alloc_page(GFP_KERNEL);
+
+			if (!page)
+				break;
+			list_add(&page->lru, &page_list);
+			i++;
+		}
+
+		spin_lock_irq(&cache->pool_lock);
+		list_splice(&page_list, &cache->page_pool);
+		cache->free_pages += i - cache->max_pages;
+		cache->max_pages = i;
+		cache->total_pages = i;
+		r5c_calculate_watermark(cache);
+		spin_unlock_irq(&cache->pool_lock);
+		return len;
+	}
+	r5c_shrink_cache_memory(cache, new);
+	return len;
+}
+
+static struct md_sysfs_entry r5c_cache_memory = __ATTR(cache_memory,
+	S_IRUGO | S_IWUSR, r5c_show_cache_memory, r5c_store_cache_memory);
+
+int r5c_min_stripe_cache_size(struct r5c_cache *cache)
+{
+	struct r5conf *conf = cache->mddev->private;
+	return (conf->chunk_sectors >> PAGE_SECTOR_SHIFT) *
+		cache->reclaim_batch;
+}
+
+static void r5c_set_reclaim_batch(struct r5c_cache *cache, int batch)
+{
+	struct mddev *mddev = cache->mddev;
+	struct r5conf *conf = mddev->private;
+	int size;
+
+	size = (STRIPE_PARITY_PAGES(cache) << PAGE_SECTOR_SHIFT) * batch;
+	if (size > cache->reserved_space) {
+		cache->reserved_space = size;
+		mutex_lock(&cache->log.io_mutex);
+		cache->log.reserved_blocks = SECTOR_BLOCK(&cache->log,
+			cache->reserved_space) + 1;
+		mutex_unlock(&cache->log.io_mutex);
+		r5c_wake_wait_reclaimer(cache,
+				RECLAIM_DISK_BACKGROUND);
+	} else {
+		mutex_lock(&cache->log.io_mutex);
+		cache->log.reserved_blocks -= SECTOR_BLOCK(&cache->log,
+			cache->reserved_space - size);
+		mutex_unlock(&cache->log.io_mutex);
+		cache->reserved_space = size;
+	}
+
+	size = (conf->chunk_sectors >> PAGE_SECTOR_SHIFT) * batch;
+
+	mddev_lock(mddev);
+	if (size > conf->max_nr_stripes)
+		raid5_set_cache_size(mddev, size);
+	mddev_unlock(mddev);
+
+	cache->reclaim_batch = batch;
+}
+
+static ssize_t r5c_show_cache_reclaim_batch(struct mddev *mddev, char *page)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+
+	return sprintf(page, "%d\n", cache->reclaim_batch);
+}
+
+static ssize_t r5c_store_cache_reclaim_batch(struct mddev *mddev,
+	const char *page, size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+	unsigned long new;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+	if (kstrtoul(page, 0, &new))
+		return -EINVAL;
+
+	if (new > MAX_FLUSH_STRIPES(&cache->log))
+		new = MAX_FLUSH_STRIPES(&cache->log);
+
+	if (new != cache->reclaim_batch)
+		r5c_set_reclaim_batch(cache, new);
+	return len;
+}
+
+static struct md_sysfs_entry r5c_cache_reclaim_batch =
+	__ATTR(cache_reclaim_batch, S_IRUGO | S_IWUSR,
+	r5c_show_cache_reclaim_batch, r5c_store_cache_reclaim_batch);
+
+static ssize_t r5c_show_cache_disk_watermark(struct mddev *mddev, char *page)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+
+	return sprintf(page, "%lld\n", cache->log.low_watermark *
+		cache->log.block_size);
+}
+
+static ssize_t r5c_store_cache_disk_watermark(struct mddev *mddev,
+	const char *page, size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+	struct r5l_log *log = &cache->log;
+	unsigned long new;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+	if (kstrtoul(page, 0, &new))
+		return -EINVAL;
+	new /= log->block_size;
+
+	if (new * 3 / 2 >= log->total_blocks)
+		return -EINVAL;
+
+	mutex_lock(&log->io_mutex);
+	log->low_watermark = new;
+	log->high_watermark = new * 3 / 2;
+	mutex_unlock(&log->io_mutex);
+	return len;
+}
+
+static struct md_sysfs_entry r5c_cache_disk_watermark =
+	__ATTR(cache_disk_watermark, S_IRUGO | S_IWUSR,
+	r5c_show_cache_disk_watermark, r5c_store_cache_disk_watermark);
+
+static ssize_t r5c_show_cache_memory_watermark(struct mddev *mddev, char *page)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+
+	return sprintf(page, "%lld\n", cache->low_watermark << PAGE_SHIFT);
+}
+
+static ssize_t r5c_store_cache_memory_watermark(struct mddev *mddev,
+	const char *page, size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	struct r5c_cache *cache = conf->cache;
+	unsigned long new;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+	if (kstrtoul(page, 0, &new))
+		return -EINVAL;
+	new >>= PAGE_SHIFT;
+
+	if (new * 3 / 2 >= cache->max_pages)
+		return -EINVAL;
+
+	spin_lock_irq(&cache->pool_lock);
+	cache->low_watermark = new;
+	cache->high_watermark = new * 3 / 2;
+	spin_unlock_irq(&cache->pool_lock);
+	return len;
+}
+
+static struct md_sysfs_entry r5c_cache_memory_watermark =
+	__ATTR(cache_memory_watermark, S_IRUGO | S_IWUSR,
+	r5c_show_cache_memory_watermark, r5c_store_cache_memory_watermark);
+
+static int r5c_init_sysfs(struct r5c_cache *cache)
+{
+	struct mddev *mddev = cache->mddev;
+	int ret;
+
+	ret = sysfs_add_file_to_group(&mddev->kobj, &r5c_cache_memory.attr,
+				      NULL);
+	if (ret)
+		return ret;
+	ret = sysfs_add_file_to_group(&mddev->kobj,
+				      &r5c_cache_reclaim_batch.attr, NULL);
+	if (ret)
+		goto err_reclaim;
+	ret = sysfs_add_file_to_group(&mddev->kobj,
+				      &r5c_cache_disk_watermark.attr, NULL);
+	if (ret)
+		goto disk_watermark;
+	ret = sysfs_add_file_to_group(&mddev->kobj,
+				      &r5c_cache_memory_watermark.attr, NULL);
+	if (ret)
+		goto memory_watermark;
+	return 0;
+memory_watermark:
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_disk_watermark.attr, NULL);
+disk_watermark:
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_reclaim_batch.attr, NULL);
+err_reclaim:
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_memory.attr, NULL);
+	return ret;
+}
+
+static void r5c_exit_sysfs(struct r5c_cache *cache)
+{
+	struct mddev *mddev = cache->mddev;
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_reclaim_batch.attr, NULL);
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_memory.attr, NULL);
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_disk_watermark.attr, NULL);
+	sysfs_remove_file_from_group(&mddev->kobj,
+		&r5c_cache_memory_watermark.attr, NULL);
+}
+
 static void r5c_free_cache_data(struct r5c_cache *cache)
 {
 	struct r5c_stripe *stripe;

@@ -2973,8 +3214,11 @@ struct r5c_cache *r5c_init_cache(struct r5conf *conf, struct md_rdev *rdev)
 	cache->reclaim_thread->timeout = CHECKPOINT_TIMEOUT;
 
 	r5c_shrink_cache_memory(cache, cache->max_pages);
-
+	if (r5c_init_sysfs(cache))
+		goto err_sysfs;
 	return cache;
+err_sysfs:
+	md_unregister_thread(&cache->reclaim_thread);
 err_page:
 	r5c_free_cache_data(cache);
 

@@ -2993,6 +3237,7 @@ struct r5c_cache *r5c_init_cache(struct r5conf *conf, struct md_rdev *rdev)
 
 void r5c_exit_cache(struct r5c_cache *cache)
 {
+	r5c_exit_sysfs(cache);
 	md_unregister_thread(&cache->reclaim_thread);
 	r5l_exit_log(&cache->log);
 

index 7273f75..14bc5fd 100644
@@ -5876,6 +5876,9 @@ raid5_set_cache_size(struct mddev *mddev, int size)
 	if (size <= 16 || size > 32768)
 		return -EINVAL;
 
+	if (conf->cache && size < r5c_min_stripe_cache_size(conf->cache))
+		size = r5c_min_stripe_cache_size(conf->cache);
+
 	conf->min_nr_stripes = size;
 	while (size < conf->max_nr_stripes &&
 	       drop_one_stripe(conf))

index e4e93bb..899ec79 100644
@@ -614,4 +614,5 @@ void r5c_exit_cache(struct r5c_cache *cache);
 void r5c_write_start(struct mddev *mddev, struct bio *bi);
 void r5c_write_end(struct mddev *mddev, struct bio *bi);
 void r5c_quiesce(struct r5conf *conf, int state);
+int r5c_min_stripe_cache_size(struct r5c_cache *cache);
 #endif

1.8.1

If cache support is enabled, don't allow resize/reshape in current stage. In the future, we can flush all data from cache to raid before resize/reshape and then allow resize/reshape.

---
 drivers/md/raid5.c | 6 ++++++
 1 file changed, 6 insertions(+)

index 14bc5fd..1365316 100644
@@ -7173,6 +7173,10 @@ static int raid5_resize(struct mddev *mddev, sector_t sectors)
 	 * worth it.
 	 */
 	sector_t newsize;
+	struct r5conf *conf = mddev->private;
+
+	if (conf->cache)
+		return -EINVAL;
 	sectors &= ~((sector_t)mddev->chunk_sectors - 1);
 	newsize = raid5_size(mddev, sectors, mddev->raid_disks);
 	if (mddev->external_size &&

@@ -7224,6 +7228,8 @@ static int check_reshape(struct mddev *mddev)
 {
 	struct r5conf *conf = mddev->private;
 
+	if (conf->cache)
+		return -EINVAL;
 	if (mddev->delta_disks == 0 &&
 	    mddev->new_layout == mddev->layout &&
 	    mddev->new_chunk_sectors == mddev->chunk_sectors)

1.8.1

Hi Shaohua,

here are a couple of notes from reading through the code in a bit more detail:

Error retries:
- What is the reason for retry_bio_list? If a driver returns an I/O error to the higher levels it already has retried and came to the conclusion this is a permanent error.

Flushes:
- no need to allocate a task here
- no real need to clone the bio either

Tasks:
- the completion argument passed to r5l_queue_bio is always the same, the code would be a lot simpler by removing this abstraction.
- that would also allow allocating the task embedded in the range and cut down on memory allocations
- we're not really manipulating the bio payload, so shouldn't a _fast cone be fine here?
In fact why do we clone the bio at all?
- r5l_queue_task should probably be split into two helpers for data vs parity
- r5l_queue_bio and r5c_copy_bio should probably use bvec iterators
- r5l_run_task does very different things for data vs metadata, it's proably better it.

Allocations:
- most metadata pages are allocated as highmem leading to constant kmap/kunmap. Maybe just allocate them as GFP_KERNEL to simplify things?

Misc:
- where does the ioctl in r5l_ioctl anѕ associated functions come from? There are not ioctls handler here so the naming seems rather confusing.
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Thanks for your time.

The retry_bio_list is to handle io to cache disk. If IO to cache disk has error, it's not a permanent error here. The cache disk is a cache, We can still dispatch the IO to its final destination, the raid disks.

clone the bio makes the IO retry easier.

Yep, my original idea is the stuff handling caching (r5c*) doesn't need to know the detail of log device. I'll look this again to check if this is a over-design.

The cache disk drive might manipulate the bio payload. The problem is really why we clone the bio. You are right we don't need to clone the bio at normal case. The exception is IO error. IO to the cache disk can fail. In that case, we will try to skip cache disk and dispatch IO to raid disks directly, which is the retry. Since cache disk driver might already manipulate the bio, cloning a bio makes retry easier.

I'll check these.

Sounds good.

Ah, it manages an io really, I'll change a name.

Thanks,
Shaohua
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

How does this work in practice? We've filled our cache disk with dirty data, and it now returns non-correctable write errors. At this point we had claimed to caller that data is on stable disk, but our cache disk is toast now. Is it really a good idea to now start a large window where we do not actually have the cache data on stable storage we can get back at but pretent business as usual?

IMHO the only sane way is to shut down the array when write to the cache disk fail. Hopefully the disk will still allow reading from it. Note that to be on the safe side you'll need a mirrored cache disk anyway.

We have a memory pool here. All data which aren't flushed to raid disks are in the pool. So if there is io error in cache disk, we still can flush the data from memory pool to raid disks. The pool is a limited resource, so can reduce IO aggregation effect though, but we flush full stripe data almost immediately to raid disks, which can mitigate a little. This will make the caching layer like hardware raid card very much.

Thanks,
Shaohua