Revert "Seperate read and write statistics of in_flight requests"

This reverts commit a9327cac440be4d8333bba975cbbf76045096275. Corrado Zoccolo <czoccolo@gmail.com> reports: "with 2.6.32-rc1 I started getting the following strange output from "iostat -kx 2": Linux 2.6.31bisect (et2) 04/10/2009 _i686_ (2 CPU) avg-cpu: %user %nice %system %iowait %steal %idle 10,70 0,00 3,16 15,75 0,00 70,38 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 18,22 0,00 0,67 0,01 14,77 0,02 43,94 0,01 10,53 39043915,03 2629219,87 sdb 60,89 9,68 50,79 3,04 1724,43 50,52 65,95 0,70 13,06 488437,47 2629219,87 avg-cpu: %user %nice %system %iowait %steal %idle 2,72 0,00 0,74 0,00 0,00 96,53 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 avg-cpu: %user %nice %system %iowait %steal %idle 6,68 0,00 0,99 0,00 0,00 92,33 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 avg-cpu: %user %nice %system %iowait %steal %idle 4,40 0,00 0,73 1,47 0,00 93,40 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 4,00 0,00 3,00 0,00 28,00 18,67 0,06 19,50 333,33 100,00 Global values for service time and utilization are garbage. For interval values, utilization is always 100%, and service time is higher than normal. I bisected it down to: [a9327cac440be4d8333bba975cbbf76045096275] Seperate read and write statistics of in_flight requests and verified that reverting just that commit indeed solves the issue on 2.6.32-rc1." So until this is debugged, revert the bad commit. Signed-off-by: Jens Axboe <jens.axboe@oracle.com>

Revert "Seperate read and write statistics of in_flight requests"
This reverts commit a9327cac440be4d8333bba975cbbf76045096275. Corrado Zoccolo <czoccolo@gmail.com> reports: "with 2.6.32-rc1 I started getting the following strange output from "iostat -kx 2": Linux 2.6.31bisect (et2) 04/10/2009 _i686_ (2 CPU) avg-cpu: %user %nice %system %iowait %steal %idle 10,70 0,00 3,16 15,75 0,00 70,38 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 18,22 0,00 0,67 0,01 14,77 0,02 43,94 0,01 10,53 39043915,03 2629219,87 sdb 60,89 9,68 50,79 3,04 1724,43 50,52 65,95 0,70 13,06 488437,47 2629219,87 avg-cpu: %user %nice %system %iowait %steal %idle 2,72 0,00 0,74 0,00 0,00 96,53 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 avg-cpu: %user %nice %system %iowait %steal %idle 6,68 0,00 0,99 0,00 0,00 92,33 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 avg-cpu: %user %nice %system %iowait %steal %idle 4,40 0,00 0,73 1,47 0,00 93,40 Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm %util sda 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 0,00 100,00 sdb 0,00 4,00 0,00 3,00 0,00 28,00 18,67 0,06 19,50 333,33 100,00 Global values for service time and utilization are garbage. For interval values, utilization is always 100%, and service time is higher than normal. I bisected it down to: [a9327cac440be4d8333bba975cbbf76045096275] Seperate read and write statistics of in_flight requests and verified that reverting just that commit indeed solves the issue on 2.6.32-rc1." So until this is debugged, revert the bad commit. Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
Jens Axboe
1 parent e00c54c36a
Showing 6 changed files with 19 additions and 42 deletions Side-by-side Diff
block/blk-core.c
block/blk-merge.c
block/genhd.c
drivers/md/dm.c
fs/partitions/check.c
include/linux/genhd.h
@@ -70,7 +70,7 @@
 		part_stat_inc(cpu, part, merges[rw]);
 	else {
 		part_round_stats(cpu, part);
-		part_inc_in_flight(part, rw);
+		part_inc_in_flight(part);
 	}
  
 	part_stat_unlock();
@@ -1032,7 +1032,7 @@
  
 	if (part->in_flight) {
 		__part_stat_add(cpu, part, time_in_queue,
-				part_in_flight(part) * (now - part->stamp));
+				part->in_flight * (now - part->stamp));
 		__part_stat_add(cpu, part, io_ticks, (now - part->stamp));
 	}
 	part->stamp = now;
