[patch v2 0/6] Add TRIM support for raid linear/0/1/10

[patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Shaohua Li]

The patches add TRIM support for raid linear/0/1/10. I'll add TRIM support for
raid 4/5/6 later. The implementation is pretty straightforward and
self-explained.

v1->v2:
1. fixed a checking issue
2. dropped discard request plug and replace it with no discard merege, because
current SCSI layer can't handle discard request merge.

Thanks,
Shaohua

[patch v2 1/6] block: makes bio_split support bio without data

[Shaohua Li]

[-- Attachment #1: bio_split-fix.patch --]
[-- Type: text/plain, Size: 1637 bytes --]

discard bio hasn't data attached. We hit a BUG_ON with such bio. This makes
bio_split works for such bio.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 fs/bio.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Index: linux/fs/bio.c
===================================================================
--- linux.orig/fs/bio.c	2012-03-09 16:56:41.203790008 +0800
+++ linux/fs/bio.c	2012-03-12 10:10:40.696612399 +0800
@@ -1492,7 +1492,7 @@ struct bio_pair *bio_split(struct bio *b
 	trace_block_split(bdev_get_queue(bi->bi_bdev), bi,
 				bi->bi_sector + first_sectors);
 
-	BUG_ON(bi->bi_vcnt != 1);
+	BUG_ON(bi->bi_vcnt != 1 && bi->bi_vcnt != 0);
 	BUG_ON(bi->bi_idx != 0);
 	atomic_set(&bp->cnt, 3);
 	bp->error = 0;
@@ -1502,17 +1502,19 @@ struct bio_pair *bio_split(struct bio *b
 	bp->bio2.bi_size -= first_sectors << 9;
 	bp->bio1.bi_size = first_sectors << 9;
 
-	bp->bv1 = bi->bi_io_vec[0];
-	bp->bv2 = bi->bi_io_vec[0];
-	bp->bv2.bv_offset += first_sectors << 9;
-	bp->bv2.bv_len -= first_sectors << 9;
-	bp->bv1.bv_len = first_sectors << 9;
+	if (bi->bi_vcnt != 0) {
+		bp->bv1 = bi->bi_io_vec[0];
+		bp->bv2 = bi->bi_io_vec[0];
+		bp->bv2.bv_offset += first_sectors << 9;
+		bp->bv2.bv_len -= first_sectors << 9;
+		bp->bv1.bv_len = first_sectors << 9;
 
-	bp->bio1.bi_io_vec = &bp->bv1;
-	bp->bio2.bi_io_vec = &bp->bv2;
+		bp->bio1.bi_io_vec = &bp->bv1;
+		bp->bio2.bi_io_vec = &bp->bv2;
 
-	bp->bio1.bi_max_vecs = 1;
-	bp->bio2.bi_max_vecs = 1;
+		bp->bio1.bi_max_vecs = 1;
+		bp->bio2.bi_max_vecs = 1;
+	}
 
 	bp->bio1.bi_end_io = bio_pair_end_1;
 	bp->bio2.bi_end_io = bio_pair_end_2;

[patch v2 2/6] blk: dont allow discard request merge temporarily

[Shaohua Li]

[-- Attachment #1: blk-discard-nomerge.patch --]
[-- Type: text/plain, Size: 1165 bytes --]

Didn't allow discard request merge temporarily, as SCSI layer isn't ready
for discard merge as Martin Petersen pointed out. This isn't fair for
non-scsi device, but looks this is the only way I can do currently.

We should have the same issue before, but maybe because discard merge is
very rare case. But now raid0/10 makes the merge quite possible, so we need
disable it explicitly.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 include/linux/blkdev.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/include/linux/blkdev.h
===================================================================
--- linux.orig/include/linux/blkdev.h	2012-03-14 09:20:06.787261188 +0800
+++ linux/include/linux/blkdev.h	2012-03-14 09:20:47.797261248 +0800
@@ -575,7 +575,7 @@ static inline void blk_clear_queue_full(
  * it already be started by driver.
  */
 #define RQ_NOMERGE_FLAGS	\
-	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
+	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA | REQ_DISCARD)
 #define rq_mergeable(rq)	\
 	(!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \
 	 (((rq)->cmd_flags & REQ_DISCARD) || \

[patch v2 3/6] md: linear supports TRIM

[Shaohua Li]

[-- Attachment #1: md-linear-discard-support.patch --]
[-- Type: text/plain, Size: 1620 bytes --]

This makes md linear support TRIM.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 drivers/md/linear.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Index: linux/drivers/md/linear.c
===================================================================
--- linux.orig/drivers/md/linear.c	2012-03-14 09:16:37.380435112 +0800
+++ linux/drivers/md/linear.c	2012-03-14 09:23:55.907261497 +0800
@@ -129,6 +129,7 @@ static struct linear_conf *linear_conf(s
 	struct linear_conf *conf;
 	struct md_rdev *rdev;
 	int i, cnt;
+	bool discard_supported = false;
 
 	conf = kzalloc (sizeof (*conf) + raid_disks*sizeof(struct dev_info),
 			GFP_KERNEL);
@@ -171,6 +172,8 @@ static struct linear_conf *linear_conf(s
 		conf->array_sectors += rdev->sectors;
 		cnt++;
 
+		if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
+			discard_supported = true;
 	}
 	if (cnt != raid_disks) {
 		printk(KERN_ERR "md/linear:%s: not enough drives present. Aborting!\n",
@@ -178,6 +181,11 @@ static struct linear_conf *linear_conf(s
 		goto out;
 	}
 
+	if (!discard_supported)
+		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+	else
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+
 	/*
 	 * Here we calculate the device offsets.
 	 */
@@ -319,6 +327,14 @@ static void linear_make_request(struct m
 	bio->bi_sector = bio->bi_sector - start_sector
 		+ tmp_dev->rdev->data_offset;
 	rcu_read_unlock();
+
+	if (unlikely((bio->bi_rw & REQ_DISCARD) &&
+		!blk_queue_discard(bdev_get_queue(bio->bi_bdev)))) {
+		/* Just ignore it */
+		bio_endio(bio, 0);
+		return;
+	}
+
 	generic_make_request(bio);
 }
 

[patch v2 4/6] md: raid 0 supports TRIM

[Shaohua Li]

[-- Attachment #1: md-raid0-discard-support.patch --]
[-- Type: text/plain, Size: 2432 bytes --]

This makes md raid 0 support TRIM.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 drivers/md/raid0.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Index: linux/drivers/md/raid0.c
===================================================================
--- linux.orig/drivers/md/raid0.c	2012-03-14 09:16:37.360435113 +0800
+++ linux/drivers/md/raid0.c	2012-03-14 09:32:11.017262138 +0800
@@ -88,6 +88,7 @@ static int create_strip_zones(struct mdd
 	char b[BDEVNAME_SIZE];
 	char b2[BDEVNAME_SIZE];
 	struct r0conf *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
+	bool discard_supported = false;
 
 	if (!conf)
 		return -ENOMEM;
@@ -201,6 +202,9 @@ static int create_strip_zones(struct mdd
 		if (!smallest || (rdev1->sectors < smallest->sectors))
 			smallest = rdev1;
 		cnt++;
+
+		if (blk_queue_discard(bdev_get_queue(rdev1->bdev)))
+			discard_supported = true;
 	}
 	if (cnt != mddev->raid_disks) {
 		printk(KERN_ERR "md/raid0:%s: too few disks (%d of %d) - "
@@ -278,6 +282,11 @@ static int create_strip_zones(struct mdd
 	blk_queue_io_opt(mddev->queue,
 			 (mddev->chunk_sectors << 9) * mddev->raid_disks);
 
+	if (!discard_supported)
+		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+	else
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+
 	pr_debug("md/raid0:%s: done.\n", mdname(mddev));
 	*private_conf = conf;
 
@@ -348,6 +357,7 @@ static int raid0_run(struct mddev *mddev
 	if (md_check_no_bitmap(mddev))
 		return -EINVAL;
 	blk_queue_max_hw_sectors(mddev->queue, mddev->chunk_sectors);
+	blk_queue_max_discard_sectors(mddev->queue, mddev->chunk_sectors);
 
 	/* if private is not null, we are here after takeover */
 	if (mddev->private == NULL) {
@@ -486,7 +496,7 @@ static void raid0_make_request(struct md
 		sector_t sector = bio->bi_sector;
 		struct bio_pair *bp;
 		/* Sanity check -- queue functions should prevent this happening */
-		if (bio->bi_vcnt != 1 ||
+		if ((bio->bi_vcnt != 1 && bio->bi_vcnt != 0) ||
 		    bio->bi_idx != 0)
 			goto bad_map;
 		/* This is a one page bio that upper layers
@@ -512,6 +522,13 @@ static void raid0_make_request(struct md
 	bio->bi_sector = sector_offset + zone->dev_start +
 		tmp_dev->data_offset;
 
+	if (unlikely((bio->bi_rw & REQ_DISCARD) &&
+		!blk_queue_discard(bdev_get_queue(bio->bi_bdev)))) {
+		/* Just ignore it */
+		bio_endio(bio, 0);
+		return;
+	}
+
 	generic_make_request(bio);
 	return;
 

[patch v2 5/6] md: raid 1 supports TRIM

[Shaohua Li]

[-- Attachment #1: md-raid1-discard-support.patch --]
[-- Type: text/plain, Size: 2829 bytes --]

This makes md raid 1 support TRIM.
If one disk supports discard and another not, or one has discard_zero_data and
another not, there could be inconsistent between data from such disks. But this
should not matter, discarded data is useless. This will add extra copy in rebuild
though.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 drivers/md/raid1.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Index: linux/drivers/md/raid1.c
===================================================================
--- linux.orig/drivers/md/raid1.c	2012-03-12 10:09:42.426612652 +0800
+++ linux/drivers/md/raid1.c	2012-03-12 10:16:50.276610783 +0800
@@ -673,7 +673,12 @@ static void flush_pending_writes(struct
 		while (bio) { /* submit pending writes */
 			struct bio *next = bio->bi_next;
 			bio->bi_next = NULL;
-			generic_make_request(bio);
+			if (unlikely((bio->bi_rw & REQ_DISCARD) &&
+			    !blk_queue_discard(bdev_get_queue(bio->bi_bdev))))
+				/* Just ignore it */
+				bio_endio(bio, 0);
+			else
+				generic_make_request(bio);
 			bio = next;
 		}
 	} else
@@ -835,6 +840,7 @@ static void make_request(struct mddev *m
 	const int rw = bio_data_dir(bio);
 	const unsigned long do_sync = (bio->bi_rw & REQ_SYNC);
 	const unsigned long do_flush_fua = (bio->bi_rw & (REQ_FLUSH | REQ_FUA));
+	const unsigned long do_discard = (bio->bi_rw & (REQ_DISCARD | REQ_SECURE));
 	struct md_rdev *blocked_rdev;
 	int plugged;
 	int first_clone;
@@ -1135,7 +1141,7 @@ read_again:
 				   conf->mirrors[i].rdev->data_offset);
 		mbio->bi_bdev = conf->mirrors[i].rdev->bdev;
 		mbio->bi_end_io	= raid1_end_write_request;
-		mbio->bi_rw = WRITE | do_flush_fua | do_sync;
+		mbio->bi_rw = WRITE | do_flush_fua | do_sync | do_discard;
 		mbio->bi_private = r1_bio;
 
 		atomic_inc(&r1_bio->remaining);
@@ -1371,6 +1377,8 @@ static int raid1_add_disk(struct mddev *
 		}
 	}
 	md_integrity_add_rdev(rdev, mddev);
+	if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
 	print_conf(conf);
 	return err;
 }
@@ -2585,6 +2593,7 @@ static int run(struct mddev *mddev)
 	struct r1conf *conf;
 	int i;
 	struct md_rdev *rdev;
+	bool discard_supported = false;
 
 	if (mddev->level != 1) {
 		printk(KERN_ERR "md/raid1:%s: raid level not set to mirroring (%d)\n",
@@ -2623,8 +2632,16 @@ static int run(struct mddev *mddev)
 			blk_queue_segment_boundary(mddev->queue,
 						   PAGE_CACHE_SIZE - 1);
 		}
+
+		if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
+			discard_supported = true;
 	}
 
+	if (discard_supported)
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+	else
+		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+
 	mddev->degraded = 0;
 	for (i=0; i < conf->raid_disks; i++)
 		if (conf->mirrors[i].rdev == NULL ||

[patch v2 6/6] md: raid 10 supports TRIM

[Shaohua Li]

[-- Attachment #1: md-raid10-discard-support.patch --]
[-- Type: text/plain, Size: 3859 bytes --]

This makes md raid 10 support TRIM.
If one disk supports discard and another not, or one has discard_zero_data and
another not, there could be inconsistent between data from such disks. But this
should not matter, discarded data is useless. This will add extra copy in rebuild
though.

Signed-off-by: Shaohua Li <shli@fusionio.com>
---
 drivers/md/raid10.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Index: linux/drivers/md/raid10.c
===================================================================
--- linux.orig/drivers/md/raid10.c	2012-03-14 09:16:37.300435113 +0800
+++ linux/drivers/md/raid10.c	2012-03-14 09:33:44.017262275 +0800
@@ -800,7 +800,12 @@ static void flush_pending_writes(struct
 		while (bio) { /* submit pending writes */
 			struct bio *next = bio->bi_next;
 			bio->bi_next = NULL;
-			generic_make_request(bio);
+			if (unlikely((bio->bi_rw & REQ_DISCARD) &&
+			    !blk_queue_discard(bdev_get_queue(bio->bi_bdev))))
+				/* Just ignore it */
+				bio_endio(bio, 0);
+			else
+				generic_make_request(bio);
 			bio = next;
 		}
 	} else
@@ -926,6 +931,7 @@ static void make_request(struct mddev *m
 	const int rw = bio_data_dir(bio);
 	const unsigned long do_sync = (bio->bi_rw & REQ_SYNC);
 	const unsigned long do_fua = (bio->bi_rw & REQ_FUA);
+	const unsigned long do_discard = (bio->bi_rw & (REQ_DISCARD | REQ_SECURE));
 	unsigned long flags;
 	struct md_rdev *blocked_rdev;
 	int plugged;
@@ -945,7 +951,7 @@ static void make_request(struct mddev *m
 		    conf->near_copies < conf->raid_disks)) {
 		struct bio_pair *bp;
 		/* Sanity check -- queue functions should prevent this happening */
-		if (bio->bi_vcnt != 1 ||
+		if ((bio->bi_vcnt != 1 && bio->bi_vcnt !=0) ||
 		    bio->bi_idx != 0)
 			goto bad_map;
 		/* This is a one page bio that upper layers
@@ -1241,7 +1247,7 @@ retry_write:
 				   conf->mirrors[d].rdev->data_offset);
 		mbio->bi_bdev = conf->mirrors[d].rdev->bdev;
 		mbio->bi_end_io	= raid10_end_write_request;
-		mbio->bi_rw = WRITE | do_sync | do_fua;
+		mbio->bi_rw = WRITE | do_sync | do_fua | do_discard;
 		mbio->bi_private = r10_bio;
 
 		atomic_inc(&r10_bio->remaining);
@@ -1266,7 +1272,7 @@ retry_write:
 				   conf->mirrors[d].replacement->data_offset);
 		mbio->bi_bdev = conf->mirrors[d].replacement->bdev;
 		mbio->bi_end_io	= raid10_end_write_request;
-		mbio->bi_rw = WRITE | do_sync | do_fua;
+		mbio->bi_rw = WRITE | do_sync | do_fua | do_discard;
 		mbio->bi_private = r10_bio;
 
 		atomic_inc(&r10_bio->remaining);
@@ -1543,6 +1549,9 @@ static int raid10_add_disk(struct mddev
 	}
 
 	md_integrity_add_rdev(rdev, mddev);
+	if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+
 	print_conf(conf);
 	return err;
 }
@@ -3214,6 +3223,7 @@ static int run(struct mddev *mddev)
 	struct mirror_info *disk;
 	struct md_rdev *rdev;
 	sector_t size;
+	bool discard_supported = false;
 
 	/*
 	 * copy the already verified devices into our private RAID10
@@ -3234,6 +3244,7 @@ static int run(struct mddev *mddev)
 	mddev->thread = conf->thread;
 	conf->thread = NULL;
 
+	blk_queue_max_discard_sectors(mddev->queue, mddev->chunk_sectors);
 	chunk_size = mddev->chunk_sectors << 9;
 	blk_queue_io_min(mddev->queue, chunk_size);
 	if (conf->raid_disks % conf->near_copies)
@@ -3273,7 +3284,16 @@ static int run(struct mddev *mddev)
 		}
 
 		disk->head_position = 0;
+
+		if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
+			discard_supported = true;
 	}
+
+	if (discard_supported)
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+	else
+		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
+
 	/* need to check that every block has at least one working mirror */
 	if (!enough(conf, -1)) {
 		printk(KERN_ERR "md/raid10:%s: not enough operational mirrors.\n",

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Holger Kiehl]

Hello,

On Fri, 16 Mar 2012, Shaohua Li wrote:

> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM support for
> raid 4/5/6 later. The implementation is pretty straightforward and
> self-explained.
>
> v1->v2:
> 1. fixed a checking issue
> 2. dropped discard request plug and replace it with no discard merege, because
> current SCSI layer can't handle discard request merge.
>
Have tested TRIM patches on three different systems with the following
hardware/ setup:

    1) root mounted on a raid1 over two SAS SSD's (200GB) and /home partition
       on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
       little usage.

    2) root and /home mounted on a raid0 over two Intel X25 Postville
       (160GB) connected to a Intel P55 Express chipset. Has seen very
       heavy usage for approx. 2 years.

    3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
       connected via ICH7 south bridge. Has seen mild usage for approx.
       1.5 years.

Made the following observations when running my own benchmark which
copies around a lot of small files and deletes them. The benchmark on
all systems was always run only on the /home partition ie. on a raid0.

For system 1) there is hardly any measurable differnce whether discard
is enabled or not (~29000 files per second).

On system 2) the performance drops from 6500->3700 files per second,
but under normal usage one does not notice any difference.

System 3) has problems during boot, it is so slow that some operations
receive a timeout during boot:

   udevd[474]: timeout '/sbin/blkid -o udev -p /dev/md0'
   udevd[474]: timeout: killing '/sbin/blkid -o udev -p /dev/md0' [866]
   systemd[1]: dev-md3.swap activation timed out. Stopping.

Even removing discard does not help and the above errors happen during
boot and booting takes a long time.

The performance in the benchmark drops from 4000->600 files per second.

Regards,
Holger

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Shaohua Li]

2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
> Hello,
>
>
> On Fri, 16 Mar 2012, Shaohua Li wrote:
>
>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM support
>> for
>> raid 4/5/6 later. The implementation is pretty straightforward and
>> self-explained.
>>
>> v1->v2:
>> 1. fixed a checking issue
>> 2. dropped discard request plug and replace it with no discard merege,
>> because
>> current SCSI layer can't handle discard request merge.
>>
> Have tested TRIM patches on three different systems with the following
> hardware/ setup:
>
>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home partition
>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>      little usage.
>
>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>      heavy usage for approx. 2 years.
>
>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>      connected via ICH7 south bridge. Has seen mild usage for approx.
>      1.5 years.
>
> Made the following observations when running my own benchmark which
> copies around a lot of small files and deletes them. The benchmark on
> all systems was always run only on the /home partition ie. on a raid0.
>
> For system 1) there is hardly any measurable differnce whether discard
> is enabled or not (~29000 files per second).
>
> On system 2) the performance drops from 6500->3700 files per second,
> but under normal usage one does not notice any difference.
do you have the blktrace data when the benchmark is running, especially
when doing file deletion. I'd like to check the latency of discard in this case.

> System 3) has problems during boot, it is so slow that some operations
> receive a timeout during boot:
>
>  udevd[474]: timeout '/sbin/blkid -o udev -p /dev/md0'
>  udevd[474]: timeout: killing '/sbin/blkid -o udev -p /dev/md0' [866]
>  systemd[1]: dev-md3.swap activation timed out. Stopping.
In this one, discard request is slow. And per SATA standard, discard request
can't be parallel, so only one request one time, which further slows it down.

> Even removing discard does not help and the above errors happen during
> boot and booting takes a long time.
this doesn't make sense. If you don't mount with discard option, no
discard request
is issued.

Thanks,
Shaohua

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Holger Kiehl]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2861 bytes --]

Hello,

On Tue, 20 Mar 2012, Shaohua Li wrote:

> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>> Hello,
>>
>>
>> On Fri, 16 Mar 2012, Shaohua Li wrote:
>>
>>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM support
>>> for
>>> raid 4/5/6 later. The implementation is pretty straightforward and
>>> self-explained.
>>>
>>> v1->v2:
>>> 1. fixed a checking issue
>>> 2. dropped discard request plug and replace it with no discard merege,
>>> because
>>> current SCSI layer can't handle discard request merge.
>>>
>> Have tested TRIM patches on three different systems with the following
>> hardware/ setup:
>>
>>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home partition
>>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>>      little usage.
>>
>>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>>      heavy usage for approx. 2 years.
>>
>>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>>      connected via ICH7 south bridge. Has seen mild usage for approx.
>>      1.5 years.
>>
>> Made the following observations when running my own benchmark which
>> copies around a lot of small files and deletes them. The benchmark on
>> all systems was always run only on the /home partition ie. on a raid0.
>>
>> For system 1) there is hardly any measurable differnce whether discard
>> is enabled or not (~29000 files per second).
>>
>> On system 2) the performance drops from 6500->3700 files per second,
>> but under normal usage one does not notice any difference.
> do you have the blktrace data when the benchmark is running, especially
> when doing file deletion. I'd like to check the latency of discard in this case.
>
It is uploaded on ftp://ftp.dwd.de/pub/afd/test/trim

>> System 3) has problems during boot, it is so slow that some operations
>> receive a timeout during boot:
>>
>>  udevd[474]: timeout '/sbin/blkid -o udev -p /dev/md0'
>>  udevd[474]: timeout: killing '/sbin/blkid -o udev -p /dev/md0' [866]
>>  systemd[1]: dev-md3.swap activation timed out. Stopping.
> In this one, discard request is slow. And per SATA standard, discard request
> can't be parallel, so only one request one time, which further slows it down.
>
Ok, so having three disc in a raid 0 is even worth.

>> Even removing discard does not help and the above errors happen during
>> boot and booting takes a long time.
> this doesn't make sense. If you don't mount with discard option, no
> discard request
> is issued.
>
I read some where that swap uses discard by default and if I remember
correctly it initializes the whole swap area at boot. So doing that
over three disks might explain why I get those timeout error messages
and after boot these commands work without delay.

Regards,
Holger

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Shaohua Li]

2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
> Hello,
>
>
> On Tue, 20 Mar 2012, Shaohua Li wrote:
>
>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>>
>>> Hello,
>>>
>>>
>>> On Fri, 16 Mar 2012, Shaohua Li wrote:
>>>
>>>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
>>>> support
>>>> for
>>>> raid 4/5/6 later. The implementation is pretty straightforward and
>>>> self-explained.
>>>>
>>>> v1->v2:
>>>> 1. fixed a checking issue
>>>> 2. dropped discard request plug and replace it with no discard merege,
>>>> because
>>>> current SCSI layer can't handle discard request merge.
>>>>
>>> Have tested TRIM patches on three different systems with the following
>>> hardware/ setup:
>>>
>>>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home
>>> partition
>>>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>>>      little usage.
>>>
>>>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>>>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>>>      heavy usage for approx. 2 years.
>>>
>>>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>>>      connected via ICH7 south bridge. Has seen mild usage for approx.
>>>      1.5 years.
>>>
>>> Made the following observations when running my own benchmark which
>>> copies around a lot of small files and deletes them. The benchmark on
>>> all systems was always run only on the /home partition ie. on a raid0.
>>>
>>> For system 1) there is hardly any measurable differnce whether discard
>>> is enabled or not (~29000 files per second).
>>>
>>> On system 2) the performance drops from 6500->3700 files per second,
>>> but under normal usage one does not notice any difference.
>>
>> do you have the blktrace data when the benchmark is running, especially
>> when doing file deletion. I'd like to check the latency of discard in this
>> case.
>>
> It is uploaded on ftp://ftp.dwd.de/pub/afd/test/trim
Thanks, I'll check it.

>>> System 3) has problems during boot, it is so slow that some operations
>>> receive a timeout during boot:
>>>
>>>  udevd[474]: timeout '/sbin/blkid -o udev -p /dev/md0'
>>>  udevd[474]: timeout: killing '/sbin/blkid -o udev -p /dev/md0' [866]
>>>  systemd[1]: dev-md3.swap activation timed out. Stopping.
>>
>> In this one, discard request is slow. And per SATA standard, discard
>> request
>> can't be parallel, so only one request one time, which further slows it
>> down.
>>
> Ok, so having three disc in a raid 0 is even worth.
>
>
>>> Even removing discard does not help and the above errors happen during
>>> boot and booting takes a long time.
>>
>> this doesn't make sense. If you don't mount with discard option, no
>> discard request
>> is issued.
>>
> I read some where that swap uses discard by default and if I remember
> correctly it initializes the whole swap area at boot. So doing that
> over three disks might explain why I get those timeout error messages
> and after boot these commands work without delay.
This looks a bug to me. if swapon doesn't add discard option, we shouldn't
do discard. Let me post a patch, and see how people think.

Thanks,
Shaohua

Re: [patch v2 2/6] blk: dont allow discard request merge temporarily

[Vivek Goyal]

On Fri, Mar 16, 2012 at 03:32:15PM +0800, Shaohua Li wrote:
> Didn't allow discard request merge temporarily, as SCSI layer isn't ready
> for discard merge as Martin Petersen pointed out. This isn't fair for
> non-scsi device, but looks this is the only way I can do currently.
> 
> We should have the same issue before, but maybe because discard merge is
> very rare case. But now raid0/10 makes the merge quite possible, so we need
> disable it explicitly.
> 
> Signed-off-by: Shaohua Li <shli@fusionio.com>
> ---
>  include/linux/blkdev.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/include/linux/blkdev.h
> ===================================================================
> --- linux.orig/include/linux/blkdev.h	2012-03-14 09:20:06.787261188 +0800
> +++ linux/include/linux/blkdev.h	2012-03-14 09:20:47.797261248 +0800
> @@ -575,7 +575,7 @@ static inline void blk_clear_queue_full(
>   * it already be started by driver.
>   */
>  #define RQ_NOMERGE_FLAGS	\
> -	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
> +	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA | REQ_DISCARD)
>  #define rq_mergeable(rq)	\
>  	(!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \
>  	 (((rq)->cmd_flags & REQ_DISCARD) || \

I think you will need to do little more cleanup to make discard
unmergeable.

- Change rq_mergeable(rq)
- Change attempt_merge() and get rid of special conditions of allowing
  discard merge.

Martin had a bigger patch where he wanted to cleanup many discard specific
condition checks.

As you are just focusing on disabling merging for discard requests, you
might as well just pick the relevant pieces from the patch.

http://www.spinics.net/lists/linux-scsi/msg57779.html

Thanks
Vivek

Re: [patch v2 2/6] blk: dont allow discard request merge temporarily

[Shaohua Li]

2012/3/21 Vivek Goyal <vgoyal@redhat.com>:
> On Fri, Mar 16, 2012 at 03:32:15PM +0800, Shaohua Li wrote:
>> Didn't allow discard request merge temporarily, as SCSI layer isn't ready
>> for discard merge as Martin Petersen pointed out. This isn't fair for
>> non-scsi device, but looks this is the only way I can do currently.
>>
>> We should have the same issue before, but maybe because discard merge is
>> very rare case. But now raid0/10 makes the merge quite possible, so we need
>> disable it explicitly.
>>
>> Signed-off-by: Shaohua Li <shli@fusionio.com>
>> ---
>>  include/linux/blkdev.h |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: linux/include/linux/blkdev.h
>> ===================================================================
>> --- linux.orig/include/linux/blkdev.h 2012-03-14 09:20:06.787261188 +0800
>> +++ linux/include/linux/blkdev.h      2012-03-14 09:20:47.797261248 +0800
>> @@ -575,7 +575,7 @@ static inline void blk_clear_queue_full(
>>   * it already be started by driver.
>>   */
>>  #define RQ_NOMERGE_FLAGS     \
>> -     (REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
>> +     (REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA | REQ_DISCARD)
>>  #define rq_mergeable(rq)     \
>>       (!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \
>>        (((rq)->cmd_flags & REQ_DISCARD) || \
>
> I think you will need to do little more cleanup to make discard
> unmergeable.
>
> - Change rq_mergeable(rq)
> - Change attempt_merge() and get rid of special conditions of allowing
>  discard merge.
>
> Martin had a bigger patch where he wanted to cleanup many discard specific
> condition checks.
>
> As you are just focusing on disabling merging for discard requests, you
> might as well just pick the relevant pieces from the patch.
>
> http://www.spinics.net/lists/linux-scsi/msg57779.html
Thanks for pointing out the thread. I didn't think disabling discard merging
permanently is a good idea. We can't do the merge because that code isn't
ready (actually just for driver of SCSI). Enabling discard merge is required
for device with slow discard (and very helpful for raid), so I just want a
temporarily disabling for the merge. Just changing RQ_NOMERGE_FLAGS
is an easy workaround for this goal.

Thanks,
Shaohua

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Shaohua Li]

2012/3/20 Shaohua Li <shli@kernel.org>:
> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>> Hello,
>>
>>
>> On Tue, 20 Mar 2012, Shaohua Li wrote:
>>
>>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>>>
>>>> Hello,
>>>>
>>>>
>>>> On Fri, 16 Mar 2012, Shaohua Li wrote:
>>>>
>>>>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
>>>>> support
>>>>> for
>>>>> raid 4/5/6 later. The implementation is pretty straightforward and
>>>>> self-explained.
>>>>>
>>>>> v1->v2:
>>>>> 1. fixed a checking issue
>>>>> 2. dropped discard request plug and replace it with no discard merege,
>>>>> because
>>>>> current SCSI layer can't handle discard request merge.
>>>>>
>>>> Have tested TRIM patches on three different systems with the following
>>>> hardware/ setup:
>>>>
>>>>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home
>>>> partition
>>>>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>>>>      little usage.
>>>>
>>>>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>>>>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>>>>      heavy usage for approx. 2 years.
>>>>
>>>>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>>>>      connected via ICH7 south bridge. Has seen mild usage for approx.
>>>>      1.5 years.
>>>>
>>>> Made the following observations when running my own benchmark which
>>>> copies around a lot of small files and deletes them. The benchmark on
>>>> all systems was always run only on the /home partition ie. on a raid0.
>>>>
>>>> For system 1) there is hardly any measurable differnce whether discard
>>>> is enabled or not (~29000 files per second).
>>>>
>>>> On system 2) the performance drops from 6500->3700 files per second,
>>>> but under normal usage one does not notice any difference.
>>>
>>> do you have the blktrace data when the benchmark is running, especially
>>> when doing file deletion. I'd like to check the latency of discard in this
>>> case.
>>>
>> It is uploaded on ftp://ftp.dwd.de/pub/afd/test/trim
> Thanks, I'll check it.
Thanks for the testing. The trace data is very helpful. In the intel
SSD, trace data
shows a discard request uses about 1 ~ 3 ms. The filesystem suffers from
fragmentation too, so lots of small discard requests. When ext4 starts doing
discard, it usually uses more than 1 minutes. That's too bad.
If just looking one disk's trace data, there are some extra latencies between
two discard requests. The combined trace data of two disks show the latency
comes from waiting for another disk, so nothing abnormal. I thought we could
do an optimization for this case in the future.
So in summary, discard from the SSDs is slow. When your filesystem is
fragmented, the performance will be terrible.

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

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Roberto Spadim]

=/ well maybe with a sas disk it could be faster? maybe a pciexpress disk too?

Em 20 de março de 2012 23:08, Shaohua Li <shli@kernel.org> escreveu:
> 2012/3/20 Shaohua Li <shli@kernel.org>:
>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>> Hello,
>>>
>>>
>>> On Tue, 20 Mar 2012, Shaohua Li wrote:
>>>
>>>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>>>>
>>>>> Hello,
>>>>>
>>>>>
>>>>> On Fri, 16 Mar 2012, Shaohua Li wrote:
>>>>>
>>>>>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
>>>>>> support
>>>>>> for
>>>>>> raid 4/5/6 later. The implementation is pretty straightforward and
>>>>>> self-explained.
>>>>>>
>>>>>> v1->v2:
>>>>>> 1. fixed a checking issue
>>>>>> 2. dropped discard request plug and replace it with no discard merege,
>>>>>> because
>>>>>> current SCSI layer can't handle discard request merge.
>>>>>>
>>>>> Have tested TRIM patches on three different systems with the following
>>>>> hardware/ setup:
>>>>>
>>>>>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home
>>>>> partition
>>>>>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>>>>>      little usage.
>>>>>
>>>>>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>>>>>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>>>>>      heavy usage for approx. 2 years.
>>>>>
>>>>>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>>>>>      connected via ICH7 south bridge. Has seen mild usage for approx.
>>>>>      1.5 years.
>>>>>
>>>>> Made the following observations when running my own benchmark which
>>>>> copies around a lot of small files and deletes them. The benchmark on
>>>>> all systems was always run only on the /home partition ie. on a raid0.
>>>>>
>>>>> For system 1) there is hardly any measurable differnce whether discard
>>>>> is enabled or not (~29000 files per second).
>>>>>
>>>>> On system 2) the performance drops from 6500->3700 files per second,
>>>>> but under normal usage one does not notice any difference.
>>>>
>>>> do you have the blktrace data when the benchmark is running, especially
>>>> when doing file deletion. I'd like to check the latency of discard in this
>>>> case.
>>>>
>>> It is uploaded on ftp://ftp.dwd.de/pub/afd/test/trim
>> Thanks, I'll check it.
> Thanks for the testing. The trace data is very helpful. In the intel
> SSD, trace data
> shows a discard request uses about 1 ~ 3 ms. The filesystem suffers from
> fragmentation too, so lots of small discard requests. When ext4 starts doing
> discard, it usually uses more than 1 minutes. That's too bad.
> If just looking one disk's trace data, there are some extra latencies between
> two discard requests. The combined trace data of two disks show the latency
> comes from waiting for another disk, so nothing abnormal. I thought we could
> do an optimization for this case in the future.
> So in summary, discard from the SSDs is slow. When your filesystem is
> fragmented, the performance will be terrible.
>
> Thanks,
> Shaohua
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Roberto Spadim
Spadim Technology / SPAEmpresarial

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Mathias Burén]

On 21 March 2012 02:08, Shaohua Li <shli@kernel.org> wrote:
> 2012/3/20 Shaohua Li <shli@kernel.org>:
>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>> Hello,
>>>
>>>
>>> On Tue, 20 Mar 2012, Shaohua Li wrote:
>>>
<snip>
> Thanks for the testing. The trace data is very helpful. In the intel
> SSD, trace data
> shows a discard request uses about 1 ~ 3 ms. The filesystem suffers from
> fragmentation too, so lots of small discard requests. When ext4 starts doing
> discard, it usually uses more than 1 minutes. That's too bad.
> If just looking one disk's trace data, there are some extra latencies between
> two discard requests. The combined trace data of two disks show the latency
> comes from waiting for another disk, so nothing abnormal. I thought we could
> do an optimization for this case in the future.
> So in summary, discard from the SSDs is slow. When your filesystem is
> fragmented, the performance will be terrible.
>
> Thanks,
> Shaohua
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Many people prefer to have the filesystems mounted without discard,
then they run a TRIM command/script at night (or whenever the disk
isn't used very much). This way the TRIM commands don't slow down the
'overall' performance.

Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Mathias Burén]

On 21 March 2012 02:24, Roberto Spadim <roberto@spadim.com.br> wrote:
> =/ well maybe with a sas disk it could be faster? maybe a pciexpress disk too?
>
> Em 20 de março de 2012 23:08, Shaohua Li <shli@kernel.org> escreveu:
>> 2012/3/20 Shaohua Li <shli@kernel.org>:
>>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>>> Hello,
>>>>
>>>>
>>>> On Tue, 20 Mar 2012, Shaohua Li wrote:
>>>>
>>>>> 2012/3/20 Holger Kiehl <Holger.Kiehl@dwd.de>:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>>
>>>>>> On Fri, 16 Mar 2012, Shaohua Li wrote:
>>>>>>
>>>>>>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
>>>>>>> support
>>>>>>> for
>>>>>>> raid 4/5/6 later. The implementation is pretty straightforward and
>>>>>>> self-explained.
>>>>>>>
>>>>>>> v1->v2:
>>>>>>> 1. fixed a checking issue
>>>>>>> 2. dropped discard request plug and replace it with no discard merege,
>>>>>>> because
>>>>>>> current SCSI layer can't handle discard request merge.
>>>>>>>
>>>>>> Have tested TRIM patches on three different systems with the following
>>>>>> hardware/ setup:
>>>>>>
>>>>>>   1) root mounted on a raid1 over two SAS SSD's (200GB) and /home
>>>>>> partition
>>>>>>      on a raid0 over a fusionio ioDrive Duo. Is very new and seen very
>>>>>>      little usage.
>>>>>>
>>>>>>   2) root and /home mounted on a raid0 over two Intel X25 Postville
>>>>>>      (160GB) connected to a Intel P55 Express chipset. Has seen very
>>>>>>      heavy usage for approx. 2 years.
>>>>>>
>>>>>>   3) root and /home mounted on a raid0 over three OCZ-VERTEX2 (120GB)
>>>>>>      connected via ICH7 south bridge. Has seen mild usage for approx.
>>>>>>      1.5 years.
>>>>>>
>>>>>> Made the following observations when running my own benchmark which
>>>>>> copies around a lot of small files and deletes them. The benchmark on
>>>>>> all systems was always run only on the /home partition ie. on a raid0.
>>>>>>
>>>>>> For system 1) there is hardly any measurable differnce whether discard
>>>>>> is enabled or not (~29000 files per second).
>>>>>>
>>>>>> On system 2) the performance drops from 6500->3700 files per second,
>>>>>> but under normal usage one does not notice any difference.
>>>>>
>>>>> do you have the blktrace data when the benchmark is running, especially
>>>>> when doing file deletion. I'd like to check the latency of discard in this
>>>>> case.
>>>>>
>>>> It is uploaded on ftp://ftp.dwd.de/pub/afd/test/trim
>>> Thanks, I'll check it.
>> Thanks for the testing. The trace data is very helpful. In the intel
>> SSD, trace data
>> shows a discard request uses about 1 ~ 3 ms. The filesystem suffers from
>> fragmentation too, so lots of small discard requests. When ext4 starts doing
>> discard, it usually uses more than 1 minutes. That's too bad.
>> If just looking one disk's trace data, there are some extra latencies between
>> two discard requests. The combined trace data of two disks show the latency
>> comes from waiting for another disk, so nothing abnormal. I thought we could
>> do an optimization for this case in the future.
>> So in summary, discard from the SSDs is slow. When your filesystem is
>> fragmented, the performance will be terrible.
>>
>> Thanks,
>> Shaohua
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Roberto Spadim
> Spadim Technology / SPAEmpresarial
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Not relevant.

