Re: [PATCH mini-os enhancements for vtpm 2/8] add posix io to blkfront

[Samuel Thibault <samuel.thibault@ens-lyon.org>]

Matthew Fioravante, le Mon 17 Sep 2012 17:55:43 -0400, a écrit :
> +   /* Read mode checks */
> +   else
> +   {
> +      /*If the requested read is bigger than the disk, just
> +       * read as much as we can until the end */
> +      if(offset + count > disksize) {
> +         count = offset >= disksize ? 0 : disksize - offset;
> +      }
> +   }

Perhaps return 0 here already instead of just setting count to 0?

> +
> +   /* Setup aiocb block object */
> +   aiocb.aio_dev = dev;
> +   aiocb.aio_nbytes = blocksize;
> +   aiocb.aio_offset = blknum * blocksize;
> +   aiocb.aio_cb = NULL;
> +   aiocb.data = NULL;
> +
> +   /* If our buffer is unaligned or its aligned but we will need to rw
> a partial block
> +    * then a copy will have to be done */
> +   if(!alignedbuf || blkoff != 0 || count % blocksize != 0) {
> +      copybuf = _xmalloc(blocksize, dev->info.sector_size);
> +   }
> +
> +   rc = count;
> +   while(count > 0) {
> +      /* determine how many bytes to read/write from/to the current
> block buffer */
> +      bytes = count > (blocksize - blkoff) ? blocksize - blkoff : count;

Mmm. Optimizing the non-aligned case would make the code
tricky, but shouldn't optimizing the aligned case at least
a little bit not too hard? i.e. set bytes to max(count,
BLKIF_MAX_SEGMENTS_PER_REQUEST*PAGE_SIZE); in the optimized case? That'd
get much better 44KiB transfers instead of one sector at a time.

Ideally we should even push a series of aio requests, but that's a lot
less easy since you need to know how many requests you can afford.

Apart from that,

Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>