Patch to alter BLKIF_OP_TRIM to BLKIF_OP_DISCARD (v1).
PATCH 1/1 Patch to alter BLKIF_OP_TRIM to BLKIF_OP_DISCARD (v1).
Hey guys,
Pasi mentioned on Li's (and Owen's) patches which provide TRIM/UNMAP support to the Linux backend/frontend that:
"
Isn't the generic name for this functionality "discard" in Linux?
and "trim" being the ATA specific discard-implementation, and "scsi unmap" the SAS/SCSI specific discard-implementation?
Just wondering..
"
and it sounds right to me. The problem is of course that the 'feature-trim' is already in the interface - but I was wondering how much it is set in stone? Could it be feasible to modify the backend/frontend in Citrix product to use 'feature-discard' instead? And naturally also the Novell drivers.
PATCH 1/1 interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1314761891 14400
# Node ID d2538ec1df2b9f4c39978d96ebe3bccccb068ad0 # Parent ac9aa65050e9abc8f1c12c8603acf3b99e22cddc
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.
xen/include/public/io/blkif.h +18 -14
@@ -82,25 +82,29 @@*/ #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 info. +* 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 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! -* -* Trim operation is a request for the underlying block device to mark -* extents to be erased. Trim operations 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. The specified sectors should be trimmed if the -* underlying block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP -* 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 mark +* extents to be erased. Discard operations are passed with sector_number as the +* sector index to begin discard operations at and nr_sectors as the number of +* sectors to be discarded. The specified sectors should be discarded if the +* underlying block device supports trim (ATA) or unmap (SCSI) operations, +* 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 */ -#define BLKIF_OP_TRIM 5 +#define BLKIF_OP_DISCARD 5/* * Maximum scatter/gather segments per request.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Re: [PATCH 1 of 1] interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD
Quoted 41 lines · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1314761891 14400
# Node IDd2538ec1df2b9f4c39978d96ebe3bccccb068ad0# Parent ac9aa65050e9abc8f1c12c8603acf3b99e22cddc
interface:BLKIF_OP_TRIM->BLKIF_OP_DISCARDThe 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.
CC Pasi Kärkkäinen <[email protected]>Signed-off-by Konrad Rzeszutek Wilk <[email protected]>diff -r
ac9aa65050e9-rd2538ec1df2bxen/include/public/io/blkif.hxen/include/public/io/blkif.h Tue Aug 30 23:38:11 2011 -0400 renamed from xen/include/public/io/blkif.h Tue Aug 30 11:46:58 2011 +0100 +4
-9@@ -82,25 +82,29 @@ */ #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 info. + * 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 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! - * - * Trim operation is a request for the underlying block device to mark - * extents to be erased. Trim operations are passed with sector_number as the
xen/include/public/io/blkif.h Tue Aug 30 23:38:11 2011 -0400 line 96
Quoted 10 lines
xen/include/public/io/blkif.h Tue Aug 30 23:38:11 2011 -0400 renamed from xen/include/public/io/blkif.h Tue Aug 30 11:46:58 2011 +0100 +6
-4- * 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_EOPNOTSUPP - * 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 mark + * extents to be erased. Discard operations are passed with sector_number as the
well I think we need to change the comments here, discard doesn't guarantee the blocks
will be erased from the device, it's just a hint to the device controller these blocks are no longer
used, and what to do with these blocks are left to the device controller, on an SSD the controller
might erase them and use the blank blocks to do some optimization like gc and better wear leveling.
and for the novell driver, we are free to change to BLKIF_OP_DISCARD, cause we have nothing depend
on BLKIF_OP_TRIM, and Jan should know better than me on this, correct me if am wrong, Thanks.
Quoted 16 lines
xen/include/public/io/blkif.h Tue Aug 30 23:38:11 2011 -0400 renamed from xen/include/public/io/blkif.h Tue Aug 30 11:46:58 2011 +0100 +8
-1+ * sector index to begin discard operations at and nr_sectors as the number of + * sectors to be discarded. The specified sectors should be discarded if the + * underlying block device supports trim (ATA) or unmap (SCSI) operations, + * 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 */ -#define BLKIF_OP_TRIM 5 +#define BLKIF_OP_DISCARD 5 /* * Maximum scatter/gather segments per request.
Re: Re: [PATCH 1 of 1] interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD
Quoted 6 lines
well I think we need to change the comments here, discard doesn't guarantee the blocks
will be erased from the device, it's just a hint to the device controller these blocks are no longer
used, and what to do with these blocks are left to the device controller, on an SSD the controller
How about this one below:
# HG changeset patch
# Parent ac9aa65050e9abc8f1c12c8603acf3b99e22cddc
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.
xen/include/public/io/blkif.h +22 -15
@@ -82,26 +82,33 @@*/ #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 info. +* 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 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! -* -* Trim operation is a request for the underlying block device to mark -* extents to be erased. Trim operations 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. The specified sectors should be trimmed if the -* underlying block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP -* 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 mark +* extents to be erased. However, discard does not guarantee that the blocks +* 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 number of +* sectors to be discarded. The specified sectors should be discarded if the +* underlying block device supports trim (ATA) or unmap (SCSI) operations, +* 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 */ -#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) <= PAGE_SIZE.
The windows frontends in current XenServer product don't have the trim code enabled since the backends don't really do anything useful with it yet so I have no objection to a name change if it really matters that much.
Paul
Quoted 30 lines
-----Original Message-----
From: Konrad Rzeszutek Wilk [mailto:[email protected]]
Sent: 31 August 2011 04:41
To: [email protected]; Ian Jackson; Stefano Stabellini; Ian Campbell; [email protected]; Owen Smith; Paul Durrant; [email protected]; [email protected]
Cc: [email protected]
Subject: [PATCH 0 of 1] Patch to alterBLKIF_OP_TRIMtoBLKIF_OP_DISCARD(v1).Hey guys,
Pasi mentioned on Li's (and Owen's) patches which provide TRIM/UNMAP support to the Linux backend/frontend that:
"
Isn't the generic name for this functionality "discard" in Linux?and "trim" being the ATA specific discard-implementation, and "scsi unmap" the SAS/SCSI specific discard-implementation?
Just wondering..
"and it sounds right to me. The problem is of course that the 'feature-trim'
is already in the interface - but I was wondering how much it is set in stone? Could it be feasible to modify the backend/frontend in Citrix product to use 'feature-discard' instead? And naturally also the Novell drivers.