[1/1] block: Add discard to stats.

Commit Message

Michael Callahan May 5, 2016, 4:33 a.m. UTC

Separate out discards from writes counts in /block/diskstats as well as
the disk related /sys stat files.

Sign-off-by: Michael Callahan <[e-mail]>
---
The STAT_* macros might be better defined somewhere else.

The part_*_in_flight macros should probably be per_cpu.  I mixed up a
patch to test that but they are used awkwardly in drivers/md/dm.c and
accumulating inflights on cpus would be susceptible to overflow errors
on 32 bit machines (increment on one cpu, complete on another, they sum
correctly until one overflows)

This patch adds the new stats to the end of stat and diskstat for more
backwards compatibility.  However this patch breaks iostat which will
need to be updated to grab the additional fields.  iostat already has
cases for 4 and 11 entries in stat so adding another for 15 should be
easy enough and it's likely that adding these to the end of the file
rather than right after the write stats is unnecessary.

Needs Documentation update.

--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jeff Moyer May 5, 2016, 3:27 p.m. UTC | #1

Michael Callahan <[e-mail]> writes:

> Separate out discards from writes counts in /block/diskstats as well as
> the disk related /sys stat files.
>
> Sign-off-by: Michael Callahan <[e-mail]>
> ---
> The STAT_* macros might be better defined somewhere else.
>
> The part_*_in_flight macros should probably be per_cpu.  I mixed up a
> patch to test that but they are used awkwardly in drivers/md/dm.c and
> accumulating inflights on cpus would be susceptible to overflow errors
> on 32 bit machines (increment on one cpu, complete on another, they sum
> correctly until one overflows)
>
> This patch adds the new stats to the end of stat and diskstat for more
> backwards compatibility.  However this patch breaks iostat which will
> need to be updated to grab the additional fields.  iostat already has
> cases for 4 and 11 entries in stat so adding another for 15 should be
> easy enough and it's likely that adding these to the end of the file
> rather than right after the write stats is unnecessary.

You can't just break iostat.  Split the fields out into another file.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michael Callahan May 5, 2016, 3:59 p.m. UTC | #2

On Thu, May 5, 2016 at 9:27 AM, Jeff Moyer <[e-mail]> wrote:
> Michael Callahan <[e-mail]> writes:
>
>> Separate out discards from writes counts in /block/diskstats as well as
>> the disk related /sys stat files.
>>
>> Sign-off-by: Michael Callahan <[e-mail]>
>> ---
>> The STAT_* macros might be better defined somewhere else.
>>
>> The part_*_in_flight macros should probably be per_cpu.  I mixed up a
>> patch to test that but they are used awkwardly in drivers/md/dm.c and
>> accumulating inflights on cpus would be susceptible to overflow errors
>> on 32 bit machines (increment on one cpu, complete on another, they sum
>> correctly until one overflows)
>>
>> This patch adds the new stats to the end of stat and diskstat for more
>> backwards compatibility.  However this patch breaks iostat which will
>> need to be updated to grab the additional fields.  iostat already has
>> cases for 4 and 11 entries in stat so adding another for 15 should be
>> easy enough and it's likely that adding these to the end of the file
>> rather than right after the write stats is unnecessary.
>
> You can't just break iostat.  Split the fields out into another file.
>
> Cheers,
> Jeff

Would it be better to make a new stat2 file, make discard_stats, or to
patch iostat.c
to handle the new format?  It appears that iostat.c already checks for 4 or 11
stat fields and should be easy to extend to check for 15.

  Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jens Axboe May 5, 2016, 4:04 p.m. UTC | #3

On 05/05/2016 09:59 AM, Michael Callahan wrote:
> On Thu, May 5, 2016 at 9:27 AM, Jeff Moyer <[e-mail]> wrote:
>> Michael Callahan <[e-mail]> writes:
>>
>>> Separate out discards from writes counts in /block/diskstats as well as
>>> the disk related /sys stat files.
>>>
>>> Sign-off-by: Michael Callahan <[e-mail]>
>>> ---
>>> The STAT_* macros might be better defined somewhere else.
>>>
>>> The part_*_in_flight macros should probably be per_cpu.  I mixed up a
>>> patch to test that but they are used awkwardly in drivers/md/dm.c and
>>> accumulating inflights on cpus would be susceptible to overflow errors
>>> on 32 bit machines (increment on one cpu, complete on another, they sum
>>> correctly until one overflows)
>>>
>>> This patch adds the new stats to the end of stat and diskstat for more
>>> backwards compatibility.  However this patch breaks iostat which will
>>> need to be updated to grab the additional fields.  iostat already has
>>> cases for 4 and 11 entries in stat so adding another for 15 should be
>>> easy enough and it's likely that adding these to the end of the file
>>> rather than right after the write stats is unnecessary.
>>
>> You can't just break iostat.  Split the fields out into another file.
>>
>> Cheers,
>> Jeff
>
> Would it be better to make a new stat2 file, make discard_stats, or to
> patch iostat.c
> to handle the new format?  It appears that iostat.c already checks for 4 or 11
> stat fields and should be easy to extend to check for 15.

