block layer sg, bsg

Hi,

Here's a little project I've been tinkering with in my copius spare time. Since it's been mentioned a few times (as bsg), I thought I'd at least try and post something. Lately Peter Jones has been contributing changes as well.

This is a block layer implementation of the SG (SCSI generic) character device, supporting only the sg v3 interface (this is on purpose, older ones are not worth supporting since they suck). It allows you to talk to a device synchronously through the SG_IO ioctl and async through regular read(2) and write(2). Additionally, there's a readv/writev interface where you pass the seperate elements of the sg_io_hdr as iovec entries. This is mainly handy for debugging purposes (you can watch what happens with strace closely), and I'm not quite sure this will even stay in the future.

There's just one character device, /dev/bsg. You open this control device and issue a BSG_IOC_ADD ioctl to bind it to any given device, and then talk to it through that file descriptor. I've attached a silly test app that just reads randomly from a device as a show case.

Patch is against 2.6-BK as of this morning.

===== drivers/block/Kconfig 1.25 vs edited =====

@@ -301,6 +301,12 @@
 
 	  Use devices /dev/sx8/$N and /dev/sx8/$Np$M.
 
+config BLK_DEV_BSG
+	tristate "Block layer SG support"
+	---help---
+	  Saying Y or M here will enable generic SG (SCSI generic) v3
+	  support for any block device.
+
 config BLK_DEV_RAM
 	tristate "RAM disk support"
 	---help---

===== drivers/block/Makefile 1.26 vs edited =====

@@ -42,4 +42,5 @@
 
 obj-$(CONFIG_VIODASD)		+= viodasd.o
 obj-$(CONFIG_BLK_DEV_SX8)	+= sx8.o
+obj-$(CONFIG_BLK_DEV_BSG)	+= bsg.o
 

===== drivers/block/ll_rw_blk.c 1.261 vs edited =====

===== drivers/block/scsi_ioctl.c 1.48 vs edited =====

===== drivers/block/paride/pd.c 1.76 vs edited =====

@@ -743,6 +743,7 @@
 	rq.rq_disk = disk->gd;
 	rq.ref_count = 1;
 	rq.waiting = &wait;
+	rq.end_io = blk_end_sync_rq;
 	blk_insert_request(disk->gd->queue, &rq, 0, func, 0);
 	wait_for_completion(&wait);
 	rq.waiting = NULL;

===== drivers/ide/ide-floppy.c 1.39 vs edited =====

@@ -1210,19 +1210,25 @@
 	set_bit(PC_DMA_RECOMMENDED, &pc->flags);
 }
 
-static int
+static void
 idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy, idefloppy_pc_t *pc, struct request *rq)
 {
-	/*
-	 * just support eject for now, it would not be hard to make the
-	 * REQ_BLOCK_PC support fully-featured
-	 */
-	if (rq->cmd[0] != IDEFLOPPY_START_STOP_CMD)
-		return 1;
-
 	idefloppy_init_pc(pc);
+	pc->callback = &idefloppy_rw_callback;
 	memcpy(pc->c, rq->cmd, sizeof(pc->c));
-	return 0;
+	pc->rq = rq;
+	pc->b_count = rq->data_len;
+	if (rq->data_len && rq_data_dir(rq) == WRITE)
+		set_bit(PC_WRITING, &pc->flags);
+	pc->buffer = rq->data;
+	if (rq->bio)
+		set_bit(PC_DMA_RECOMMENDED, &pc->flags);
+		
+	/*
+	 * possibly problematic, doesn't look like ide-floppy correctly
+	 * handled scattered requests if dma fails...
+	 */
+	pc->request_transfer = pc->buffer_size = rq->data_len;
 }
 
 /*

@@ -1267,10 +1273,7 @@
 		pc = (idefloppy_pc_t *) rq->buffer;
 	} else if (rq->flags & REQ_BLOCK_PC) {
 		pc = idefloppy_next_pc_storage(drive);
-		if (idefloppy_blockpc_cmd(floppy, pc, rq)) {
-			idefloppy_do_end_request(drive, 0, 0);
-			return ide_stopped;
-		}
+		idefloppy_blockpc_cmd(floppy, pc, rq);
 	} else {
 		blk_dump_rq_flags(rq,
 			"ide-floppy: unsupported command in queue");

===== drivers/ide/ide-io.c 1.27 vs edited =====

@@ -1353,6 +1353,7 @@
 	if (must_wait) {
 		rq->ref_count++;
 		rq->waiting = &wait;
+		rq->end_io = blk_end_sync_rq;
 	}
 
 	spin_lock_irqsave(&ide_lock, flags);

===== drivers/ide/ide-tape.c 1.41 vs edited =====

@@ -2798,6 +2798,7 @@
 	}
 #endif /* IDETAPE_DEBUG_BUGS */
 	rq->waiting = &wait;