Mathias

Re: [patch v2 2/6] blk: dont allow discard request merge temporarily

[Martin K. Petersen]

>>>>> "Shaohua" == Shaohua Li <shli@kernel.org> writes:

Shaohua> Enabling discard merge is required for device with slow discard
Shaohua> (and very helpful for raid),

Before RAID support there really wasn't much point. That's one of the
reasons the current discard merge code has survived despite being
completely broken.

As for how I'd like to handle contiguous discard merges going forward
here's a proof of concept. It's on top of my write same tree so you may
have to noodle a bit.

It is crucially important that we never merge a command that can't be
processed by the device. So I'd like to do something like this:


block: Consolidate command flag and queue limit checks for merges

 - blk_check_merge_flags() verifies that cmd_flags / bi_rw are
   compatible. This function is called for both req-req and req-bio
   merging.

 - blk_rq_get_max_sectors() and blk_queue_get_max_sectors() can be used
   to query the maximum sector count for a given request or queue. The
   calls will return the right value from the queue limits given the
   type of command (RW, discard, write same, etc.)

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/block/blk-core.c b/block/blk-core.c
index 1cd55be..6619a6d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1757,8 +1757,7 @@ int blk_rq_check_limits(struct request_queue *q, struct request *rq)
 	if (!rq_mergeable(rq))
 		return 0;
 