@@ -1739,7 +1739,7 @@
 		part_stat_inc(cpu, part, ios[rw]);
 		part_stat_add(cpu, part, ticks[rw], duration);
 		part_round_stats(cpu, part);
-		part_dec_in_flight(part, rw);
+		part_dec_in_flight(part);
  
 		part_stat_unlock();
 	}
@@ -351,7 +351,7 @@
 		part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req));
  
 		part_round_stats(cpu, part);
-		part_dec_in_flight(part, rq_data_dir(req));
+		part_dec_in_flight(part);
  
 		part_stat_unlock();
 	}
@@ -869,7 +869,6 @@
 static DEVICE_ATTR(alignment_offset, S_IRUGO, disk_alignment_offset_show, NULL);
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
-static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 static struct device_attribute dev_attr_fail =
 	__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -889,7 +888,6 @@
 	&dev_attr_alignment_offset.attr,
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
-	&dev_attr_inflight.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&dev_attr_fail.attr,
 #endif
@@ -1055,7 +1053,7 @@
 			   part_stat_read(hd, merges[1]),
 			   (unsigned long long)part_stat_read(hd, sectors[1]),
 			   jiffies_to_msecs(part_stat_read(hd, ticks[1])),
-			   part_in_flight(hd),
+			   hd->in_flight,
 			   jiffies_to_msecs(part_stat_read(hd, io_ticks)),
 			   jiffies_to_msecs(part_stat_read(hd, time_in_queue))
 			);
@@ -130,7 +130,7 @@
 	/*
 	 * A list of ios that arrived while we were suspended.
 	 */
-	atomic_t pending[2];
+	atomic_t pending;
 	wait_queue_head_t wait;
 	struct work_struct work;
 	struct bio_list deferred;
  
@@ -453,14 +453,13 @@
 {
 	struct mapped_device *md = io->md;
 	int cpu;
-	int rw = bio_data_dir(io->bio);
  
 	io->start_time = jiffies;
  
 	cpu = part_stat_lock();
 	part_round_stats(cpu, &dm_disk(md)->part0);
 	part_stat_unlock();
-	dm_disk(md)->part0.in_flight[rw] = atomic_inc_return(&md->pending[rw]);
+	dm_disk(md)->part0.in_flight = atomic_inc_return(&md->pending);
 }
  
 static void end_io_acct(struct dm_io *io)
@@ -480,9 +479,8 @@
 	 * After this is decremented the bio must not be touched if it is
 	 * a barrier.
 	 */
-	dm_disk(md)->part0.in_flight[rw] = pending =
-		atomic_dec_return(&md->pending[rw]);
-	pending += atomic_read(&md->pending[rw^0x1]);
+	dm_disk(md)->part0.in_flight = pending =
+		atomic_dec_return(&md->pending);
  
 	/* nudge anyone waiting on suspend queue */
 	if (!pending)
@@ -1787,8 +1785,7 @@
 	if (!md->disk)
 		goto bad_disk;
  
-	atomic_set(&md->pending[0], 0);
-	atomic_set(&md->pending[1], 0);
+	atomic_set(&md->pending, 0);
 	init_waitqueue_head(&md->wait);
 	INIT_WORK(&md->work, dm_wq_work);
 	init_waitqueue_head(&md->eventq);
@@ -2091,8 +2088,7 @@
 				break;
 			}
 			spin_unlock_irqrestore(q->queue_lock, flags);
-		} else if (!atomic_read(&md->pending[0]) &&
-					!atomic_read(&md->pending[1]))
+		} else if (!atomic_read(&md->pending))
 			break;
  
 		if (interruptible == TASK_INTERRUPTIBLE &&
@@ -248,19 +248,11 @@
 		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_in_flight(p),
+		p->in_flight,
 		jiffies_to_msecs(part_stat_read(p, io_ticks)),
 		jiffies_to_msecs(part_stat_read(p, time_in_queue)));
 }
  