+	rq->end_io = blk_end_sync_rq;
 	spin_unlock_irq(&tape->spinlock);
 	wait_for_completion(&wait);
 	/* The stage and its struct request have been deallocated */

===== drivers/ide/ide.c 1.151 vs edited =====

@@ -1458,9 +1458,14 @@
 {
 	ide_drive_t *drive = bdev->bd_disk->private_data;
 	ide_settings_t *setting;
-	int err = 0;
+	int err;
 	void __user *p = (void __user *)arg;
 
+	err = scsi_cmd_ioctl(bdev->bd_disk, cmd, p);
+	if (err != -ENOTTY)
+		return err;
+
+	err = 0;
 	down(&ide_setting_sem);
 	if ((setting = ide_find_setting_by_ioctl(drive, cmd)) != NULL) {
 		if (cmd == setting->read_ioctl) {

@@ -1602,10 +1607,6 @@
 			}
 			return 0;
 		}
-
-		case CDROMEJECT:
-		case CDROMCLOSETRAY:
-			return scsi_cmd_ioctl(bdev->bd_disk, cmd, p);
 
 		case HDIO_GET_BUSSTATE:
 			if (!capable(CAP_SYS_ADMIN))

===== drivers/scsi/scsi_lib.c 1.129 vs edited =====

@@ -711,8 +711,7 @@
 	}
 
 	if (blk_pc_request(req)) { /* SG_IO ioctl from block level */
-		req->errors = (driver_byte(result) & DRIVER_SENSE) ?
-			      (CHECK_CONDITION << 1) : (result & 0xff);
+		req->errors = result;
 		if (result) {
 			clear_errors = 0;
 			if (cmd->sense_buffer[0] & 0x70) {

===== fs/bio.c 1.63 vs edited =====

@@ -644,6 +644,7 @@
  */
 void bio_unmap_user(struct bio *bio)
 {
+	BUG_ON(!bio_flagged(bio, BIO_USER_MAPPED));
 	__bio_unmap_user(bio);
 	bio_put(bio);
 }

===== include/linux/blkdev.h 1.149 vs edited =====

@@ -22,6 +22,8 @@
 struct elevator_s;
 typedef struct elevator_s elevator_t;
 struct request_pm_state;
+struct request;
+struct sg_io_hdr;
 
 #define BLKDEV_MIN_RQ	4
 #define BLKDEV_MAX_RQ	128	/* Default maximum */

@@ -76,6 +78,8 @@
 void copy_io_context(struct io_context **pdst, struct io_context **psrc);
 void swap_io_context(struct io_context **ioc1, struct io_context **ioc2);
 
+typedef void (rq_end_io_fn)(struct request *);
+
 struct request_list {
 	int count[2];
 	mempool_t *rq_pool;

@@ -163,6 +167,13 @@
 	 * For Power Management requests
 	 */
 	struct request_pm_state *pm;
+
+	rq_end_io_fn *end_io;
+
+	/*
+	 * issuer private data
+	 */
+	void *private;
 };
 
 /*

@@ -479,6 +490,11 @@
 #define BLK_BOUNCE_ANY		((u64)blk_max_pfn << PAGE_SHIFT)
 #define BLK_BOUNCE_ISA		(ISA_DMA_THRESHOLD)
 
+/*
+ * default timeout for SG_IO if none specified
+ */
+#define BLK_DEFAULT_SG_TIMEOUT	(60 * HZ)
+
 #ifdef CONFIG_MMU
 extern int init_emergency_isa_pool(void);
 extern void blk_queue_bounce(request_queue_t *q, struct bio **bio);

@@ -506,10 +522,11 @@
 extern void register_disk(struct gendisk *dev);
 extern void generic_make_request(struct bio *bio);
 extern void blk_put_request(struct request *);
+extern void __blk_put_request(request_queue_t *, struct request *);
+extern void blk_end_sync_rq(struct request *rq);
 extern void blk_attempt_remerge(request_queue_t *, struct request *);
 extern void __blk_attempt_remerge(request_queue_t *, struct request *);
 extern struct request *blk_get_request(request_queue_t *, int, int);
-extern void blk_put_request(struct request *);
 extern void blk_insert_request(request_queue_t *, struct request *, int, void *, int);
 extern void blk_requeue_request(request_queue_t *, struct request *);
 extern void blk_plug_device(request_queue_t *);

@@ -525,6 +542,9 @@
 extern void blk_queue_activity_fn(request_queue_t *, activity_fn *, void *);
 extern struct request *blk_rq_map_user(request_queue_t *, int, void __user *, unsigned int);
 extern int blk_rq_unmap_user(struct request *, struct bio *, unsigned int);
+extern int blk_fill_sghdr_rq(request_queue_t *, struct request *, struct sg_io_hdr *);
+extern int blk_unmap_sghdr_rq(struct request *, struct sg_io_hdr *);
+extern int blk_complete_sghdr_rq(struct request *, struct sg_io_hdr *, struct bio *);
 extern int blk_execute_rq(request_queue_t *, struct gendisk *, struct request *);
 
 static inline request_queue_t *bdev_get_queue(struct block_device *bdev)

===== include/scsi/sg.h 1.15 vs edited =====

@@ -193,6 +193,8 @@
 #define SG_SET_RESERVED_SIZE 0x2275  /* request a new reserved buffer size */
 #define SG_GET_RESERVED_SIZE 0x2272  /* actual size of reserved buffer */
 
+#define SG_GET_MAX_COMMAND_SIZE 0x2277 /* how many bytes is q->max_sectors */
+
 /* The following ioctl has a 'sg_scsi_id_t *' object as its 3rd argument. */
 #define SG_GET_SCSI_ID 0x2276   /* Yields fd's bus, chan, dev, lun + type */
 /* SCSI id information can also be obtained from SCSI_IOCTL_GET_IDLUN */

@@ -0,0 +1,7 @@
+#ifndef BSG_H
+#define BSG_H
+
+#define BSG_IOC_MAGIC		0x89
+#define BSG_IOC_ADD		0x8900
+
+#endif

Jens Axboe

#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <getopt.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <sys/time.h>
#include <sys/stat.h>
#include <scsi/sg.h>

#ifndef BSG_IOC_ADD
#define BSG_IOC_ADD 0x8900
#define BSG_IOC_DEL 0x8901
#endif

#define NR_QUEUE (64)
#define NR_VECS (4)

#if 1
#define SECTORS (768)
#define BS (512)
#define ACCOUNT (100)
#else
#define SECTORS (32)
#define BS (2048)
#define ACCOUNT (1)
#endif

#define ALIGN(buf) (char *) (((unsigned long) (buf) + 4095) & ~(4095))

struct sg_io_hdr *hdr;
char *cdb;
char *buffer[NR_QUEUE];
#if NR_VECS > 1
static struct sg_iovec vecs[NR_QUEUE][NR_VECS];
#endif
char *sense;
static int nr_queue = NR_QUEUE;

static unsigned long ms;
static unsigned long kb;

typedef unsigned char u8;
#define __LITTLE_ENDIAN_BITFIELD

struct request_sense {

#if defined(__BIG_ENDIAN_BITFIELD)
	u8 valid		: 1;
	u8 error_code		: 7;
#elif defined(__LITTLE_ENDIAN_BITFIELD)
	u8 error_code		: 7;
	u8 valid		: 1;
#endif
	u8 segment_number;
#if defined(__BIG_ENDIAN_BITFIELD)
	u8 reserved1		: 2;
	u8 ili			: 1;
	u8 reserved2		: 1;
	u8 sense_key		: 4;
#elif defined(__LITTLE_ENDIAN_BITFIELD)
	u8 sense_key		: 4;
	u8 reserved2		: 1;
	u8 ili			: 1;
	u8 reserved1		: 2;
#endif
	u8 information[4];
	u8 add_sense_len;
	u8 command_info[4];
	u8 asc;
	u8 ascq;
	u8 fruc;
	u8 sks[3];
	u8 asb[46];
};

int queue_cmds(int fd, struct sg_io_hdr *hdr, int nr)

{
	return write(fd, hdr, sizeof(struct sg_io_hdr) * nr);
}

inline void account_hdr(struct sg_io_hdr *hdr)

{
	kb += hdr->dxfer_len / 1024;
	ms += hdr->duration;
	if (kb > ACCOUNT*1024) {
		printf("data rate %luKiB/sec\n", kb * 1000 / ms);
		ms = kb = 0;
	}
}

void sense_decode(struct sg_io_hdr *h)

{
	struct request_sense *s = (struct request_sense *) h->sbp;
	int i;

	printf("cdb: ");
	for (i = 0; i < h->cmd_len; i++)
		printf("%02x ", h->cmdp[i]);
	printf("\n");
	printf("0x%02x/0x%02x/0x%02x\n", s->sense_key, s->asc, s->ascq);
}

int wait_cmd(int fd, struct sg_io_hdr *hdr, int nr)

{
	int ret = read(fd, hdr, sizeof(struct sg_io_hdr) * nr);
	int i;

	if (ret == -1)
		return ret;

	for (i = 0; i < nr; i++) {
		struct sg_io_hdr *h = hdr + i;

		//printf("%d completed\n", h->pack_id);
		if (h->status == 0x02)
			sense_decode(h);
		account_hdr(h);
	}

	return ret;
}

void do_read10(int fd, struct sg_io_hdr *hdr, unsigned long lba)

{
	hdr->interface_id = 'S';
	hdr->cmd_len = 10;
	hdr->dxfer_direction = SG_DXFER_FROM_DEV;
	hdr->dxfer_len = SECTORS * BS;

	hdr->cmdp[0] = 0x28;
	hdr->cmdp[2] = (lba >> 24) & 0xff;
	hdr->cmdp[3] = (lba >> 16) & 0xff;
	hdr->cmdp[4] = (lba >>  8) & 0xff;
	hdr->cmdp[5] = (lba      ) & 0xff;
	hdr->cmdp[7] = (SECTORS >> 8) & 0xff;
	hdr->cmdp[8] = (SECTORS     ) & 0xff;
}

int bind_device(const char *device)

{
	int fd = open(device, O_RDONLY | O_NONBLOCK);
	int bsg_fd;

	if (fd == -1) {
		perror("open input");
		return -1;
	}

	bsg_fd = open("/dev/bsg", O_RDWR);
	if (bsg_fd == -1) {
		perror("open bsg");
		return -1;
	}

	if (ioctl(bsg_fd, BSG_IOC_ADD, fd) == -1) {
		perror("BSG_IOC_ADD");
		return -1;
	}

	return bsg_fd;
}

int main(int argc, const char *argv[])

{
	unsigned long lba = 0, read_kb;
	int fd, i, ret;

	if (argc < 2) {
		printf("%s: <device>\n", argv[0]);
		return 1;
	}

	fd = bind_device(argv[1]);
	if (fd == -1)
		return 1;

	cdb = malloc(10 * NR_QUEUE);
	memset(cdb, 0, 10 * NR_QUEUE);

	hdr = malloc(NR_QUEUE * sizeof(struct sg_io_hdr));
	memset(hdr, 0, NR_QUEUE * sizeof(struct sg_io_hdr));

	sense = malloc(NR_QUEUE * 64);
	memset(sense, 0, 64);
	
	for (i = 0; i < NR_QUEUE; i++) {
#if NR_VECS > 1
		int j;
		hdr[i].dxferp = vecs[i];
		hdr[i].iovec_count = NR_VECS;
		for (j = 0; j < NR_VECS; j++) {
			int size = SECTORS*BS/NR_VECS;
			vecs[i][j].iov_base = ALIGN(malloc(size*2));
			vecs[i][j].iov_len = size;
		}
#else
		buffer[i] = ALIGN(malloc(SECTORS*BS+BS-1));
#endif
	}

	for (read_kb = 0; read_kb < 256*1024;) {
		for (i = 0; i < nr_queue; i++) {
			struct sg_io_hdr *h = hdr + i;
#if NR_VECS <= 1
			h->dxferp = buffer[i];
#endif
			h->cmdp = cdb + i*10;
			h->pack_id = i;
			h->sbp = sense + i*64;
			h->mx_sb_len = 64;

			lba = (1+(int) (5120000.0*rand()/(RAND_MAX+1.0)));

			do_read10(fd, h, lba);
			lba += SECTORS;
		}

		ret = queue_cmds(fd, hdr, nr_queue);
		if (ret == -1) {
			perror("write cmds\n");
			break;
		} else if (!ret) {
			printf("strange, nothing queued but no error\n");
			break;
		} else if ((ret / sizeof(struct sg_io_hdr)) < nr_queue) {
			nr_queue = ret / sizeof(struct sg_io_hdr);
			printf("adjusting depth %d\n", nr_queue);
		}
		if (wait_cmd(fd, hdr, nr_queue) == -1) {
			perror("read cmd\n");
			break;
		}
		read_kb += nr_queue * (SECTORS >> 1);
	}

	close(fd);
	return 0;
}

When you pass data structures in via {read,write}{,v}() system calls, you make it next to impossible for the CONFIG_COMPAT layer to cope with this.

Please consider another way to pass in those sg_io_* things.

Any suggestions?

Jens Axboe

ioctl() :-(

Or use a more portable well-defined type which does not change size nor layout between 32-bit and 64-bit environments.

ioctls suck :-)

Yeah that's what I thought you meant... Problem is, to stay compatible with sg v3 I cannot do much about the structure. It should have been designed with this in mind from the beginning.

But I can certainly draft a new structure that works...

Jens Axboe

IMO if this (the above) is not done, the interface needs work.

For interfaces that replace ioctl(2) with read(2)/write(2), for passing data structures to/from the kernel, Al has rightly suggested that these structures be not only fixed size (as David described above), but also fixed-endian.

Jeff

I completely agree with that, we need a different structure for other devices as well. Show me what you'd like for libata, for instance.

Jens Axboe

If a new structure with anything but 'S' (0x53) in the first 4 bytes was chosen then the bsg driver could handle the new structure and sg_io_hdr.

For example:

struct bsg_io_hdr {
    int8_t interface_id[4];   /* [i] 'B' in each element (required) */
    uint8_t dxfer_direction;
    .............
};

SCSI commands are all big endian and that seems to work fine. Could pointers be passed as 8 byte big endian unsigned integers?

Doug Gilbert