If the new format doesn't break iostat (and others), then we don't need 
a new file. If it does, we obviously do. We don't tie kernel and app 
upgrades together, so asking users to upgrade iostat because it breaks 
with a new kernel is not an option.

Don't add a specific discard stats.

Michael Callahan May 5, 2016, 4:21 p.m. UTC | #4

On Thu, May 5, 2016 at 10:04 AM, Jens Axboe <[e-mail]> wrote:
> On 05/05/2016 09:59 AM, Michael Callahan wrote:
>>
>> On Thu, May 5, 2016 at 9:27 AM, Jeff Moyer <[e-mail]> wrote:
>>>
>>> Michael Callahan <[e-mail]> writes:
>>>
>>>> Separate out discards from writes counts in /block/diskstats as well as
>>>> the disk related /sys stat files.
>>>>
>>>> Sign-off-by: Michael Callahan <[e-mail]>
>>>> ---
>>>> The STAT_* macros might be better defined somewhere else.
>>>>
>>>> The part_*_in_flight macros should probably be per_cpu.  I mixed up a
>>>> patch to test that but they are used awkwardly in drivers/md/dm.c and
>>>> accumulating inflights on cpus would be susceptible to overflow errors
>>>> on 32 bit machines (increment on one cpu, complete on another, they sum
>>>> correctly until one overflows)
>>>>
>>>> This patch adds the new stats to the end of stat and diskstat for more
>>>> backwards compatibility.  However this patch breaks iostat which will
>>>> need to be updated to grab the additional fields.  iostat already has
>>>> cases for 4 and 11 entries in stat so adding another for 15 should be
>>>> easy enough and it's likely that adding these to the end of the file
>>>> rather than right after the write stats is unnecessary.
>>>
>>>
>>> You can't just break iostat.  Split the fields out into another file.
>>>
>>> Cheers,
>>> Jeff
>>
>>
>> Would it be better to make a new stat2 file, make discard_stats, or to
>> patch iostat.c
>> to handle the new format?  It appears that iostat.c already checks for 4
>> or 11
>> stat fields and should be easy to extend to check for 15.
>
>
> If the new format doesn't break iostat (and others), then we don't need a
> new file. If it does, we obviously do. We don't tie kernel and app upgrades
> together, so asking users to upgrade iostat because it breaks with a new
> kernel is not an option.
>
> Don't add a specific discard stats.
>
> --
> Jens Axboe
>

Tested iostat more and it appears to work fine with this patch.
Looking at the code
iostat.c pulls the first 11 fields and ignores the new ones.  What
else needs to be
checked?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Moyer May 5, 2016, 5:29 p.m. UTC | #5

Michael Callahan <[e-mail]> writes:

> Tested iostat more and it appears to work fine with this patch.

What exactly did you test?  I've got sysstat 10.1.5 here, and with your
patch, I get no statistics output from iostat -x:

# iostat -x
Linux 4.6.0-rc5+ (slayer.lab.bos.redhat.com) 	05/05/16 	_x86_64_	(4 CPU)

avg-cpu:  %user   %nice %system %iowait  %steal   %idle
           0.31    0.00    0.17    0.52    0.00   99.00

Device:         rrqm/s   wrqm/s     r/s     w/s    rkB/s    wkB/s avgrq-sz avgqu-sz   await r_await w_await  svctm  %util

# 

I just grabbed 11.3.4, and it also shows the same behavior.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michael Callahan May 5, 2016, 5:48 p.m. UTC | #6

On Thu, May 5, 2016 at 11:29 AM, Jeff Moyer <[e-mail]> wrote:
> Michael Callahan <[e-mail]> writes:
>
>> Tested iostat more and it appears to work fine with this patch.
>
> What exactly did you test?  I've got sysstat 10.1.5 here, and with your
> patch, I get no statistics output from iostat -x:
>
> # iostat -x
> Linux 4.6.0-rc5+ (slayer.lab.bos.redhat.com)    05/05/16        _x86_64_        (4 CPU)
>
> avg-cpu:  %user   %nice %system %iowait  %steal   %idle
>            0.31    0.00    0.17    0.52    0.00   99.00
>
> Device:         rrqm/s   wrqm/s     r/s     w/s    rkB/s    wkB/s avgrq-sz avgqu-sz   await r_await w_await  svctm  %util
>
> #
>
> I just grabbed 11.3.4, and it also shows the same behavior.
>
> Cheers,
> Jeff