-ssize_t part_inflight_show(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	struct hd_struct *p = dev_to_part(dev);
-
-	return sprintf(buf, "%8u %8u\n", p->in_flight[0], p->in_flight[1]);
-}
-
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 ssize_t part_fail_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
@@ -289,7 +281,6 @@
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
 static DEVICE_ATTR(alignment_offset, S_IRUGO, part_alignment_offset_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
-static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 static struct device_attribute dev_attr_fail =
 	__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -301,7 +292,6 @@
 	&dev_attr_size.attr,
 	&dev_attr_alignment_offset.attr,
 	&dev_attr_stat.attr,
-	&dev_attr_inflight.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&dev_attr_fail.attr,
 #endif
@@ -98,7 +98,7 @@
 	int make_it_fail;
 #endif
 	unsigned long stamp;
-	int in_flight[2];
+	int in_flight;
 #ifdef	CONFIG_SMP
 	struct disk_stats *dkstats;
 #else
  
  
  
  
  
  
@@ -322,25 +322,20 @@
 #define part_stat_sub(cpu, gendiskp, field, subnd)			\
 	part_stat_add(cpu, gendiskp, field, -subnd)
  
-static inline void part_inc_in_flight(struct hd_struct *part, int rw)
+static inline void part_inc_in_flight(struct hd_struct *part)
 {
-	part->in_flight[rw]++;
+	part->in_flight++;
 	if (part->partno)
-		part_to_disk(part)->part0.in_flight[rw]++;
+		part_to_disk(part)->part0.in_flight++;
 }
  
-static inline void part_dec_in_flight(struct hd_struct *part, int rw)
+static inline void part_dec_in_flight(struct hd_struct *part)
 {
-	part->in_flight[rw]--;
+	part->in_flight--;
 	if (part->partno)
-		part_to_disk(part)->part0.in_flight[rw]--;
+		part_to_disk(part)->part0.in_flight--;
 }
  
-static inline int part_in_flight(struct hd_struct *part)
-{
-	return part->in_flight[0] + part->in_flight[1];
-}
-
 /* block/blk-core.c */
 extern void part_round_stats(int cpu, struct hd_struct *part);
  
@@ -550,8 +545,6 @@
 extern ssize_t part_size_show(struct device *dev,
 			      struct device_attribute *attr, char *buf);
 extern ssize_t part_stat_show(struct device *dev,
-			      struct device_attribute *attr, char *buf);
-extern ssize_t part_inflight_show(struct device *dev,
 			      struct device_attribute *attr, char *buf);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 extern ssize_t part_fail_show(struct device *dev,
...	...	@@ -70,7 +70,7 @@
70	70	part_stat_inc(cpu, part, merges[rw]);
71	71	else {
72	72	part_round_stats(cpu, part);
73		- part_inc_in_flight(part, rw);
	73	+ part_inc_in_flight(part);
74	74	}
75	75
76	76	part_stat_unlock();
...	...	@@ -1032,7 +1032,7 @@
1032	1032
1033	1033	if (part->in_flight) {
1034	1034	__part_stat_add(cpu, part, time_in_queue,
1035		- part_in_flight(part) * (now - part->stamp));
	1035	+ part->in_flight * (now - part->stamp));
1036	1036	__part_stat_add(cpu, part, io_ticks, (now - part->stamp));
1037	1037	}
1038	1038	part->stamp = now;
...	...	@@ -1739,7 +1739,7 @@
1739	1739	part_stat_inc(cpu, part, ios[rw]);
1740	1740	part_stat_add(cpu, part, ticks[rw], duration);
1741	1741	part_round_stats(cpu, part);
1742		- part_dec_in_flight(part, rw);
	1742	+ part_dec_in_flight(part);
1743	1743
1744	1744	part_stat_unlock();
1745	1745	}
...	...	@@ -351,7 +351,7 @@
351	351	part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req));
352	352
353	353	part_round_stats(cpu, part);
354		- part_dec_in_flight(part, rq_data_dir(req));
	354	+ part_dec_in_flight(part);
