From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 1 of 1] interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD Date: Tue, 11 Oct 2011 14:27:41 -0400 Message-ID: <20111011182741.GA1530@phenom.oracle.com> References: <20111010195842.GB5755@phenom.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20111010195842.GB5755@phenom.oracle.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: xen-devel@lists.xensource.com, Ian.Jackson@eu.citrix.com, Ian.Campbell@citrix.com, lidongyang@novell.com, owen.smith@citrix.com, paul.durrant@citrix.com, pasik@iki.fi, JBeulich@novell List-Id: xen-devel@lists.xenproject.org On Mon, Oct 10, 2011 at 03:58:42PM -0400, Konrad Rzeszutek Wilk wrote: > On Mon, Oct 10, 2011 at 01:50:11PM -0400, Konrad Rzeszutek Wilk wrote: Per Ian and Jan's suggestion (note, the structure is 4-byte aligned so we do not need to pad it): # HG changeset patch # Parent 72f339bc600d7a9629d3f9eb8a279fbf8be25b12 interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD The name 'trim' is specific to the ATA discard implementation. The name 'scsi unmap' is specific to the SCSI discard implementation. We should really use a generic name - and the name 'discard' looks to be the most generic of them all. Also update the description to mention the other parameters that the frontend can query the backend for: discard-aligment, discard-granularity, and discard-secure. We also utilize per Jan Beulich keen suggestion, the 8-bit reserved field to use as a flag value. Currently the only flag that can be passed for a discard operation is secure delete: BLKIF_OP_DISCARD_FLAG_SECURE. CC: lidongyang@novell.com CC: owen.smith@citrix.com CC: Pasi K=E4rkk=E4inen CC: JBeulich@novell.com Signed-off-by: Konrad Rzeszutek Wilk diff -r 72f339bc600d xen/include/public/io/blkif.h --- a/xen/include/public/io/blkif.h Mon Oct 10 11:21:51 2011 +0100 +++ b/xen/include/public/io/blkif.h Tue Oct 11 14:10:33 2011 -0400 @@ -82,26 +82,47 @@ */ #define BLKIF_OP_RESERVED_1 4 /* - * 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 + * Recognised only if "feature-discard" is present in backend xenbus inf= o. + * The "feature-discard" node contains a boolean indicating whether trim + * (ATA) or unmap (SCSI) - conviently called discard requests are likely + * to succeed or fail. Either way, a discard request * may fail at any time with BLKIF_RSP_EOPNOTSUPP if it is unsupported b= y * the underlying block-device hardware. The boolean simply indicates wh= ether - * 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! - *=20 - * Trim operation is a request for the underlying block device to mark - * extents to be erased. Trim operations are passed with sector_number a= s the - * sector index to begin trim operations at and nr_sectors as the number= of - * sectors to be trimmed. The specified sectors should be trimmed if the - * underlying block device supports trim operations, or a BLKIF_RSP_EOPN= OTSUPP - * should be returned. More information about trim operations at: + * or not it is worthwhile for the frontend to attempt discard requests. + * If a backend does not recognise BLKIF_OP_DISCARD, it should *not* + * create the "feature-discard" node! + * + * Discard operation is a request for the underlying block device to mar= k + * extents to be erased. However, discard does not guarantee that the bl= ocks + * will be erased from the device - it is just a hint to the device + * controller that these blocks are no longer in use. What the device + * controller does with that information is left to the controller. + * Discard operations are passed with sector_number as the + * sector index to begin discard operations at and nr_sectors as the num= ber of + * sectors to be discarded. The specified sectors should be discarded if= the + * underlying block device supports trim (ATA) or unmap (SCSI) operation= s, + * or a BLKIF_RSP_EOPNOTSUPP should be returned. + * More information about trim/unmap operations at: * http://t13.org/Documents/UploadedDocuments/docs2008/ * e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc + * http://www.seagate.com/staticfiles/support/disc/manuals/ + * Interface%20manuals/100293068c.pdf + * The backend can optionally provide three extra XenBus attributes to + * further optimize the discard functionality: + * 'discard-aligment' - Devices that support discard functionality may + * internally allocate space in units that are bigger than the exported + * logical block size. The discard-alignment parameter indicates how man= y bytes + * the beginning of the partition is offset from the internal allocation= unit's + * natural alignment. + * 'discard-granularity' - Devices that support discard functionality m= ay + * internally allocate space using units that are bigger than the logica= l block + * size. The discard-granularity parameter indicates the size of the int= ernal + * allocation unit in bytes if reported by the device. Otherwise the + * discard-granularity will be set to match the device's physical block = size. + * 'discard-secure' - All copies of the discarded sectors (potentially c= reated by + * garbage collection) must also be erased. */ -#define BLKIF_OP_TRIM 5 - +#define BLKIF_OP_DISCARD 5 /* * Maximum scatter/gather segments per request. * This is carefully chosen so that sizeof(blkif_ring_t) <=3D PAGE_SIZE. @@ -134,18 +155,20 @@ struct blkif_request { typedef struct blkif_request blkif_request_t; =20 /* - * Cast to this structure when blkif_request.operation =3D=3D BLKIF_OP_T= RIM - * sizeof(struct blkif_request_trim) <=3D sizeof(struct blkif_request) + * Cast to this structure when blkif_request.operation =3D=3D BLKIF_OP_D= ISCARD + * sizeof(struct blkif_request_discard) <=3D sizeof(struct blkif_request= ) */ -struct blkif_request_trim { - uint8_t operation; /* BLKIF_OP_TRIM = */ - uint8_t reserved; /* = */ +struct blkif_request_discard { + uint8_t operation; /* BLKIF_OP_DISCARD = */ + /* ignored if 'discard-secure=3D0' = */ +#define BLKIF_OP_DISCARD_FLAG_SECURE (1<<0) + uint8_t flag; /* BLKIF_OP_DISCARD_FLAG_SECURE or 0 = */ blkif_vdev_t handle; /* same as for read/write requests = */ uint64_t id; /* private guest value, echoed in resp = */ blkif_sector_t sector_number;/* start sector idx on disk = */ uint64_t nr_sectors; /* number of contiguous sectors to trim= */ }; -typedef struct blkif_request_trim blkif_request_trim_t; +typedef struct blkif_request_discard blkif_request_discard_t; =20 struct blkif_response { uint64_t id; /* copied from request */