| Message ID | 20100912214256.GA4854@lst.de |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 12, 2010 at 10:42 PM, Christoph Hellwig <hch@lst.de> wrote: > Use qemu_blockalign for all allocations in the block layer. This allows > increasing the required alignment, which is need to support O_DIRECT on > devices with large block sizes. > > Signed-off-by: Christoph Hellwig <hch@lst.de> I noticed that the backing file may have different alignment requirements than the image file but qemu_blockalign() currently only takes into account the image file's requirements. Not necessarily something for this patch, but since you're in the area I wanted to mention it. It seems safe to take the max alignment (they should both be powers of two I think) of the image and backing files. Stefan
On Mon, Sep 13, 2010 at 08:02:28PM +0100, Stefan Hajnoczi wrote: > On Sun, Sep 12, 2010 at 10:42 PM, Christoph Hellwig <hch@lst.de> wrote: > > Use qemu_blockalign for all allocations in the block layer. ?This allows > > increasing the required alignment, which is need to support O_DIRECT on > > devices with large block sizes. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > I noticed that the backing file may have different alignment > requirements than the image file but qemu_blockalign() currently only > takes into account the image file's requirements. Not necessarily > something for this patch, but since you're in the area I wanted to > mention it. > > It seems safe to take the max alignment (they should both be powers of > two I think) of the image and backing files. Currently we check the required alignment based on the blocksize we present to the guest, which is set by the driver. That's a bit upside down, but the best we can do. The problem is that the blocksize of the device the image (and it's backing file reside on) are the lower bound of what we can support in the guest without massive read/modify/write cycles. If we use the page cache the kernel does the cycles for us, but for cache=none we would have to do it ourselves in a rather inefficient way.
Index: qemu/hw/scsi-disk.c =================================================================== --- qemu.orig/hw/scsi-disk.c 2010-09-12 14:42:40.942759377 -0300 +++ qemu/hw/scsi-disk.c 2010-09-12 14:43:04.694759377 -0300 @@ -70,14 +70,15 @@ struct SCSIDiskState char *serial; }; -static SCSIDiskReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun) +static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag, + uint32_t lun) { SCSIRequest *req; SCSIDiskReq *r; - req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun); + req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun); r = DO_UPCAST(SCSIDiskReq, req, req); - r->iov.iov_base = qemu_memalign(512, SCSI_DMA_BUF_SIZE); + r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE); return r; } @@ -939,7 +940,7 @@ static int32_t scsi_send_command(SCSIDev } /* ??? Tags are not unique for different luns. We only implement a single lun, so this should not matter. */ - r = scsi_new_request(d, tag, lun); + r = scsi_new_request(s, tag, lun); outbuf = (uint8_t *)r->iov.iov_base; is_write = 0; DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]); Index: qemu/hw/sd.c =================================================================== --- qemu.orig/hw/sd.c 2010-09-12 14:31:04.387759376 -0300 +++ qemu/hw/sd.c 2010-09-12 14:43:04.695759377 -0300 @@ -440,7 +440,7 @@ SDState *sd_init(BlockDriverState *bs, i SDState *sd; sd = (SDState *) qemu_mallocz(sizeof(SDState)); - sd->buf = qemu_memalign(512, 512); + sd->buf = qemu_blockalign(bs, 512); sd->spi = is_spi; sd->enable = 1; sd_reset(sd, bs); Index: qemu/qemu-io.c =================================================================== --- qemu.orig/qemu-io.c 2010-09-12 14:31:04.394759376 -0300 +++ qemu/qemu-io.c 2010-09-12 14:43:04.699759377 -0300 @@ -61,7 +61,7 @@ static void *qemu_io_alloc(size_t len, i if (misalign) len += MISALIGN_OFFSET; - buf = qemu_memalign(512, len); + buf = qemu_blockalign(bs, len); memset(buf, pattern, len); if (misalign) buf += MISALIGN_OFFSET; Index: qemu/qemu-nbd.c =================================================================== --- qemu.orig/qemu-nbd.c 2010-09-12 14:31:04.401759376 -0300 +++ qemu/qemu-nbd.c 2010-09-12 14:43:04.706759377 -0300 @@ -446,7 +446,7 @@ int main(int argc, char **argv) max_fd = sharing_fds[0]; nb_fds++; - data = qemu_memalign(512, NBD_BUFFER_SIZE); + data = qemu_blockalign(bs, NBD_BUFFER_SIZE); if (data == NULL) errx(EXIT_FAILURE, "Cannot allocate data buffer"); Index: qemu/posix-aio-compat.c =================================================================== --- qemu.orig/posix-aio-compat.c 2010-09-12 14:42:46.725759377 -0300 +++ qemu/posix-aio-compat.c 2010-09-12 14:43:04.711759377 -0300 @@ -270,7 +270,7 @@ static ssize_t handle_aiocb_rw(struct qe * Ok, we have to do it the hard way, copy all segments into * a single aligned buffer. */ - buf = qemu_memalign(512, aiocb->aio_nbytes); + buf = qemu_blockalign(aiocb->common.bs, aiocb->aio_nbytes); if (aiocb->aio_type & QEMU_AIO_WRITE) { char *p = buf; int i;
Use qemu_blockalign for all allocations in the block layer. This allows increasing the required alignment, which is need to support O_DIRECT on devices with large block sizes. Signed-off-by: Christoph Hellwig <hch@lst.de>