355	355
356	356	part_stat_unlock();
357	357	}
...	...	@@ -869,7 +869,6 @@
869	869	static DEVICE_ATTR(alignment_offset, S_IRUGO, disk_alignment_offset_show, NULL);
870	870	static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
871	871	static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
872		-static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
873	872	#ifdef CONFIG_FAIL_MAKE_REQUEST
874	873	static struct device_attribute dev_attr_fail =
875	874	__ATTR(make-it-fail, S_IRUGO\|S_IWUSR, part_fail_show, part_fail_store);
...	...	@@ -889,7 +888,6 @@
889	888	&dev_attr_alignment_offset.attr,
890	889	&dev_attr_capability.attr,
891	890	&dev_attr_stat.attr,
892		- &dev_attr_inflight.attr,
893	891	#ifdef CONFIG_FAIL_MAKE_REQUEST
894	892	&dev_attr_fail.attr,
895	893	#endif
...	...	@@ -1055,7 +1053,7 @@
1055	1053	part_stat_read(hd, merges[1]),
1056	1054	(unsigned long long)part_stat_read(hd, sectors[1]),
1057	1055	jiffies_to_msecs(part_stat_read(hd, ticks[1])),
1058		- part_in_flight(hd),
	1056	+ hd->in_flight,
1059	1057	jiffies_to_msecs(part_stat_read(hd, io_ticks)),
1060	1058	jiffies_to_msecs(part_stat_read(hd, time_in_queue))
1061	1059	);
...	...	@@ -130,7 +130,7 @@
130	130	/*
131	131	* A list of ios that arrived while we were suspended.
132	132	*/
133		- atomic_t pending[2];
	133	+ atomic_t pending;
134	134	wait_queue_head_t wait;
135	135	struct work_struct work;
136	136	struct bio_list deferred;
137	137
...	...	@@ -453,14 +453,13 @@
453	453	{
454	454	struct mapped_device *md = io->md;
455	455	int cpu;
456		- int rw = bio_data_dir(io->bio);
457	456
458	457	io->start_time = jiffies;
459	458
460	459	cpu = part_stat_lock();
461	460	part_round_stats(cpu, &dm_disk(md)->part0);
462	461	part_stat_unlock();
463		- dm_disk(md)->part0.in_flight[rw] = atomic_inc_return(&md->pending[rw]);
	462	+ dm_disk(md)->part0.in_flight = atomic_inc_return(&md->pending);
464	463	}
465	464
466	465	static void end_io_acct(struct dm_io *io)
...	...	@@ -480,9 +479,8 @@
480	479	* After this is decremented the bio must not be touched if it is
481	480	* a barrier.
482	481	*/
483		- dm_disk(md)->part0.in_flight[rw] = pending =
484		- atomic_dec_return(&md->pending[rw]);
485		- pending += atomic_read(&md->pending[rw^0x1]);
	482	+ dm_disk(md)->part0.in_flight = pending =
	483	+ atomic_dec_return(&md->pending);
486	484
487	485	/* nudge anyone waiting on suspend queue */
488	486	if (!pending)
...	...	@@ -1787,8 +1785,7 @@
1787	1785	if (!md->disk)
1788	1786	goto bad_disk;
1789	1787
1790		- atomic_set(&md->pending[0], 0);
1791		- atomic_set(&md->pending[1], 0);
	1788	+ atomic_set(&md->pending, 0);
1792	1789	init_waitqueue_head(&md->wait);
1793	1790	INIT_WORK(&md->work, dm_wq_work);
1794	1791	init_waitqueue_head(&md->eventq);
...	...	@@ -2091,8 +2088,7 @@
2091	2088	break;
2092	2089	}
2093	2090	spin_unlock_irqrestore(q->queue_lock, flags);
2094		- } else if (!atomic_read(&md->pending[0]) &&
2095		- !atomic_read(&md->pending[1]))
	2091	+ } else if (!atomic_read(&md->pending))
