From ffcb1b9c2c9bed8a25fe91c4aee4e0ae56c55896 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Wed, 13 Apr 2011 18:26:39 +0000 Subject: [PATCH] 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). --- WHATS_NEW | 2 +- doc/example.conf.in | 12 ++++++++---- lib/metadata/pv_manip.c | 24 ++++++++++++++---------- man/lvm.conf.5.in | 13 ++++++++----- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 952c984f5..e23b7d805 100644 --- 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 cf3185532..dfc2c7017 100644 --- 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 8637d6df8..35ac02cc3 100644 --- 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 0b0accfce..661983517 100644 --- 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 -- 2.43.5