I tried 'iostat -p ALL' with and without the x.  I suppose the code
that automatically determines which devices to display could be
broken.

  Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michael Callahan May 5, 2016, 6:29 p.m. UTC | #7

So I dropped a handful of spaces in genhd.c:diskstats_show while I was
making it prettier.  Smashing them back in fixes iostat without the -p
option.  Sorry about that.  Any other similar programs to worry about?



On Thu, May 5, 2016 at 11:48 AM, Michael Callahan
<[e-mail]> wrote:
> On Thu, May 5, 2016 at 11:29 AM, Jeff Moyer <[e-mail]> wrote:
>> Michael Callahan <[e-mail]> writes:
>>
>>> Tested iostat more and it appears to work fine with this patch.
>>
>> What exactly did you test?  I've got sysstat 10.1.5 here, and with your
>> patch, I get no statistics output from iostat -x:
>>
>> # iostat -x
>> Linux 4.6.0-rc5+ (slayer.lab.bos.redhat.com)    05/05/16        _x86_64_        (4 CPU)
>>
>> avg-cpu:  %user   %nice %system %iowait  %steal   %idle
>>            0.31    0.00    0.17    0.52    0.00   99.00
>>
>> Device:         rrqm/s   wrqm/s     r/s     w/s    rkB/s    wkB/s avgrq-sz avgqu-sz   await r_await w_await  svctm  %util
>>
>> #
>>
>> I just grabbed 11.3.4, and it also shows the same behavior.
>>
>> Cheers,
>> Jeff
>
> I tried 'iostat -p ALL' with and without the x.  I suppose the code
> that automatically determines which devices to display could be
> broken.
>
>   Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Moyer May 5, 2016, 7:55 p.m. UTC | #8

Michael Callahan <[e-mail]> writes:

> So I dropped a handful of spaces in genhd.c:diskstats_show while I was
> making it prettier.  Smashing them back in fixes iostat without the -p
> option.  Sorry about that.  Any other similar programs to worry about?

vmstat is the only thing that immediately comes to mind.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michael Callahan May 6, 2016, 9:28 p.m. UTC | #9

Jens asked me to dice this up, resubmitting in pieces.


On Thu, May 5, 2016 at 1:55 PM, Jeff Moyer <[e-mail]> wrote:
> Michael Callahan <[e-mail]> writes:
>
>> So I dropped a handful of spaces in genhd.c:diskstats_show while I was
>> making it prettier.  Smashing them back in fixes iostat without the -p
>> option.  Sorry about that.  Any other similar programs to worry about?
>
> vmstat is the only thing that immediately comes to mind.
>
> Cheers,
> Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/block/bio.c b/block/bio.c
index 807d25e..eb568ff 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1678,27 +1678,29 @@  void bio_check_pages_dirty(struct bio *bio)
 	}
 }
 
