All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikanth Karthikesan <knikanth@suse.de>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: linux-kernel@vger.kernel.org, Tejun Heo <tj@kernel.org>
Subject: [PATCH] [RFC] make hd_struct->in_flight atomic to avoid diskstat corruption
Date: Thu, 16 Apr 2009 12:54:40 +0530	[thread overview]
Message-ID: <200904161254.41433.knikanth@suse.de> (raw)

The disk statistics exported to userspace through proc and sysfs are not
protected by locks to avoid performance overhead. Since most of the statistics
are maintained in the per_cpu struct disk_stats, the chances of them getting
corrupted is negligible. But the in_flight counter, that records the no of
requests currently in progress is not per-cpu. This increases the chance of it
getting corrupted. And corruption of this value would result in visibly
distorted statistics such as negative in_flight. This can be avoided by making
this field atomic.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

diff --git a/block/blk-core.c b/block/blk-core.c
index 07ab754..c295deb 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1013,12 +1013,15 @@ static inline void add_request(struct request_queue *q, struct request *req)
 static void part_round_stats_single(int cpu, struct hd_struct *part,
 				    unsigned long now)
 {
+	int in_flight;
+
 	if (now == part->stamp)
 		return;
 
-	if (part->in_flight) {
+	in_flight = atomic_read(&part->in_flight);
+	if (in_flight) {
 		__part_stat_add(cpu, part, time_in_queue,
-				part->in_flight * (now - part->stamp));
+				in_flight * (now - part->stamp));
 		__part_stat_add(cpu, part, io_ticks, (now - part->stamp));
 	}
 	part->stamp = now;
diff --git a/block/genhd.c b/block/genhd.c
index a9ec910..9436991 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1028,7 +1028,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 			   part_stat_read(hd, merges[1]),
 			   (unsigned long long)part_stat_read(hd, sectors[1]),
 			   jiffies_to_msecs(part_stat_read(hd, ticks[1])),
-			   hd->in_flight,
+			   atomic_read(&hd->in_flight),
 			   jiffies_to_msecs(part_stat_read(hd, io_ticks)),
 			   jiffies_to_msecs(part_stat_read(hd, time_in_queue))
 			);
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 99e33ef..ae1f55a 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -241,7 +241,7 @@ ssize_t part_stat_show(struct device *dev,
 		part_stat_read(p, merges[WRITE]),
 		(unsigned long long)part_stat_read(p, sectors[WRITE]),
 		jiffies_to_msecs(part_stat_read(p, ticks[WRITE])),
-		p->in_flight,
+		atomic_read(&p->in_flight),
 		jiffies_to_msecs(part_stat_read(p, io_ticks)),
 		jiffies_to_msecs(part_stat_read(p, time_in_queue)));
 }
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 634c530..5921400 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -97,7 +97,7 @@ struct hd_struct {
 	int make_it_fail;
 #endif
 	unsigned long stamp;
-	int in_flight;
+	atomic_t in_flight;
 #ifdef	CONFIG_SMP
 	struct disk_stats *dkstats;
 #else
@@ -321,16 +321,16 @@ static inline void free_part_stats(struct hd_struct *part)
 
 static inline void part_inc_in_flight(struct hd_struct *part)
 {
-	part->in_flight++;
+	atomic_inc(&part->in_flight);
 	if (part->partno)
-		part_to_disk(part)->part0.in_flight++;
+		atomic_inc(&part_to_disk(part)->part0.in_flight);
 }
 
 static inline void part_dec_in_flight(struct hd_struct *part)
 {
-	part->in_flight--;
+	atomic_dec(&part->in_flight);
 	if (part->partno)
-		part_to_disk(part)->part0.in_flight--;
+		atomic_dec(&part_to_disk(part)->part0.in_flight);
 }
 
 /* block/blk-core.c */


             reply	other threads:[~2009-04-16  7:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-16  7:24 Nikanth Karthikesan [this message]
2009-04-16  7:35 ` [PATCH] [RFC] make hd_struct->in_flight atomic to avoid diskstat corruption Jens Axboe
2009-04-16  9:15   ` Nikanth Karthikesan
2009-04-16 14:40     ` Tejun Heo
2009-04-16 16:32       ` Jens Axboe
2009-04-19  8:51         ` Tejun Heo
2009-04-21  7:31           ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200904161254.41433.knikanth@suse.de \
    --to=knikanth@suse.de \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.