Re: [PATCH v5] blk: fix a wrong accounting of hd_struct->in_flight

Yasuaki Ishimatsu
2010-10-19T02:23:43Z

Hi Jens,

> This version looks good, thanks for following through on this. What kind
> of testing did you do?

I did three kinds of test.

1. run a reproducer.
I confirmed the problem was fixed using attached the reproducer.

2. remove and make a partition which is running I/O.
I confirmed the kernel panic or hungup did not occur using following steps.

# dd if=/dev/sda of=null &
# while true
> do
> /sbin/parted /dev/sda rm 3 > /dev/null
> /sbin/parted /dev/sda mkpart primary ext2 42.5GB 52.5GB > /dev/null
> sleep 2
> done

3. reload a partition which is running I/O.
I confirmed the kernel panic or hungup did not occur using following steps.

# dd if=/dev/sda of=null &
# while true
> do
> /sbin/hdparm -z /dev/sda
> sleep 2
> done

--- <reproducer>
/*
* This test program continues to read 512 bytes data from specified arguments.
*
* Usage : read <sec_no> <blk_dev>
* @sec_no : sector number
* @blk_dev : block device
*
* ex) If you want to read 512 bytes data from sector number 100 of
* /dev/sda
*
* # ./read 100 /dev/sda
*
* How to build:
*
* $ gcc -D _GNU_SOURCE -o read read.c
*
* How to occurs a problem which is a wrong accounting of
* hd_struct->in_flight:
*
* 1. confirm a start sector of a partition
* # cat /sys/block/sda/sda2/start
* 1044225
* 2. run the test program against sector no.1044225 of /dev/sda
* # ./read 1044225 /dev/sda &
* 3. run the test program against sector no.1044224 of /dev/sda
* # ./read 1044224 /dev/sda &
* 4. confirm the /proc/diskstats
* # cat /proc/diskstats |grep sda
* 8 0 sda 90524 7579 102154 20464 0 0 0 0 0 14096 20089
* 8 1 sda1 19085 1352 21841 4209 0 0 0 0 4294967064 15689 4293424691
* 8 2 sda2 71252 3624 74891 15950 0 0 0 0 232 23995 1562390
* 8 3 sda3 54 487 2188 92 0 0 0 0 0 88 92
* 8 4 sda4 4 0 8 0 0 0 0 0 0 0 0
* 8 5 sda5 81 2027 2130 138 0 0 0 0 0 87 137
*/

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>

#include <sys/types.h>
#include <sys/stat.h>

#define SECTOR 512 // sector size
#define ALIGN 512 // for direct io

int main(int argc, char *argv[])
{
int fd;
int i;
int sector_n;
unsigned long ret;
void *buf;

if ( argc != 3 ) {
printf("invalid argument\n");
exit(1);
}

sector_n = atoi(argv[1]);
printf("read 512 bytes data from setor no.%d\n", sector_n);

ret = posix_memalign(&buf, ALIGN, SECTOR);
if (ret != 0) {
perror("posix_memalign");
exit(1);
}

fd = open(argv[2], O_RDONLY|O_DIRECT);

if (fd < 0) {
perror("open");
exit(1);
}

while (1) {
ret = lseek(fd, 0, SEEK_SET);
if (ret < 0) {
perror("lseek");
printf("ret = %d\n", ret);
exit(1);
}
for (i = 0; i < SECTOR; i++) {
ret = lseek(fd, sector_n, SEEK_CUR);
if (ret < 0) {
perror("lseek");
printf("ret = %d\n", ret);
exit(1) ;
}
}

ret = read(fd, buf, SECTOR);
if (ret != SECTOR) {
perror("read");
exit(1);
}
}

free(buf);
close(fd);
}
---

Regards,
Yasuaki Ishimatsu