2096	2092	break;
2097	2093
2098	2094	if (interruptible == TASK_INTERRUPTIBLE &&
...	...	@@ -248,19 +248,11 @@
248	248	part_stat_read(p, merges[WRITE]),
249	249	(unsigned long long)part_stat_read(p, sectors[WRITE]),
250	250	jiffies_to_msecs(part_stat_read(p, ticks[WRITE])),
251		- part_in_flight(p),
	251	+ p->in_flight,
252	252	jiffies_to_msecs(part_stat_read(p, io_ticks)),
253	253	jiffies_to_msecs(part_stat_read(p, time_in_queue)));
254	254	}
255	255
256		-ssize_t part_inflight_show(struct device *dev,
257		- struct device_attribute attr, char buf)
258		-{
259		- struct hd_struct *p = dev_to_part(dev);
260		-
261		- return sprintf(buf, "%8u %8u\n", p->in_flight[0], p->in_flight[1]);
262		-}
263		-
264	256	#ifdef CONFIG_FAIL_MAKE_REQUEST
265	257	ssize_t part_fail_show(struct device *dev,
266	258	struct device_attribute attr, char buf)
...	...	@@ -289,7 +281,6 @@
289	281	static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
290	282	static DEVICE_ATTR(alignment_offset, S_IRUGO, part_alignment_offset_show, NULL);
291	283	static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
292		-static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
293	284	#ifdef CONFIG_FAIL_MAKE_REQUEST
294	285	static struct device_attribute dev_attr_fail =
295	286	__ATTR(make-it-fail, S_IRUGO\|S_IWUSR, part_fail_show, part_fail_store);
...	...	@@ -301,7 +292,6 @@
301	292	&dev_attr_size.attr,
302	293	&dev_attr_alignment_offset.attr,
303	294	&dev_attr_stat.attr,
304		- &dev_attr_inflight.attr,
305	295	#ifdef CONFIG_FAIL_MAKE_REQUEST
306	296	&dev_attr_fail.attr,
307	297	#endif
...	...	@@ -98,7 +98,7 @@
98	98	int make_it_fail;
99	99	#endif
100	100	unsigned long stamp;
101		- int in_flight[2];
	101	+ int in_flight;
102	102	#ifdef CONFIG_SMP
103	103	struct disk_stats *dkstats;
104	104	#else
105	105
106	106
107	107
108	108
109	109
110	110
...	...	@@ -322,25 +322,20 @@
322	322	#define part_stat_sub(cpu, gendiskp, field, subnd) \
323	323	part_stat_add(cpu, gendiskp, field, -subnd)
324	324
325		-static inline void part_inc_in_flight(struct hd_struct *part, int rw)
	325	+static inline void part_inc_in_flight(struct hd_struct *part)
326	326	{
327		- part->in_flight[rw]++;
	327	+ part->in_flight++;
328	328	if (part->partno)
329		- part_to_disk(part)->part0.in_flight[rw]++;
	329	+ part_to_disk(part)->part0.in_flight++;
330	330	}
331	331
332		-static inline void part_dec_in_flight(struct hd_struct *part, int rw)
	332	+static inline void part_dec_in_flight(struct hd_struct *part)
333	333	{
334		- part->in_flight[rw]--;
	334	+ part->in_flight--;
335	335	if (part->partno)
336		- part_to_disk(part)->part0.in_flight[rw]--;
	336	+ part_to_disk(part)->part0.in_flight--;
337	337	}
338	338
339		-static inline int part_in_flight(struct hd_struct *part)
340		-{
341		- return part->in_flight[0] + part->in_flight[1];
342		-}
343		-
344	339	/* block/blk-core.c */
345	340	extern void part_round_stats(int cpu, struct hd_struct *part);
346	341
...	...	@@ -550,8 +545,6 @@
550	545	extern ssize_t part_size_show(struct device *dev,
551	546	struct device_attribute attr, char buf);
552	547	extern ssize_t part_stat_show(struct device *dev,
553		- struct device_attribute attr, char buf);
554		-extern ssize_t part_inflight_show(struct device *dev,
555	548	struct device_attribute attr, char buf);
556	549	#ifdef CONFIG_FAIL_MAKE_REQUEST
557	550	extern ssize_t part_fail_show(struct device *dev,