-void generic_start_io_acct(int rw, unsigned long sectors,
+void generic_start_io_acct(int rwd, unsigned long sectors,
 			   struct hd_struct *part)
 {
+	int rw = !!rwd;
 	int cpu = part_stat_lock();
 
 	part_round_stats(cpu, part);
-	part_stat_inc(cpu, part, ios[rw]);
-	part_stat_add(cpu, part, sectors[rw], sectors);
+	part_stat_inc(cpu, part, ios[rwd]);
+	part_stat_add(cpu, part, sectors[rwd], sectors);
 	part_inc_in_flight(part, rw);
 
 	part_stat_unlock();
 }
 EXPORT_SYMBOL(generic_start_io_acct);
 
-void generic_end_io_acct(int rw, struct hd_struct *part,
+void generic_end_io_acct(int rwd, struct hd_struct *part,
 			 unsigned long start_time)
 {
 	unsigned long duration = jiffies - start_time;
+	int rw = !!rwd;
 	int cpu = part_stat_lock();
 
-	part_stat_add(cpu, part, ticks[rw], duration);
+	part_stat_add(cpu, part, ticks[rwd], duration);
 	part_round_stats(cpu, part);
 	part_dec_in_flight(part, rw);
 
diff --git a/block/blk-core.c b/block/blk-core.c
index b60537b..1a162d8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2273,13 +2273,13 @@  EXPORT_SYMBOL_GPL(blk_rq_err_bytes);
 void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	if (blk_do_io_stat(req)) {
-		const int rw = rq_data_dir(req);
+		const int rwd = rq_stat_group(req);
 		struct hd_struct *part;
 		int cpu;
 
 		cpu = part_stat_lock();
 		part = req->part;
-		part_stat_add(cpu, part, sectors[rw], bytes >> 9);
+		part_stat_add(cpu, part, sectors[rwd], bytes >> 9);
 		part_stat_unlock();
 	}
 }
@@ -2293,6 +2293,7 @@  void blk_account_io_done(struct request *req)
 	 */
 	if (blk_do_io_stat(req) && !(req->cmd_flags & REQ_FLUSH_SEQ)) {
 		unsigned long duration = jiffies - req->start_time;
+		const int rwd = rq_stat_group(req);
 		const int rw = rq_data_dir(req);
 		struct hd_struct *part;
 		int cpu;
@@ -2300,8 +2301,8 @@  void blk_account_io_done(struct request *req)
 		cpu = part_stat_lock();
 		part = req->part;
 
-		part_stat_inc(cpu, part, ios[rw]);
-		part_stat_add(cpu, part, ticks[rw], duration);
+		part_stat_inc(cpu, part, ios[rwd]);
+		part_stat_add(cpu, part, ticks[rwd], duration);
 		part_round_stats(cpu, part);
 		part_dec_in_flight(part, rw);
 
@@ -2335,6 +2336,7 @@  static inline struct request *blk_pm_peek_request(struct request_queue *q,
 void blk_account_io_start(struct request *rq, bool new_io)
 {
 	struct hd_struct *part;
+	int rwd = rq_stat_group(rq);
 	int rw = rq_data_dir(rq);
 	int cpu;
 
@@ -2345,7 +2347,7 @@  void blk_account_io_start(struct request *rq, bool new_io)
 
 	if (!new_io) {
 		part = rq->part;
-		part_stat_inc(cpu, part, merges[rw]);
+		part_stat_inc(cpu, part, merges[rwd]);
 	} else {
 		part = disk_map_sector_rcu(rq->rq_disk, blk_rq_pos(rq));
 		if (!hd_struct_try_get(part)) {
diff --git a/block/genhd.c b/block/genhd.c
index 9f42526..d470d8c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1195,21 +1195,29 @@  static int diskstats_show(struct seq_file *seqf, void *v)
 		cpu = part_stat_lock();
 		part_round_stats(cpu, hd);
 		part_stat_unlock();
-		seq_printf(seqf, "%4d %7d %s %lu %lu %lu "
-			   "%u %lu %lu %lu %u %u %u %u\n",
+		seq_printf(seqf, "%4d %7d %s"
+			   "%lu %lu %lu %u"
+			   "%lu %lu %lu %u"
+			   "%u %u %u"
+			   "%lu %lu %lu %u"
+			   "\n",
 			   MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
 			   disk_name(gp, hd->partno, buf),
-			   part_stat_read(hd, ios[READ]),
-			   part_stat_read(hd, merges[READ]),
-			   part_stat_read(hd, sectors[READ]),
-			   jiffies_to_msecs(part_stat_read(hd, ticks[READ])),
-			   part_stat_read(hd, ios[WRITE]),
-			   part_stat_read(hd, merges[WRITE]),
-			   part_stat_read(hd, sectors[WRITE]),
-			   jiffies_to_msecs(part_stat_read(hd, ticks[WRITE])),
+			   part_stat_read(hd, ios[STAT_READ]),
+			   part_stat_read(hd, merges[STAT_READ]),
+			   part_stat_read(hd, sectors[STAT_READ]),
+			   jiffies_to_msecs(part_stat_read(hd, ticks[STAT_READ])),
+			   part_stat_read(hd, ios[STAT_WRITE]),
+			   part_stat_read(hd, merges[STAT_WRITE]),
+			   part_stat_read(hd, sectors[STAT_WRITE]),
+			   jiffies_to_msecs(part_stat_read(hd, ticks[STAT_WRITE])),
 			   part_in_flight(hd),
 			   jiffies_to_msecs(part_stat_read(hd, io_ticks)),
-			   jiffies_to_msecs(part_stat_read(hd, time_in_queue))
+			   jiffies_to_msecs(part_stat_read(hd, time_in_queue)),
+			   part_stat_read(hd, ios[STAT_DISCARD]),
+			   part_stat_read(hd, merges[STAT_DISCARD]),
+			   part_stat_read(hd, sectors[STAT_DISCARD]),
+			   jiffies_to_msecs(part_stat_read(hd, ticks[STAT_DISCARD]))
 			);
 	}
 	disk_part_iter_exit(&piter);
diff --git a/block/partition-generic.c b/block/partition-generic.c
index d7eb77e..adaf3f7 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -122,18 +122,23 @@  ssize_t part_stat_show(struct device *dev,
 		"%8lu %8lu %8llu %8u "
 		"%8lu %8lu %8llu %8u "
 		"%8u %8u %8u"
+		"%8lu %8lu %8llu %8u "
 		"\n",
-		part_stat_read(p, ios[READ]),
-		part_stat_read(p, merges[READ]),
-		(unsigned long long)part_stat_read(p, sectors[READ]),
-		jiffies_to_msecs(part_stat_read(p, ticks[READ])),
-		part_stat_read(p, ios[WRITE]),
-		part_stat_read(p, merges[WRITE]),
-		(unsigned long long)part_stat_read(p, sectors[WRITE]),
-		jiffies_to_msecs(part_stat_read(p, ticks[WRITE])),
+		part_stat_read(p, ios[STAT_READ]),
+		part_stat_read(p, merges[STAT_READ]),
+		(unsigned long long)part_stat_read(p, sectors[STAT_READ]),
+		jiffies_to_msecs(part_stat_read(p, ticks[STAT_READ])),
+		part_stat_read(p, ios[STAT_WRITE]),
+		part_stat_read(p, merges[STAT_WRITE]),
+		(unsigned long long)part_stat_read(p, sectors[STAT_WRITE]),
+		jiffies_to_msecs(part_stat_read(p, ticks[STAT_WRITE])),
 		part_in_flight(p),
 		jiffies_to_msecs(part_stat_read(p, io_ticks)),
-		jiffies_to_msecs(part_stat_read(p, time_in_queue)));
+		jiffies_to_msecs(part_stat_read(p, time_in_queue)),
+		part_stat_read(p, ios[STAT_DISCARD]),
+		part_stat_read(p, merges[STAT_DISCARD]),
+		(unsigned long long)part_stat_read(p, sectors[STAT_DISCARD]),
+		jiffies_to_msecs(part_stat_read(p, ticks[STAT_DISCARD])));
 }
 
 ssize_t part_inflight_show(struct device *dev,
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 050aaa1..0de448c 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2504,8 +2504,9 @@  bool drbd_rs_c_min_rate_throttle(struct drbd_device *device)
 	if (c_min_rate == 0)
 		return false;
 
-	curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
-		      (int)part_stat_read(&disk->part0, sectors[1]) -
+	curr_events = (int)part_stat_read(&disk->part0, sectors[STAT_READ]) +
+		      (int)part_stat_read(&disk->part0, sectors[STAT_WRITE]) +
+		      (int)part_stat_read(&disk->part0, sectors[STAT_DISCARD]) -
 			atomic_read(&device->rs_sect_ev);
 
 	if (atomic_read(&device->ap_actlog_cnt)
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 2255dcf..ce81d8f 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -36,14 +36,14 @@  static bool drbd_may_do_local_read(struct drbd_device *device, sector_t sector,
 /* Update disk stats at start of I/O request */
 static void _drbd_start_io_acct(struct drbd_device *device, struct drbd_request *req)
 {
-	generic_start_io_acct(bio_data_dir(req->master_bio), req->i.size >> 9,
+	generic_start_io_acct(bio_stat_group(req->master_bio), req->i.size >> 9,
 			      &device->vdisk->part0);
 }
 
 /* Update disk stats when completing request upwards */
 static void _drbd_end_io_acct(struct drbd_device *device, struct drbd_request *req)
 {
-	generic_end_io_acct(bio_data_dir(req->master_bio),
+	generic_end_io_acct(bio_stat_group(req->master_bio),
 			    &device->vdisk->part0, req->start_jif);
 }
 
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 4d87499..c60de94 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1592,8 +1592,9 @@  void drbd_rs_controller_reset(struct drbd_device *device)
 	atomic_set(&device->rs_sect_ev, 0);
 	device->rs_in_flight = 0;
 	device->rs_last_events =
-		(int)part_stat_read(&disk->part0, sectors[0]) +
-		(int)part_stat_read(&disk->part0, sectors[1]);
+		(int)part_stat_read(&disk->part0, sectors[STAT_READ]) +
+		(int)part_stat_read(&disk->part0, sectors[STAT_WRITE]) +
+		(int)part_stat_read(&disk->part0, sectors[STAT_DISCARD]);
 
 	/* Updating the RCU protected object in place is necessary since
 	   this function gets called from atomic context.
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index e1b8b70..c9ee769 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -112,7 +112,7 @@  static const struct block_device_operations rsxx_fops = {
 
 static void disk_stats_start(struct rsxx_cardinfo *card, struct bio *bio)
 {
-	generic_start_io_acct(bio_data_dir(bio), bio_sectors(bio),
+	generic_start_io_acct(bio_stat_group(bio), bio_sectors(bio),
 			     &card->gendisk->part0);
 }
 
@@ -120,7 +120,7 @@  static void disk_stats_complete(struct rsxx_cardinfo *card,
 				struct bio *bio,
 				unsigned long start_time)
 {
-	generic_end_io_acct(bio_data_dir(bio), &card->gendisk->part0,
+	generic_end_io_acct(bio_stat_group(bio), &card->gendisk->part0,
 			   start_time);
 }
 
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 370c2f7..634f43f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -810,12 +810,12 @@  static void zram_bio_discard(struct zram *zram, u32 index,
 }
 
 static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
-			int offset, int rw)
+			int offset, int rw, int rwd)
 {
 	unsigned long start_time = jiffies;
 	int ret;
 
-	generic_start_io_acct(rw, bvec->bv_len >> SECTOR_SHIFT,
+	generic_start_io_acct(rwd, bvec->bv_len >> SECTOR_SHIFT,
 			&zram->disk->part0);
 
 	if (rw == READ) {
@@ -826,7 +826,7 @@  static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
 		ret = zram_bvec_write(zram, bvec, index, offset);
 	}
 
-	generic_end_io_acct(rw, &zram->disk->part0, start_time);
+	generic_end_io_acct(rwd, &zram->disk->part0, start_time);
 
 	if (unlikely(ret)) {
 		if (rw == READ)
@@ -840,7 +840,7 @@  static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
 
 static void __zram_make_request(struct zram *zram, struct bio *bio)
 {
-	int offset, rw;
+	int offset, rw, rwd;
 	u32 index;
 	struct bio_vec bvec;
 	struct bvec_iter iter;
@@ -856,6 +856,7 @@  static void __zram_make_request(struct zram *zram, struct bio *bio)
 	}
 
 	rw = bio_data_dir(bio);
+	rwd = bio_stat_group(bio);
 	bio_for_each_segment(bvec, bio, iter) {
 		int max_transfer_size = PAGE_SIZE - offset;
 
@@ -870,15 +871,15 @@  static void __zram_make_request(struct zram *zram, struct bio *bio)
 			bv.bv_len = max_transfer_size;
 			bv.bv_offset = bvec.bv_offset;
 
-			if (zram_bvec_rw(zram, &bv, index, offset, rw) < 0)
+			if (zram_bvec_rw(zram, &bv, index, offset, rw, rwd) < 0)
 				goto out;
 
 			bv.bv_len = bvec.bv_len - max_transfer_size;
 			bv.bv_offset += max_transfer_size;
-			if (zram_bvec_rw(zram, &bv, index + 1, 0, rw) < 0)
+			if (zram_bvec_rw(zram, &bv, index + 1, 0, rw, rwd) < 0)
 				goto out;
 		} else
-			if (zram_bvec_rw(zram, &bvec, index, offset, rw) < 0)
+			if (zram_bvec_rw(zram, &bvec, index, offset, rw, rwd) < 0)
 				goto out;
 
 		update_position(&index, &offset, &bvec);
@@ -959,7 +960,8 @@  static int zram_rw_page(struct block_device *bdev, sector_t sector,
 	bv.bv_len = PAGE_SIZE;
 	bv.bv_offset = 0;
 
-	err = zram_bvec_rw(zram, &bv, index, offset, rw);
+	/* Use rw as the stat group here as the page isn't being discarded. */
+	err = zram_bvec_rw(zram, &bv, index, offset, rw, rw);
 put_zram:
 	zram_meta_put(zram);
 out:
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 25fa844..8cc8d4f 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -609,7 +609,7 @@  static void request_endio(struct bio *bio)
 static void bio_complete(struct search *s)
 {
 	if (s->orig_bio) {
-		generic_end_io_acct(bio_data_dir(s->orig_bio),
+		generic_end_io_acct(bio_stat_group(s->orig_bio),
 				    &s->d->disk->part0, s->start_time);
 
 		trace_bcache_request_end(s->d, s->orig_bio);
@@ -966,7 +966,7 @@  static blk_qc_t cached_dev_make_request(struct request_queue *q,
 	struct cached_dev *dc = container_of(d, struct cached_dev, disk);
 	int rw = bio_data_dir(bio);
 
-	generic_start_io_acct(rw, bio_sectors(bio), &d->disk->part0);
+	generic_start_io_acct(bio_stat_group(bio), bio_sectors(bio), &d->disk->part0);
 
 	bio->bi_bdev = dc->bdev;
 	bio->bi_iter.bi_sector += dc->sb.data_offset;
@@ -1081,7 +1081,7 @@  static blk_qc_t flash_dev_make_request(struct request_queue *q,
 	struct bcache_device *d = bio->bi_bdev->bd_disk->private_data;
 	int rw = bio_data_dir(bio);
 
-	generic_start_io_acct(rw, bio_sectors(bio), &d->disk->part0);
+	generic_start_io_acct(bio_stat_group(bio), bio_sectors(bio), &d->disk->part0);
 
 	s = search_alloc(bio, d);
 	cl = &s->cl;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 3d3ac13..5804548 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -735,7 +735,7 @@  static void end_io_acct(struct dm_io *io)
 	int pending;
 	int rw = bio_data_dir(bio);
 
-	generic_end_io_acct(rw, &dm_disk(md)->part0, io->start_time);
+	generic_end_io_acct(bio_stat_group(bio), &dm_disk(md)->part0, io->start_time);
 
 	if (unlikely(dm_stats_used(&md->stats)))
 		dm_stats_account_io(&md->stats, bio->bi_rw, bio->bi_iter.bi_sector,
@@ -1820,14 +1820,13 @@  static void __split_and_process_bio(struct mapped_device *md,
  */
 static blk_qc_t dm_make_request(struct request_queue *q, struct bio *bio)
 {
-	int rw = bio_data_dir(bio);
 	struct mapped_device *md = q->queuedata;
 	int srcu_idx;
 	struct dm_table *map;
 
 	map = dm_get_live_table(md, &srcu_idx);
 
-	generic_start_io_acct(rw, bio_sectors(bio), &dm_disk(md)->part0);
+	generic_start_io_acct(bio_stat_group(bio), bio_sectors(bio), &dm_disk(md)->part0);
 
 	/* if we're suspended, we have to queue this io for later */
 	if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 14d3b37..a8bc226 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -245,6 +245,7 @@  static DEFINE_SPINLOCK(all_mddevs_lock);
 static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
 {
 	const int rw = bio_data_dir(bio);
+	const int rwd = bio_stat_group(bio);
 	struct mddev *mddev = q->queuedata;
 	unsigned int sectors;
 	int cpu;
@@ -289,8 +290,8 @@  static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
 	mddev->pers->make_request(mddev, bio);
 
 	cpu = part_stat_lock();
-	part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
-	part_stat_add(cpu, &mddev->gendisk->part0, sectors[rw], sectors);
+	part_stat_inc(cpu, &mddev->gendisk->part0, ios[rwd]);
+	part_stat_add(cpu, &mddev->gendisk->part0, sectors[rwd], sectors);
 	part_stat_unlock();
 
 	if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended)
@@ -7626,8 +7627,9 @@  static int is_mddev_idle(struct mddev *mddev, int init)
 	rcu_read_lock();
 	rdev_for_each_rcu(rdev, mddev) {
 		struct gendisk *disk = rdev->bdev->bd_contains->bd_disk;
-		curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
-			      (int)part_stat_read(&disk->part0, sectors[1]) -
+		curr_events = (int)part_stat_read(&disk->part0, sectors[STAT_READ]) +
+			      (int)part_stat_read(&disk->part0, sectors[STAT_WRITE]) +
+			      (int)part_stat_read(&disk->part0, sectors[STAT_DISCARD]) -
 			      atomic_read(&disk->sync_io);
 		/* sync IO will cause sync_io to increase before the disk_stats
 		 * as sync_io is counted when a request starts, and
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 182a93f..50c6956 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -218,13 +218,14 @@  ssize_t nd_sector_size_store(struct device *dev, const char *buf,
 void __nd_iostat_start(struct bio *bio, unsigned long *start)
 {
 	struct gendisk *disk = bio->bi_bdev->bd_disk;
+	const int rwd = bio_stat_group(bio);
 	const int rw = bio_data_dir(bio);
 	int cpu = part_stat_lock();
 
 	*start = jiffies;
 	part_round_stats(cpu, &disk->part0);
-	part_stat_inc(cpu, &disk->part0, ios[rw]);
-	part_stat_add(cpu, &disk->part0, sectors[rw], bio_sectors(bio));
+	part_stat_inc(cpu, &disk->part0, ios[rwd]);
+	part_stat_add(cpu, &disk->part0, sectors[rwd], bio_sectors(bio));
 	part_inc_in_flight(&disk->part0, rw);
 	part_stat_unlock();
 }
@@ -234,10 +235,11 @@  void nd_iostat_end(struct bio *bio, unsigned long start)
 {
 	struct gendisk *disk = bio->bi_bdev->bd_disk;
 	unsigned long duration = jiffies - start;
+	const int rwd = bio_stat_group(bio);
 	const int rw = bio_data_dir(bio);
 	int cpu = part_stat_lock();
 
-	part_stat_add(cpu, &disk->part0, ticks[rw], duration);
+	part_stat_add(cpu, &disk->part0, ticks[rwd], duration);
 	part_round_stats(cpu, &disk->part0);
 	part_dec_in_flight(&disk->part0, rw);
 	part_stat_unlock();
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 304c712..b5b549e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3182,7 +3182,7 @@  static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_sb_block = sb_block;
 	if (sb->s_bdev->bd_part)
 		sbi->s_sectors_written_start =
-			part_stat_read(sb->s_bdev->bd_part, sectors[1]);
+			part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]);
 
 	/* Cleanup superblock name */
 	strreplace(sb->s_id, '/', '!');
@@ -4359,7 +4359,7 @@  static int ext4_commit_super(struct super_block *sb, int sync)
 	if (sb->s_bdev->bd_part)
 		es->s_kbytes_written =
 			cpu_to_le64(EXT4_SB(sb)->s_kbytes_written +
-			    ((part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+			    ((part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]) -
 			      EXT4_SB(sb)->s_sectors_written_start) >> 1));
 	else
 		es->s_kbytes_written =
diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
index 1420a3c..ec65565 100644
--- a/fs/ext4/sysfs.c
+++ b/fs/ext4/sysfs.c
@@ -55,7 +55,7 @@  static ssize_t session_write_kbytes_show(struct ext4_attr *a,
 	if (!sb->s_bdev->bd_part)
 		return snprintf(buf, PAGE_SIZE, "0\n");
 	return snprintf(buf, PAGE_SIZE, "%lu\n",
-			(part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+			(part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]) -
 			 sbi->s_sectors_written_start) >> 1);
 }
 
@@ -68,7 +68,7 @@  static ssize_t lifetime_write_kbytes_show(struct ext4_attr *a,
 		return snprintf(buf, PAGE_SIZE, "0\n");
 	return snprintf(buf, PAGE_SIZE, "%llu\n",
 			(unsigned long long)(sbi->s_kbytes_written +
-			((part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+			((part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]) -
 			  EXT4_SB(sb)->s_sectors_written_start) >> 1)));
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7a4558d..b62015b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -808,7 +808,7 @@  struct f2fs_sb_info {
  * and the return value is in kbytes. s is of struct f2fs_sb_info.
  */
 #define BD_PART_WRITTEN(s)						 \
-(((u64)part_stat_read(s->sb->s_bdev->bd_part, sectors[1]) -		 \
+(((u64)part_stat_read(s->sb->s_bdev->bd_part, sectors[STAT_WRITE]) -		 \
 		s->sectors_written_start) >> 1)
 
 static inline void f2fs_update_time(struct f2fs_sb_info *sbi, int type)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 006f87d..0d4c45f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1464,7 +1464,7 @@  try_onemore:
 	/* For write statistics */
 	if (sb->s_bdev->bd_part)
 		sbi->sectors_written_start =
-			(u64)part_stat_read(sb->s_bdev->bd_part, sectors[1]);
+			(u64)part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]);
 
 	/* Read accumulated write IO statistics if exists */
 	seg_i = CURSEG_I(sbi, CURSEG_HOT_NODE);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 669e419..30de897 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -599,6 +599,16 @@  static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define rq_data_dir(rq)		((int)((rq)->cmd_flags & 1))
 
 /*
+ * Return the stat group for this request, STAT_READ, STAT_WRITE, or STAT_DISCARD
+ */
+static inline int rq_stat_group(struct request *rq)
+{
+	if (rq->cmd_flags & REQ_DISCARD)
+		return STAT_DISCARD;
+	return rq_data_dir(rq);
+}
+
+/*
  * Driver can handle struct request, if it either has an old style
  * request_fn defined, or is blk-mq based.
  */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 70e61b5..9a7c899 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2448,6 +2448,20 @@  extern bool is_bad_inode(struct inode *);
  */
 #define bio_data_dir(bio)	((bio)->bi_rw & 1)
 
+#define STAT_READ	0
+#define STAT_WRITE	1
+#define STAT_DISCARD	2
+
+/*
+ * return stat group, READ, WRITE, or 2 for DISCARD
+ */
+static inline int bio_stat_group(struct bio *bio)
+{
+	if (bio->bi_rw & REQ_DISCARD)
+		return STAT_DISCARD;
+	return bio_data_dir(bio);
+}
+
 extern void check_disk_size_change(struct gendisk *disk,
 				   struct block_device *bdev);
 extern int revalidate_disk(struct gendisk *);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 5c70676..c02eddb 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -80,10 +80,10 @@  struct partition {
 } __attribute__((packed));
 
 struct disk_stats {
-	unsigned long sectors[2];	/* READs and WRITEs */
-	unsigned long ios[2];
-	unsigned long merges[2];
-	unsigned long ticks[2];
+	unsigned long sectors[3];	/* STAT_READ / STAT_WRITE / STAT_DISCARD */
+	unsigned long ios[3];
+	unsigned long merges[3];
+	unsigned long ticks[3];
 	unsigned long io_ticks;
 	unsigned long time_in_queue;
 };