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

Re: [Xen-devel] [PATCH] add trim command to blkback interface



On 12/06/2010 03:28 AM, Owen Smith wrote:
>
> [PATCH] add trim command to blkback interface
>
>  
>
> This patch adds the trim operation to the blkback ring protocol.
>
> The ring protocol is extended by unioning the current read/write/write
> barrier specific fields with fields specific for trim, and allowing
> further expansion with additional union members, providing the
> structure size and alignment do not change.
>
>  
>
> Blkback will respond to trim operations with a BLKIF_RSP_EOPNOTSUPP,
> to avoid using the default response of writing an error log entry and
> returning BLKIF_RSP_ERROR.
>
>  
>
> Trim commands are passed with sector_number as the sector index to
> begin trim operations at, and nr_sectors as the number of sectors to
> be trimmed. These sectors should be trimmed if the underlying block
> device supports trimming. More information about trim commands:
>
> http://t13.org/Documents/UploadedDocuments/docs2008/e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc
>

Aside from the comments in my reply to Jan's mail, I can't actually get
an applyable patch out of this; its all mashed up with
quoted-printable.  Could you repost as a plain-text attachment if you
can't convince your mailer to send a plain-text email?

Thanks,
    J

>  
>
> This patch is intended to fix an interface, not supply the
> implementation of trim in the backend drivers.
>
>  
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
>
>  
>
>  
>
> diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
>
> index 0bef445..bd09512 100644
>
> --- a/drivers/xen/blkback/blkback.c
>
> +++ b/drivers/xen/blkback/blkback.c
>
> @@ -367,6 +367,11 @@ static int do_block_io_op(blkif_t *blkif)
>
>                                                blkif->st_wr_req++;
>
>                                               
> dispatch_rw_block_io(blkif, &req, pending_req);
>
>                                                break;
>
> +                             case BLKIF_OP_TRIM:
>
> +                                             make_response(blkif,
> req.id, req.operation,
>
> +                                                                  
> BLKIF_RSP_EOPNOTSUPP);
>
> +                                             free_req(pending_req);
>
> +                                             break;
>
>                                default:
>
>                                                /* A good sign
> something is wrong: sleep for a while to
>
>                                                 * avoid excessive CPU
> consumption by a bad guest. */
>
> @@ -424,7 +429,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
>                }
>
>                 preq.dev           = req->handle;
>
> -              preq.sector_number = req->sector_number;
>
> +             preq.sector_number = req->rw.sector_number;
>
>                preq.nr_sects      = 0;
>
>                 pending_req->blkif     = blkif;
>
> @@ -436,11 +441,11 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
>                for (i = 0; i < nseg; i++) {
>
>                                uint32_t flags;
>
> -                              seg[i].nsec = req->seg[i].last_sect -
>
> -                                              req->seg[i].first_sect + 1;
>
> +                             seg[i].nsec = req->rw.seg[i].last_sect -
>
> +                                            
> req->rw.seg[i].first_sect + 1;
>
> -                              if ((req->seg[i].last_sect >=
> (PAGE_SIZE >> 9)) ||
>
> -                                  (req->seg[i].last_sect <
> req->seg[i].first_sect))
>
> +                             if ((req->rw.seg[i].last_sect >=
> (PAGE_SIZE >> 9)) ||
>
> +                                 (req->rw.seg[i].last_sect <
> req->rw.seg[i].first_sect))
>
>                                                goto fail_response;
>
>                                preq.nr_sects += seg[i].nsec;
>
> @@ -448,7 +453,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
>                                if (operation != READ)
>
>                                                flags |= GNTMAP_readonly;
>
>                                gnttab_set_map_op(&map[i],
> vaddr(pending_req, i), flags,
>
> -                                                               
> req->seg[i].gref, blkif->domid);
>
> +                                                              
> req->rw.seg[i].gref, blkif->domid);
>
>                }
>
>                 ret =
> HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg);
>
> @@ -466,11 +471,11 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
>                                               
> page_to_pfn(pending_page(pending_req, i)),
>
>                                               
> FOREIGN_FRAME(map[i].dev_bus_addr >> PAGE_SHIFT));
>
>                                seg[i].buf  = map[i].dev_bus_addr |
>
> -                                              (req->seg[i].first_sect
> << 9);
>
> +                                            
> (req->rw.seg[i].first_sect << 9);
>
>                               
> blkback_pagemap_set(vaddr_pagenr(pending_req, i),
>
>                                                               
>     pending_page(pending_req, i),
>
>                                                               
>     blkif->domid, req->handle,
>
> -                                                                 
> req->seg[i].gref);
>
> +                                                                
> req->rw.seg[i].gref);
>
>                                pending_handle(pending_req, i) =
> map[i].handle;
>
>                }
>
> diff --git a/include/xen/blkif.h b/include/xen/blkif.h
>
> index 7172081..e727e5d 100644
>
> --- a/include/xen/blkif.h
>
> +++ b/include/xen/blkif.h
>
> @@ -44,8 +44,18 @@ struct blkif_x86_32_request {
>
>                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  */
>
> -              blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only)  */
>
> -              struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> +             union {
>
> +                             struct blkif_x86_32_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];
>
> +                             } rw;
>
> +
>
> +                             struct blkif_x86_32_request_trim {
>
> +                                             blkif_sector_t
> sector_number;
>
> +                                             uint64_t       nr_sectors;
>
> +                             } trim;
>
> +             };
>
> };
>
> struct blkif_x86_32_response {
>
>                uint64_t        id;              /* copied from request */
>
> @@ -62,8 +72,18 @@ struct blkif_x86_64_request {
>
>                uint8_t        nr_segments;  /* number of
> segments                   */
>
>                blkif_vdev_t   handle;       /* only for read/write
> requests         */
>
>                uint64_t       __attribute__((__aligned__(8))) id;
>
> -              blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only)  */
>
> -              struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> +             union {
>
> +                             struct blkif_x86_64_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];
>
> +                             } rw;
>
> +
>
> +                             struct blkif_x86_64_request_trim {
>
> +                                             blkif_sector_t
> sector_number;
>
> +                                             uint64_t       nr_sectors;
>
> +                             } trim;
>
> +             };
>
> };
>
> struct blkif_x86_64_response {
>
>                uint64_t       __attribute__((__aligned__(8))) id;
>
> @@ -97,12 +117,24 @@ static void inline blkif_get_x86_32_req(struct
> blkif_request *dst, struct blkif_
>
>                dst->nr_segments = src->nr_segments;
>
>                dst->handle = src->handle;
>
>                dst->id = src->id;
>
> -              dst->sector_number = src->sector_number;
>
> -              barrier();
>
> -              if (n > dst->nr_segments)
>
> -                              n = dst->nr_segments;
>
> -              for (i = 0; i < n; i++)
>
> -                              dst->seg[i] = src->seg[i];
>
> +             switch (src->operation) {
>
> +             case BLKIF_OP_READ:
>
> +             case BLKIF_OP_WRITE:
>
> +             case BLKIF_OP_WRITE_BARRIER:
>
> +                             dst->rw.sector_number =
> src->rw.sector_number;
>
> +                             barrier();
>
> +                             if (n > dst->nr_segments)
>
> +                                             n = dst->nr_segments;
>
> +                             for (i = 0; i < n; i++)
>
> +                                             dst->rw.seg[i] =
> src->rw.seg[i];
>
> +                             break;
>
> +             case BLKIF_OP_TRIM:
>
> +                             dst->trim.sector_number =
> src->trim.sector_number;
>
> +                             dst->trim.nr_sectors = src->trim.nr_sectors;
>
> +                             break;
>
> +             default:
>
> +                             break;
>
> +             }
>
> }
>
>  static void inline blkif_get_x86_64_req(struct blkif_request *dst,
> struct blkif_x86_64_request *src)
>
> @@ -112,12 +144,24 @@ static void inline blkif_get_x86_64_req(struct
> blkif_request *dst, struct blkif_
>
>                dst->nr_segments = src->nr_segments;
>
>                dst->handle = src->handle;
>
>                dst->id = src->id;
>
> -              dst->sector_number = src->sector_number;
>
> -              barrier();
>
> -              if (n > dst->nr_segments)
>
> -                              n = dst->nr_segments;
>
> -              for (i = 0; i < n; i++)
>
> -                              dst->seg[i] = src->seg[i];
>
> +             switch (src->operation) {
>
> +             case BLKIF_OP_READ:
>
> +             case BLKIF_OP_WRITE:
>
> +             case BLKIF_OP_WRITE_BARRIER:
>
> +                             dst->rw.sector_number =
> src->rw.sector_number;
>
> +                             barrier();
>
> +                             if (n > dst->nr_segments)
>
> +                                             n = dst->nr_segments;
>
> +                             for (i = 0; i < n; i++)
>
> +                                             dst->rw.seg[i] =
> src->rw.seg[i];
>
> +                             break;
>
> +             case BLKIF_OP_TRIM:
>
> +                             dst->trim.sector_number =
> src->trim.sector_number;
>
> +                             dst->trim.nr_sectors = src->trim.nr_sectors;
>
> +                             break;
>
> +             default:
>
> +                             break;
>
> +             }
>
> }
>
>  #endif /* __XEN_BLKIF_H__ */
>
> diff --git a/include/xen/interface/io/blkif.h
> b/include/xen/interface/io/blkif.h
>
> index 68dd2b4..54bb598 100644
>
> --- a/include/xen/interface/io/blkif.h
>
> +++ b/include/xen/interface/io/blkif.h
>
> @@ -43,6 +43,17 @@ typedef uint64_t blkif_sector_t;
>
>   * create the "feature-barrier" node!
>
>   */
>
> #define BLKIF_OP_WRITE_BARRIER     2
>
> +/*
>
> + * Recognised only if "feature-trim" is present in backend xenbus info.
>
> + * The "feature_trim" node contains a boolean indicating whether trim
>
> + * requests are likely to succeed or fail. Either way, a trim request
>
> + * may fail at any time with BLKIF_RSP_EOPNOTSUPP if it is unsupported by
>
> + * the underlying block-device hardware. The boolean simply indicates
> whether
>
> + * or not it is worthwhile for the frontend to attempt trim requests.
>
> + * If a backend does not recognise BLKIF_OP_TRIM, it should *not*
>
> + * create the "feature-trim" node!
>
> + */
>
> +#define BLKIF_OP_TRIM             4
>
>  /*
>
>   * Maximum scatter/gather segments per request.
>
> @@ -56,13 +67,23 @@ struct blkif_request {
>
>                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  */
>
> -              blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only)  */
>
> -              struct blkif_request_segment {
>
> -                              grant_ref_t gref;        /* reference
> to I/O buffer frame        */
>
> -                              /* @first_sect: first sector in frame
> to transfer (inclusive).   */
>
> -                              /* @last_sect: last sector in frame to
> transfer (inclusive).     */
>
> -                              uint8_t     first_sect, last_sect;
>
> -              } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> +             union {
>
> +                             struct blkif_request_rw {
>
> +                                             blkif_sector_t
> sector_number;/* start sector idx on disk (r/w only)  */
>
> +                                             struct
> blkif_request_segment {
>
> +                                                            
> grant_ref_t gref;        /* reference to I/O buffer frame        */
>
> +                                                             /*
> @first_sect: first sector in frame to transfer (inclusive).   */
>
> +                                                             /*
> @last_sect: last sector in frame to transfer (inclusive).     */
>
> +                                                            
> uint8_t     first_sect, last_sect;
>
> +                                             }
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +                             } rw;
>
> +
>
> +                             struct blkif_request_trim {
>
> +                             blkif_sector_t sector_number;
>
> +                                     uint64_t       nr_sectors;
>
> +                             } trim;
>
> +             };
>
> };
>
>  struct blkif_response {
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel


_______________________________________________
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®.