[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union



On Friday 14 January 2011 17:44:22 owen.smith@xxxxxxxxxx wrote:
> From: Owen Smith <owen.smith@xxxxxxxxxx>
>
> # HG
> changeset patch
> # User root@xxxxxxxxxxxxxxxxxxxxxxx
> # Date 1294919321 0
> # Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
> # Parent  7b4c82f07281ad9c48b652e2a305a7be607c5283
> blkif: Move read/write/barrier specific fields into a union
>
> Patches xen-unstable.hg
>
> Modifies the blkif ring interface by placing the request specific
> fields into a union in order to support additional operation types.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

When a blkif interface change is acceptable can you also
allow to deal with 64kb blocksize, please?
This fixes a performance problem with *BSD guests then.

Thanks,
Christoph


>
> diff -r 7b4c82f07281 -r 428420495fd3 extras/mini-os/blkfront.c
> --- a/extras/mini-os/blkfront.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/extras/mini-os/blkfront.c Thu Jan 13 11:48:41 2011 +0000
> @@ -361,22 +361,22 @@
>      req->nr_segments = n;
>      req->handle = dev->handle;
>      req->id = (uintptr_t) aiocbp;
> -    req->sector_number = aiocbp->aio_offset / 512;
> +    req->u.rw.sector_number = aiocbp->aio_offset / 512;
>
>      for (j = 0; j < n; j++) {
> -        req->seg[j].first_sect = 0;
> -        req->seg[j].last_sect = PAGE_SIZE / 512 - 1;
> +        req->u.rw.seg[j].first_sect = 0;
> +        req->u.rw.seg[j].last_sect = PAGE_SIZE / 512 - 1;
>      }
> -    req->seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) /
> 512; -    req->seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf +
> aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512; +   
> req->u.rw.seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) /
> 512; +    req->u.rw.seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf +
> aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512; for (j = 0; j < n; j++) {
>         uintptr_t data = start + j * PAGE_SIZE;
>          if (!write) {
>              /* Trigger CoW if needed */
> -            *(char*)(data + (req->seg[j].first_sect << 9)) = 0;
> +            *(char*)(data + (req->u.rw.seg[j].first_sect << 9)) = 0;
>              barrier();
>          }
> -       aiocbp->gref[j] = req->seg[j].gref =
> +       aiocbp->gref[j] = req->u.rw.seg[j].gref =
>              gnttab_grant_access(dev->dom, virtual_to_mfn(data), write);
>      }
>
> @@ -432,7 +432,7 @@
>      req->handle = dev->handle;
>      req->id = id;
>      /* Not needed anyway, but the backend will check it */
> -    req->sector_number = 0;
> +    req->u.rw.sector_number = 0;
>      dev->ring.req_prod_pvt = i + 1;
>      wmb();
>      RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&dev->ring, notify);
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap/drivers/tapdisk.c
> --- a/tools/blktap/drivers/tapdisk.c    Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap/drivers/tapdisk.c    Thu Jan 13 11:48:41 2011 +0000
> @@ -528,10 +528,10 @@
>  segment_start(blkif_request_t *req, int sidx)
>  {
>         int i;
> -       uint64_t start = req->sector_number;
> +       uint64_t start = req->u.rw.sector_number;
>
>         for (i = 0; i < sidx; i++)
> -               start += (req->seg[i].last_sect - req->seg[i].first_sect +
> 1); +               start += (req->u.rw.seg[i].last_sect -
> req->u.rw.seg[i].first_sect + 1);
>
>         return start;
>  }
> @@ -600,13 +600,13 @@
>         struct disk_driver *parent = dd->next;
>
>         seg_start = segment_start(req, sidx);
> -       seg_end   = seg_start + req->seg[sidx].last_sect + 1;
> +       seg_end   = seg_start + req->u.rw.seg[sidx].last_sect + 1;
>
>         ASSERT(sector >= seg_start && sector + nr_secs <= seg_end);
>
>         page  = (char *)MMAP_VADDR(info->vstart,
>                                    (unsigned long)req->id, sidx);
> -       page += (req->seg[sidx].first_sect << SECTOR_SHIFT);
> +       page += (req->u.rw.seg[sidx].first_sect << SECTOR_SHIFT);
>         page += ((sector - seg_start) << SECTOR_SHIFT);
>
>         if (!parent) {
> @@ -667,7 +667,7 @@
>                                req, sizeof(*req));
>                         blkif->pending_list[idx].status = BLKIF_RSP_OKAY;
>                         blkif->pending_list[idx].submitting = 1;
> -                       sector_nr = req->sector_number;
> +                       sector_nr = req->u.rw.sector_number;
>                 }
>
>                 if ((dd->flags & TD_RDONLY) &&
> @@ -677,16 +677,16 @@
>                 }
>
>                 for (i = start_seg; i < req->nr_segments; i++) {
> -                       nsects = req->seg[i].last_sect -
> -                                req->seg[i].first_sect + 1;
> +                       nsects = req->u.rw.seg[i].last_sect -
> +                                req->u.rw.seg[i].first_sect + 1;
>
> -                       if ((req->seg[i].last_sect >= page_size >> 9) ||
> +                       if ((req->u.rw.seg[i].last_sect >= page_size >> 9)
> || (nsects <= 0))
>                                 continue;
>
>                         page  = (char *)MMAP_VADDR(info->vstart,
>                                                    (unsigned long)req->id,
> i); -                       page += (req->seg[i].first_sect <<
> SECTOR_SHIFT); +                       page += (req->u.rw.seg[i].first_sect
> << SECTOR_SHIFT);
>
>                         if (sector_nr >= s->size) {
>                                 DPRINTF("Sector request failed:\n");
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-diff.c
> --- a/tools/blktap2/drivers/tapdisk-diff.c      Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-diff.c      Thu Jan 13 11:48:41
> 2011 +0000 @@ -363,13 +363,13 @@
>         breq                = &sreq->blkif_req;
>         breq->id            = idx;
>         breq->nr_segments   = r->blkif_req.nr_segments;
> -       breq->sector_number = r->blkif_req.sector_number;
> +       breq->u.rw.sector_number = r->blkif_req.u.rw.sector_number;
>         breq->operation     = BLKIF_OP_READ;
>
>         for (int i = 0; i < r->blkif_req.nr_segments; i++) {
> -               struct blkif_request_segment *seg = breq->seg + i;
> -               seg->first_sect = r->blkif_req.seg[i].first_sect;
> -               seg->last_sect  = r->blkif_req.seg[i].last_sect;
> +               struct blkif_request_segment *seg = breq->u.rw.seg + i;
> +               seg->first_sect = r->blkif_req.u.rw.seg[i].first_sect;
> +               seg->last_sect  = r->blkif_req.u.rw.seg[i].last_sect;
>         }
>         s->cur += sreq->secs;
>
> @@ -426,12 +426,12 @@
>                 breq                = &sreq->blkif_req;
>                 breq->id            = idx;
>                 breq->nr_segments   = 0;
> -               breq->sector_number = sreq->sec;
> +               breq->u.rw.sector_number = sreq->sec;
>                 breq->operation     = BLKIF_OP_READ;
>
>                 for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
>                         uint32_t secs;
> -                       struct blkif_request_segment *seg = breq->seg + i;
> +                       struct blkif_request_segment *seg = breq->u.rw.seg
> + i;
>
>                         secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
>                         secs = MIN(((blk + 1) << SPB_SHIFT) - s->cur,
> secs); diff -r 7b4c82f07281 -r 428420495fd3
> tools/blktap2/drivers/tapdisk-image.c ---
> a/tools/blktap2/drivers/tapdisk-image.c     Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-image.c     Thu Jan 13 11:48:41 2011
> +0000 @@ -148,15 +148,15 @@
>         psize = getpagesize();
>
>         for (i = 0; i < req->nr_segments; i++) {
> -               nsects = req->seg[i].last_sect - req->seg[i].first_sect +
> 1; +               nsects = req->u.rw.seg[i].last_sect -
> req->u.rw.seg[i].first_sect + 1;
>
> -               if (req->seg[i].last_sect >= psize >> 9 || nsects <= 0)
> +               if (req->u.rw.seg[i].last_sect >= psize >> 9 || nsects <=
> 0) goto fail;
>
>                 total += nsects;
>         }
>
> -       if (req->sector_number + nsects > info->size)
> +       if (req->u.rw.sector_number + nsects > info->size)
>                 goto fail;
>
>         return 0;
> @@ -164,6 +164,6 @@
>  fail:
>         ERR(-EINVAL, "bad request on %s (%s, %"PRIu64"): id: %"PRIu64": %d
> at %"PRIu64, image->name, (rdonly ? "ro" : "rw"), info->size, req->id, -   
>        req->operation, req->sector_number + total);
> +           req->operation, req->u.rw.sector_number + total);
>         return -EINVAL;
>  }
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-stream.c
> --- a/tools/blktap2/drivers/tapdisk-stream.c    Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-stream.c    Thu Jan 13 11:48:41
> 2011 +0000 @@ -293,12 +293,12 @@
>                 breq                = &sreq->blkif_req;
>                 breq->id            = idx;
>                 breq->nr_segments   = 0;
> -               breq->sector_number = sreq->sec;
> +               breq->u.rw.sector_number = sreq->sec;
>                 breq->operation     = BLKIF_OP_READ;
>
>                 for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
>                         uint32_t secs = MIN(s->end - s->cur, psize >>
> SECTOR_SHIFT); -                       struct blkif_request_segment *seg =
> breq->seg + i; +                       struct blkif_request_segment *seg =
> breq->u.rw.seg + i;
>
>                         if (!secs)
>                                 break;
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-vbd.c
> --- a/tools/blktap2/drivers/tapdisk-vbd.c       Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-vbd.c       Thu Jan 13 11:48:41
> 2011 +0000 @@ -1066,7 +1066,7 @@
>         rsp->status = vreq->status;
>
>         DBG(TLOG_DBG, "writing req %d, sec 0x%08"PRIx64", res %d to
> ring\n", -           (int)tmp.id, tmp.sector_number, vreq->status);
> +           (int)tmp.id, tmp.u.rw.sector_number, vreq->status);
>
>         if (rsp->status != BLKIF_RSP_OKAY)
>                 ERR(EIO, "returning BLKIF_RSP %d", rsp->status);
> @@ -1181,10 +1181,10 @@
>  tapdisk_vbd_breq_get_sector(blkif_request_t *breq, td_request_t treq)
>  {
>      int seg, nsects;
> -    uint64_t sector_nr = breq->sector_number;
> +    uint64_t sector_nr = breq->u.rw.sector_number;
>
>      for(seg=0; seg < treq.sidx; seg++) {
> -        nsects = breq->seg[seg].last_sect - breq->seg[seg].first_sect + 1;
> +        nsects = breq->u.rw.seg[seg].last_sect -
> breq->u.rw.seg[seg].first_sect + 1; sector_nr += nsects;
>      }
>
> @@ -1366,7 +1366,7 @@
>         req       = &vreq->req;
>         id        = req->id;
>         ring      = &vbd->ring;
> -       sector_nr = req->sector_number;
> +       sector_nr = req->u.rw.sector_number;
>         image     = tapdisk_vbd_first_image(vbd);
>
>         vreq->submitting = 1;
> @@ -1385,10 +1385,10 @@
>                 goto fail;
>
>         for (i = 0; i < req->nr_segments; i++) {
> -               nsects = req->seg[i].last_sect - req->seg[i].first_sect +
> 1; +               nsects = req->u.rw.seg[i].last_sect -
> req->u.rw.seg[i].first_sect + 1; page   = (char *)MMAP_VADDR(ring->vstart,
>                                            (unsigned long)req->id, i);
> -               page  += (req->seg[i].first_sect << SECTOR_SHIFT);
> +               page  += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
>
>                 treq.id             = id;
>                 treq.sidx           = i;
> @@ -1482,7 +1482,7 @@
>                 vreq->status = BLKIF_RSP_OKAY;
>                 DBG(TLOG_DBG, "retry #%d of req %"PRIu64", "
>                     "sec 0x%08"PRIx64", nr_segs: %d\n", vreq->num_retries,
> -                   vreq->req.id, vreq->req.sector_number,
> +                   vreq->req.id, vreq->req.u.rw.sector_number,
>                     vreq->req.nr_segments);
>
>                 err = tapdisk_vbd_issue_request(vbd, vreq);
> diff -r 7b4c82f07281 -r 428420495fd3 xen/include/public/io/blkif.h
> --- a/xen/include/public/io/blkif.h     Wed Jan 05 23:54:15 2011 +0000
> +++ b/xen/include/public/io/blkif.h     Thu Jan 13 11:48:41 2011 +0000
> @@ -103,7 +103,22 @@
>      uint8_t     first_sect, last_sect;
>  };
>
> +struct blkif_request_rw {
> +    blkif_sector_t sector_number;/* start sector idx on disk (r/w only) 
> */ +    struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> +};
> +
>  struct blkif_request {
> +    uint8_t        operation;    /* BLKIF_OP_???                        
> */ +    uint8_t        nr_segments;  /* number of segments                 
>  */ +    blkif_vdev_t   handle;       /* only for read/write requests      
>   */ +    uint64_t       id;           /* private guest value, echoed in
> resp  */ +    union {
> +        struct blkif_request_rw     rw;
> +    } u;
> +};
> +
> +struct blkif_request_old {
>      uint8_t        operation;    /* BLKIF_OP_???                        
> */ uint8_t        nr_segments;  /* number of segments                   */
> blkif_vdev_t   handle;       /* only for read/write requests         */
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.