-	if (blk_rq_sectors(rq) > queue_max_sectors(q) ||
-	    blk_rq_bytes(rq) > queue_max_hw_sectors(q) << 9) {
+	if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, rq->cmd_flags)) {
 		printk(KERN_ERR "%s: over max size limit.\n", __func__);
 		return -EIO;
 	}
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 9172606..8bd91d2 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -228,14 +228,8 @@ no_merge:
 int ll_back_merge_fn(struct request_queue *q, struct request *req,
 		     struct bio *bio)
 {
-	unsigned short max_sectors;
-
-	if (unlikely(req->cmd_type == REQ_TYPE_BLOCK_PC))
-		max_sectors = queue_max_hw_sectors(q);
-	else
-		max_sectors = queue_max_sectors(q);
-
-	if (blk_rq_sectors(req) + bio_sectors(bio) > max_sectors) {
+	if (blk_rq_sectors(req) + bio_sectors(bio) >
+	    blk_rq_get_max_sectors(req)) {
 		req->cmd_flags |= REQ_NOMERGE;
 		if (req == q->last_merge)
 			q->last_merge = NULL;
@@ -252,15 +246,8 @@ int ll_back_merge_fn(struct request_queue *q, struct request *req,
 int ll_front_merge_fn(struct request_queue *q, struct request *req,
 		      struct bio *bio)
 {
-	unsigned short max_sectors;
-
-	if (unlikely(req->cmd_type == REQ_TYPE_BLOCK_PC))
-		max_sectors = queue_max_hw_sectors(q);
-	else
-		max_sectors = queue_max_sectors(q);
-
-
-	if (blk_rq_sectors(req) + bio_sectors(bio) > max_sectors) {
+	if (blk_rq_sectors(req) + bio_sectors(bio) >
+	    blk_rq_get_max_sectors(req)) {
 		req->cmd_flags |= REQ_NOMERGE;
 		if (req == q->last_merge)
 			q->last_merge = NULL;
@@ -291,7 +278,8 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
 	/*
 	 * Will it become too large?
 	 */
-	if ((blk_rq_sectors(req) + blk_rq_sectors(next)) > queue_max_sectors(q))
+	if ((blk_rq_sectors(req) + blk_rq_sectors(next)) >
+	    blk_rq_get_max_sectors(req))
 		return 0;
 
 	total_phys_segments = req->nr_phys_segments + next->nr_phys_segments;
@@ -370,6 +358,9 @@ static int attempt_merge(struct request_queue *q, struct request *req,
 	if (!rq_mergeable(req) || !rq_mergeable(next))
 		return 0;
 
+	if (!blk_check_merge_flags(req->cmd_flags, next->cmd_flags))
+		return 0;
+
 	/*
 	 * not contiguous
 	 */
@@ -465,6 +456,9 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
 	if (!rq_mergeable(rq) || !bio_mergeable(bio))
 		return false;
 
+	if (!blk_check_merge_flags(rq->cmd_flags, bio->bi_rw))
+		return false;
+
 	/* different data direction or already started, don't merge */
 	if (bio_data_dir(bio) != rq_data_dir(rq))
 		return false;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 7718e69..b7994eb 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -175,8 +175,7 @@ enum rq_flag_bits {
 
 /* This mask is used for both bio and request merge checking */
 #define REQ_NOMERGE_FLAGS \
-	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA | \
-	 REQ_DISCARD | REQ_WRITE_SAME)
+	(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
 
 #define REQ_RAHEAD		(1 << __REQ_RAHEAD)
 #define REQ_THROTTLED		(1 << __REQ_THROTTLED)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 92956b7..cf11980 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -580,6 +580,20 @@ static inline bool rq_mergeable(struct request *rq)
 	return true;
 }
 
+static inline bool blk_check_merge_flags(unsigned int flags1,
+					 unsigned int flags2)
+{
+	if ((flags1 & REQ_DISCARD) != (flags2 & REQ_DISCARD))
+		return false;
+
+	if ((flags1 & REQ_SECURE) != (flags2 & REQ_SECURE))
+		return false;
+
+	if ((flags1 & REQ_WRITE_SAME) != (flags2 & REQ_WRITE_SAME))
+		return false;
+
+	return true;
+}
 
 /*
  * q->prep_rq_fn return values
@@ -776,6 +790,28 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
 	return blk_rq_cur_bytes(rq) >> 9;
 }
 
+static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
+						     unsigned int cmd_flags)
+{
+	if (unlikely(cmd_flags & REQ_DISCARD))
+		return q->limits.max_discard_sectors;
+
+	if (unlikely(cmd_flags & REQ_WRITE_SAME))
+		return q->limits.max_write_same_sectors;
+
+	return q->limits.max_sectors;
+}
+
+static inline unsigned int blk_rq_get_max_sectors(struct request *rq)
+{
+	struct request_queue *q = rq->q;
+
+	if (unlikely(rq->cmd_type == REQ_TYPE_BLOCK_PC))
+		return q->limits.max_hw_sectors;
+
+	return blk_queue_get_max_sectors(q, rq->cmd_flags);
+}
+
 /*
  * Request issue related functions.
  */

Re: [patch v2 2/6] blk: dont allow discard request merge temporarily

[Shaohua Li]

2012/3/22 Martin K. Petersen <martin.petersen@oracle.com>:
>>>>>> "Shaohua" == Shaohua Li <shli@kernel.org> writes:
>
> Shaohua> Enabling discard merge is required for device with slow discard
> Shaohua> (and very helpful for raid),
>
> Before RAID support there really wasn't much point. That's one of the
> reasons the current discard merge code has survived despite being
> completely broken.
Exactly.

> As for how I'd like to handle contiguous discard merges going forward
> here's a proof of concept. It's on top of my write same tree so you may
> have to noodle a bit.
>
> It is crucially important that we never merge a command that can't be
> processed by the device. So I'd like to do something like this:
This is great to consolidate the check with this patch. But looks this
isn't enough. As far as I know the current discard merge is broken
because blk_update_request() can't handle such request because
we override the first BIO to add payload. I had a quick hack and post
out yesterday. It works in my SATA ssd, but I really didn't know if there
is something missing in SCSI side, can you share anything here?

Thanks,
Shaohua

Re: [patch v2 2/6] blk: dont allow discard request merge temporarily

[Christoph Hellwig]

On Wed, Mar 21, 2012 at 10:18:54PM -0400, Martin K. Petersen wrote:
> >>>>> "Shaohua" == Shaohua Li <shli@kernel.org> writes:
> 
> Shaohua> Enabling discard merge is required for device with slow discard
> Shaohua> (and very helpful for raid),
> 
> Before RAID support there really wasn't much point. That's one of the
> reasons the current discard merge code has survived despite being
> completely broken.

I've actually send repeated bug reports about it beeing broken when
I enabled async discard in XFS which led to merges, and debugged it down
to beeing a problem with merges.  But Jens didn't like my patch to mark
them nomerged, so we still couldn't commit the async discard support for
XFS which would have helped quite a bit with discard performance.

RE: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Patelczyk, Maciej]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of Shaohua Li
> Sent: Friday, March 16, 2012 8:32 AM
> To: linux-kernel@vger.kernel.org; linux-raid@vger.kernel.org
> Cc: neilb@suse.de; axboe@kernel.dk; vgoyal@redhat.com;
> martin.petersen@oracle.com
> Subject: [patch v2 0/6] Add TRIM support for raid linear/0/1/10
> 
> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
> support for raid 4/5/6 later. The implementation is pretty
> straightforward and self-explained.
> 
> v1->v2:
> 1. fixed a checking issue
> 2. dropped discard request plug and replace it with no discard merege,
> because current SCSI layer can't handle discard request merge.
> 
> Thanks,
> Shaohua

Hi,
Can't see anywhere patch 2/6 from this series.


maciej

Re: [patch v2 0/6] Add TRIM support for raid linear/0/1/10

[Shaohua Li]

2012/5/8 Patelczyk, Maciej <maciej.patelczyk@intel.com>:
>> -----Original Message-----
>> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
>> owner@vger.kernel.org] On Behalf Of Shaohua Li
>> Sent: Friday, March 16, 2012 8:32 AM
>> To: linux-kernel@vger.kernel.org; linux-raid@vger.kernel.org
>> Cc: neilb@suse.de; axboe@kernel.dk; vgoyal@redhat.com;
>> martin.petersen@oracle.com
>> Subject: [patch v2 0/6] Add TRIM support for raid linear/0/1/10
>>
>> The patches add TRIM support for raid linear/0/1/10. I'll add TRIM
>> support for raid 4/5/6 later. The implementation is pretty
>> straightforward and self-explained.
>>
>> v1->v2:
>> 1. fixed a checking issue
>> 2. dropped discard request plug and replace it with no discard merege,
>> because current SCSI layer can't handle discard request merge.
>>
>> Thanks,
>> Shaohua
>
> Hi,
> Can't see anywhere patch 2/6 from this series.
http://marc.info/?l=linux-kernel&m=133188350823292&w=2

Thanks,
Shaohua