From mboxrd@z Thu Jan 1 00:00:00 1970 From: Li Dongyang Subject: Re: [PATCH V3 1/3] xen-blkback: add BLKIF_OP_TRIM and backend types Date: Thu, 25 Aug 2011 14:34:14 +0800 Message-ID: References: <1314177825-22360-1-git-send-email-lidongyang@novell.com> <1314177825-22360-2-git-send-email-lidongyang@novell.com> <4E54EE080200007800052DEC@victor.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <4E54EE080200007800052DEC@victor.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: xen-devel@lists.xensource.com, owen.smith@citrix.com List-Id: xen-devel@lists.xenproject.org On Wed, Aug 24, 2011 at 6:26 PM, Jan Beulich wrote: >>>> On 24.08.11 at 11:23, Li Dongyang wrote: >> This adds the BLKIF_OP_TRIM for blkfront and blkback, also 2 enums telli= ng >> us the type of the backend, used in blkback to determine what to do when= we >> see a trim request. >> The BLKIF_OP_TRIM part is just taken from Owen Smith, Thanks >> >> Signed-off-by: Owen Smith >> Signed-off-by: Li Dongyang >> --- >> =A0drivers/block/xen-blkback/common.h | =A0 10 +++++++++- >> =A0include/xen/interface/io/blkif.h =A0 | =A0 19 +++++++++++++++++++ >> =A02 files changed, 28 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/block/xen-blkback/common.h >> b/drivers/block/xen-blkback/common.h >> index 9e40b28..146d325 100644 >> --- a/drivers/block/xen-blkback/common.h >> +++ b/drivers/block/xen-blkback/common.h >> @@ -113,6 +113,11 @@ enum blkif_protocol { >> =A0 =A0 =A0 BLKIF_PROTOCOL_X86_64 =3D 3, >> =A0}; >> >> +enum blkif_backend_type { >> + =A0 =A0 BLKIF_BACKEND_PHY =A0=3D 1, >> + =A0 =A0 BLKIF_BACKEND_FILE =3D 2, >> +}; >> + >> =A0struct xen_vbd { >> =A0 =A0 =A0 /* What the domain refers to this vbd as. */ >> =A0 =A0 =A0 blkif_vdev_t =A0 =A0 =A0 =A0 =A0 =A0handle; >> @@ -138,6 +143,7 @@ struct xen_blkif { >> =A0 =A0 =A0 unsigned int =A0 =A0 =A0 =A0 =A0 =A0irq; >> =A0 =A0 =A0 /* Comms information. */ >> =A0 =A0 =A0 enum blkif_protocol =A0 =A0 blk_protocol; >> + =A0 =A0 enum blkif_backend_type blk_backend_type; >> =A0 =A0 =A0 union blkif_back_rings =A0blk_rings; >> =A0 =A0 =A0 struct vm_struct =A0 =A0 =A0 =A0*blk_ring_area; >> =A0 =A0 =A0 /* The VBD attached to this interface. */ >> @@ -159,8 +165,10 @@ struct xen_blkif { >> =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_wr_req; >> =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_oo_req; >> =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_f_req; >> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_tr_req; >> =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_rd_sect; >> =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_wr_sect; >> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 st_tr_sect; > > Just to repeat - I don't think this piece of statistic is very useful, th= e > more that you use "int" here while ... > >> >> =A0 =A0 =A0 wait_queue_head_t =A0 =A0 =A0 waiting_to_free; >> >> @@ -182,7 +190,7 @@ struct xen_blkif { >> >> =A0struct phys_req { >> =A0 =A0 =A0 unsigned short =A0 =A0 =A0 =A0 =A0dev; >> - =A0 =A0 unsigned short =A0 =A0 =A0 =A0 =A0nr_sects; >> + =A0 =A0 blkif_sector_t =A0 =A0 =A0 =A0 =A0nr_sects; > > ... you specifically widen the field to 64 bits here. > sounds reasonable, gonna kill the stat stuff > Also, all of the changes to this header look somewhat misplaced, they > should rather be part of the backend patch. > > Jan > >> =A0 =A0 =A0 struct block_device =A0 =A0 *bdev; >> =A0 =A0 =A0 blkif_sector_t =A0 =A0 =A0 =A0 =A0sector_number; >> =A0}; >> diff --git a/include/xen/interface/io/blkif.h >> b/include/xen/interface/io/blkif.h >> index 3d5d6db..43762dd 100644 >> --- a/include/xen/interface/io/blkif.h >> +++ b/include/xen/interface/io/blkif.h >> @@ -57,6 +57,19 @@ typedef uint64_t blkif_sector_t; >> =A0 * "feature-flush-cache" node! >> =A0 */ >> =A0#define BLKIF_OP_FLUSH_DISKCACHE =A0 3 >> + >> +/* >> + * 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 =A0 =A0 =A0 =A0 =A0 =A0 =A05 >> + >> =A0/* >> =A0 * Maximum scatter/gather segments per request. >> =A0 * This is carefully chosen so that sizeof(struct blkif_ring) <=3D PA= GE_SIZE. >> @@ -74,6 +87,11 @@ struct blkif_request_rw { >> =A0 =A0 =A0 } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; >> =A0}; >> >> +struct blkif_request_trim { >> + =A0 =A0 blkif_sector_t sector_number; >> + =A0 =A0 uint64_t nr_sectors; >> +}; >> + >> =A0struct blkif_request { >> =A0 =A0 =A0 uint8_t =A0 =A0 =A0 =A0operation; =A0 =A0/* BLKIF_OP_??? =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ >> =A0 =A0 =A0 uint8_t =A0 =A0 =A0 =A0nr_segments; =A0/* number of segments= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ >> @@ -81,6 +99,7 @@ struct blkif_request { >> =A0 =A0 =A0 uint64_t =A0 =A0 =A0 id; =A0 =A0 =A0 =A0 =A0 /* private gues= t value, echoed in resp =A0*/ >> =A0 =A0 =A0 union { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct blkif_request_rw rw; >> + =A0 =A0 =A0 =A0 =A0 =A0 struct blkif_request_trim trim; >> =A0 =A0 =A0 } u; >> =A0}; >> > > > >