Improve the discard documentation. Also improve discard code in

pv_manip.c to properly account for case when pe_start=0 and the first
physical extent is to be released (currently skip the first extent to
avoid discarding the PV label).

diff --git a/WHATS_NEW b/WHATS_NEW
index 952c984f5dbe8e2dae534e5e9d28b495ce766c4c..e23b7d805d7ec41ae86e76d10753179840453e83 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,7 +1,7 @@
 Version 2.02.85 - 
 ===================================
   Add "devices/issue_discards" to lvm.conf.
-  Issue discards on lvremove if enabled and both storage and kernel have support.
+  Issue discards on lvremove, lvreduce, etc if enabled and supported.
   Fix incorrect tests for dm_snprintf() failure.
   Fix some unmatching sign comparation gcc warnings in the code.
   Allow lv_extend() to work on zero length intrinsically layered LVs.

diff --git a/doc/example.conf.in b/doc/example.conf.in
index cf31855322c5e8ee745bbab9fce2460eeacb7309..dfc2c70178fb41c7e0b672b29bd1aa159638b943 100644 (file)
--- a/doc/example.conf.in
+++ b/doc/example.conf.in
@@ -152,11 +152,15 @@ devices {
     # pv_min_size = 2048
     pv_min_size = 512
 
-    # Issue discards to an LV's underlying PV(s) when the LV is removed.
-    # Discards inform the storage that a region is no longer in use.  If set
+    # Issue discards to a logical volumes's underlying physical volume(s) when
+    # the logical volume is no longer using the physical volumes' space (e.g.
+    # lvremove, lvreduce, etc).  Discards inform the storage that a region is
+    # no longer in use.  Storage that supports discards advertise the protocol
+    # specific way discards should be issued by the kernel (TRIM, UNMAP, or
+    # WRITE SAME with UNMAP bit set).  Not all storage will support or benefit
+    # from discards but SSDs and thinly provisioned LUNs generally do.  If set
     # to 1, discards will only be issued if both the storage and kernel provide
-    # support.  Not all storage will support or benefit from discards but SSDs
-    # or thinly provisioned LUNs generally do.
+    # support.
     # 1 enables; 0 disables.
     issue_discards = 0
 }

diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index 8637d6df865134dbc0069cf8fa31a73cc67188d7..35ac02cc368b46ccfd7772e7540ce525d54ad74f 100644 (file)
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -191,9 +191,9 @@ struct pv_segment *assign_peg_to_lvseg(struct physical_volume *pv,
 
 int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction)
 {
-       uint64_t discard_offset;
+       uint64_t discard_offset_sectors;
        uint64_t pe_start = peg->pv->pe_start;
-       uint32_t discard_area_reduction = area_reduction;
+       uint64_t discard_area_reduction = area_reduction;
 
        if (!peg->lvseg) {
                log_error("release_pv_segment with unallocated segment: "
@@ -209,16 +209,20 @@ int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction)
                                  "devices/issue_discards", DEFAULT_ISSUE_DISCARDS) &&
            dev_discard_max_bytes(peg->pv->fmt->cmd->sysfs_dir, peg->pv->dev) &&
            dev_discard_granularity(peg->pv->fmt->cmd->sysfs_dir, peg->pv->dev)) {
-               if (!pe_start) {
-                       /* skip the first extent */
-                       pe_start = peg->pv->vg->extent_size;
+               discard_offset_sectors = (peg->pe + peg->lvseg->area_len - area_reduction) *
+                       peg->pv->vg->extent_size + pe_start;
+               if (!discard_offset_sectors) {
+                       /*
+                        * pe_start=0 and the PV's first extent contains the label.
+                        * Must skip past the first extent.
+                        */
+                       discard_offset_sectors = peg->pv->vg->extent_size;
                        discard_area_reduction--;
                }
-               discard_offset = peg->pe + peg->lvseg->area_len - area_reduction;
-               discard_offset = (discard_offset * peg->pv->vg->extent_size) + pe_start;
-               log_debug("Discarding %" PRIu32 " extents offset %" PRIu64 " sectors on %s.",
-                        discard_area_reduction, discard_offset, dev_name(peg->pv->dev));
-               if (!dev_discard_blocks(peg->pv->dev, discard_offset << SECTOR_SHIFT,
+               log_debug("Discarding %" PRIu64 " extents offset %" PRIu64 " sectors on %s.",
+                         discard_area_reduction, discard_offset_sectors, dev_name(peg->pv->dev));
+               if (discard_area_reduction &&
+                   !dev_discard_blocks(peg->pv->dev, discard_offset_sectors << SECTOR_SHIFT,
                                        discard_area_reduction * peg->pv->vg->extent_size * SECTOR_SIZE))
                        return_0;
        }

diff --git a/man/lvm.conf.5.in b/man/lvm.conf.5.in
index 0b0accfce1f5beec45e271beb9a42b37223e76be..661983517a7169f9f3b5872be6facef8dd816c4d 100644 (file)
--- a/man/lvm.conf.5.in
+++ b/man/lvm.conf.5.in
@@ -181,11 +181,14 @@ ignore devices smaller than 2MB (i.e. floppy drives):
 pv_min_size = 2048
 .IP
 \fBissue_discards\fP \(em
-Issue discards to an LV's underlying PV(s) when the LV is removed.  Discards
-inform the storage that a region is no longer in use.  If set to 1, discards will
-only be issued if both the storage and kernel provide support.  Not all storage
-will support or benefit from discards but SSDs or thinly provisioned LUNs
-generally do.
+Issue discards to a logical volumes's underlying physical volume(s) when the
+logical volume is no longer using the physical volumes' space (e.g. lvremove,
+lvreduce, etc).  Discards inform the storage that a region is no longer in use.
+Storage that supports discards advertise the protocol specific way discards
+should be issued by the kernel (TRIM, UNMAP, or WRITE SAME with UNMAP bit set).
+Not all storage will support or benefit from discards but SSDs and thinly
+provisioned LUNs generally do.  If set to 1, discards will only be issued if
+both the storage and kernel provide support.  
 .IP
 .TP
 \fBallocation\fP \(em Space allocation policies