Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Laurent Bigonville]

Le 06/08/17 à 12:55, Petter Reinholdtsen a écrit :

[Laurent Bigonville]

The discards commands will also be issued when shrinking or moving a LV
to an other PV, if something is going wrong during these operations, the
data will be lost.

So it's not only when explicitly removing an LV.

Aha.  I did not have the imagination required to consider that such
commands would be issued before the operation was sccessfully completed.
It seemed to me quite obvious that the space would be released only
after the shrink or move was complete, but if that is not the case, it
seem quite unsafe to trust LVM to handle discard commands.

I got hit with a bug while migrating my data from my old non-ssd to ssd 
drive. In the end I had no data on the SSD (apparently LVM was issuing 
discards on the destination disk...) and the LV was deleted from the 
non-ssd one. Hopefully the data were still on the old disk (as it was 
not supporting DISCARD) so I was able to recover the LV.


While this bug is fixed now, with that kind of experience I can 
understand why upstream wants to be on the safe side here.

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Petter Reinholdtsen]

[Laurent Bigonville]
> The discards commands will also be issued when shrinking or moving a LV 
> to an other PV, if something is going wrong during these operations, the 
> data will be lost.
>
> So it's not only when explicitly removing an LV.

Aha.  I did not have the imagination required to consider that such
commands would be issued before the operation was sccessfully completed.
It seemed to me quite obvious that the space would be released only
after the shrink or move was complete, but if that is not the case, it
seem quite unsafe to trust LVM to handle discard commands.

--
Happy hacking
Petter Reinholdtsen

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Laurent Bigonville]

Le 06/08/17 à 10:50, Bastian Blank a écrit :

On Sun, Aug 06, 2017 at 09:16:55AM +0200, Petter Reinholdtsen wrote:

A while back, I created a deb named ssd-setup to automatically set up Debian for
a SSD disk.  Until I understand more about the LVM issue, I automatically
enable issue_discards=1 in
https://anonscm.debian.org/cgit/collab-maint/ssd-setup.git >.  But
is this a bad idea or simply useless?

In general it is a bad idea.

Let's take a look again at the current text in lvm.conf regarding this
option:
| Issue discards to PVs that are no longer used by an LV.  Discards are
| sent to an LV's underlying physical volumes when the LV is no longer
| using the physical volumes' space, e.g.  lvremove, lvreduce. Discards
| inform the storage that a region is no longer used.

So this only triggers if you do operations that change the LV itself.
It however does _not_ effect the use of discards during the normal
operation of a filesystem.

However it comes with a large drawback, not counting the bugs in the
discard handling itself:  Even with the automatic backups of the lvm
metadata, it is impossible to recover from the wrongly removed LV.  This
is the reason why this feature is off by default.


The discards commands will also be issued when shrinking or moving a LV 
to an other PV, if something is going wrong during these operations, the 
data will be lost.


So it's not only when explicitly removing an LV.

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Petter Reinholdtsen]

[Bastian Blank]
> In general it is a bad idea.

I belive this is not clear enough from the documentation and the
instructions on http://wiki.debian.org/SSDOptimization >.  But I
am unsure how to make it more clear. :/

I must admit that  describe my
expectations too.  There, Ralf Jung stated

  "You can get your data back after you explicitly told the system, as
  root, to delete it" is not a feature that anybody should expect, and
  it should only be considered if it can be provided without significant
  downsides.

I expect that the SSD releases the storage when I remove a LV, not that
I can get it back.

-- 
Happy hacking
Petter Reinholdtsen

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Bastian Blank]

On Sun, Aug 06, 2017 at 09:16:55AM +0200, Petter Reinholdtsen wrote:
> A while back, I created a deb named ssd-setup to automatically set up Debian 
> for
> a SSD disk.  Until I understand more about the LVM issue, I automatically
> enable issue_discards=1 in
> https://anonscm.debian.org/cgit/collab-maint/ssd-setup.git >.  But
> is this a bad idea or simply useless?

In general it is a bad idea.

Let's take a look again at the current text in lvm.conf regarding this
option:
| Issue discards to PVs that are no longer used by an LV.  Discards are
| sent to an LV's underlying physical volumes when the LV is no longer
| using the physical volumes' space, e.g.  lvremove, lvreduce. Discards
| inform the storage that a region is no longer used.

So this only triggers if you do operations that change the LV itself.
It however does _not_ effect the use of discards during the normal
operation of a filesystem.

However it comes with a large drawback, not counting the bugs in the
discard handling itself:  Even with the automatic backups of the lvm
metadata, it is impossible to recover from the wrongly removed LV.  This
is the reason why this feature is off by default.

Bastian

-- 
Fascinating is a word I use for the unexpected.
-- Spock, "The Squire of Gothos", stardate 2124.5

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Petter Reinholdtsen]

[Laurent Bigonville]
> What I understand from this bug and Zdenek (who is apparently LVM2 upstream)
> message is that by default the discard requests are already passed to the
> underlying device during regular operation, this was definitely not really
> clear in my head, maybe this should be explained in the comments from the
> configuration file.

For the record, people still believe issue_discards=1 is needed in
LVM for SSD disks.  I just noticed 
http://blog.liw.fi/posts/2017/08/05/enabling_trim_discard_on_debian_ext4_luks_and_lvm/
 >
suggesting to set it by default.  If it really is not needed nor wanted,
it should be documented somewhere in LVM close to where people setting in
SSD disks are looking.

A while back, I created a deb named ssd-setup to automatically set up Debian for
a SSD disk.  Until I understand more about the LVM issue, I automatically
enable issue_discards=1 in
https://anonscm.debian.org/cgit/collab-maint/ssd-setup.git >.  But
is this a bad idea or simply useless?

-- 
Happy hacking
Petter Reinholdtsen

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Laurent Bigonville]

On Fri, 19 Jul 2013 10:43:11 +0200 Petter Reinholdtsen  
wrote:

>
> Hi.

Hello,

> According to several recipes on how to set up Linux on a SSD disk, LVM
> need to be reconfigured to issue discard (aka TRIM) instructions to the
> underlying storage device. This is done by modifying /etc/lvm/lvm.conf
> and setting issue_discards = 1 in the devices section.
>
> Why not set the option automatically when the underlying device is not
> rotational? In my case, the LVM physical device is /dev/dm-0 (encrypted
> device), and /sys/block/dm-0/queue/rotational return 0. If LVM used the
> rotational value to decide to set the issue_discard option, there would
> be no need for manual configuration with LVM and SSD disks.
>
> Is there some reason this isn't done already, except lack of time to
> implement it?

What I understand from this bug and Zdenek (who is apparently LVM2 
upstream) message is that by default the discard requests are already 
passed to the underlying device during regular operation, this was 
definitely not really clear in my head, maybe this should be explained 
in the comments from the configuration file.


The "issue_discards=1" parameter is only needed if discards should also 
be issued when changing topology of the LV, ie when doing thin 
provisioning apparently.


So I don't think it's that important to enable "issue_discards" by 
default ; the automatic backups of the configuration files made by the 
lvm commands already saved my ass at least once, so...


my 2¢

Laurent Bigonville

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Zdenek Kabelac]

On Sun, 20 Dec 2015 12:50:55 -0800 Matt Taggart  wrote:

I agree that "issue_discards = 1" should become the default in lvm on
Debian.

There is a case where you might not want TRIM/discards, there are security
implications to enabling it with dm-crypt.  In the commit message which
added this support, the author states:

  Note that discard will be never enabled by default because
  of security consequences, it is up to administrator
  to enable it for encrypted devices.




Please  CLOSE this BZ as not BUG - it's really a feature.

Using  'issue_discards=1'  makes any lvremove operation 'irreversible' on
SSD -  so using 'vgcfgretore'  cannot then fix the mistake of admin when
he removes LV and he would like to get it back - since restoring TRIMED space 
is just pointless.


So 'default' being 0 is carefully chosen!

If admin does know what he is doing he may switch it to 1 - but that's mostly
usable only if the provided storage is kind of 'provisioned' one.

On typical OS - if you release LV space in your VG and your next command 
allocates this space for a new LV - then this LV is supposed to be

discarded!.

The initial BZ comment is misleading - as any new LV user - which typically is 
the filesystem - runs  blkdiscard on such LV (as might be checked by looking 
at 'strace' of e.g. mkfs.ext4)


Also there is no shortage of SSD performance nor SSD lifespan as the firmware 
in SSD is smarter and does the load balancing across whole device (so its 
relocating blocks that are used too often...)


Regards

Zdenek

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Ralf Jung]

Hi,

> Using  'issue_discards=1'  makes any lvremove operation 'irreversible' on
> SSD -  so using 'vgcfgretore'  cannot then fix the mistake of admin when
> he removes LV and he would like to get it back - since restoring TRIMED
> space is just pointless.
> 
> So 'default' being 0 is carefully chosen!

"You can get your data back after you explicitly told the system, as
root, to delete it" is not a feature that anybody should expect, and it
should only be considered if it can be provided without significant
downsides.

Kind regards,
Ralf

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Matt Taggart]

I agree that "issue_discards = 1" should become the default in lvm on 
Debian.

There is a case where you might not want TRIM/discards, there are security 
implications to enabling it with dm-crypt.  In the commit message which 
added this support, the author states:

  Note that discard will be never enabled by default because
  of security consequences, it is up to administrator
  to enable it for encrypted devices.

and in the added documentation:

  WARNING: allowing discard on encrypted device has serious irreversible
  security consequences, discarded blocks can be easily located on device
  later. This can lead to leak of information from ciphertext device
  (unique pattern for detecting filesystem type, used space etc).

https://www.redhat.com/archives/dm-devel/2011-July/msg00042.html

But if I understand it correctly, as long as dm-crypt defaults to not 
enabling it, it would still be fine if LVM did have it enabled, you just 
wouldn't have it all the way down the stack to the hardware and would lose 
the benefit but still be safe. It might be worth documenting that, 
something like:

  The option "issue_discards" is enabled by default on LVM in Debian.
  Note that in order to get the benefits of discard, it needs to be
  enabled in each block layer down to the actual hardware. Due to
  security implications, discard is disabled by default in dm-crypt,
  so if you are using that you should first consult the dm-crypt
  documentation to understand the implications before enabling.

Sound ok?

-- 
Matt Taggart
tagg...@debian.org

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Petter Reinholdtsen]

[Martin Pitt 2013-12-13]
> The documentation says that the option has no effect if the kernel or
> the drive don't support it, so I don't see why we shouldn't just
> enable this by default?

Right.  Then perhaps the Debian maintainers of lvm2 can change the
default, to make Debian better for us with SSD disks by default?

This issue received no comment from the lvm2 maintainers for more than
two years, and I wonder what can be done about it?  Anyone got any idea
about the best approach?

> I attach the Ubuntu debdiff for reference, although it's fairly
> trivial.

Right, so the change has been tested in Ubuntu for two years.  Time to
merge into Debian?

-- 
Happy hacking
Petter Reinholdtsen

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Martin Pitt]

tag 717313 patch
thanks

Petter Reinholdtsen [2013-07-19 10:43 +0200]:
 According to several recipes on how to set up Linux on a SSD disk, LVM
 need to be reconfigured to issue discard (aka TRIM) instructions to the
 underlying storage device.  This is done by modifying /etc/lvm/lvm.conf
 and setting issue_discards = 1 in the devices section.
 
 Why not set the option automatically when the underlying device is not
 rotational?

The documentation says that the option has no effect if the kernel or
the drive don't support it, so I don't see why we shouldn't just
enable this by default?

I attach the Ubuntu debdiff for reference, although it's fairly
trivial.

Thanks,

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
diff -Nru lvm2-2.02.98/debian/changelog lvm2-2.02.98/debian/changelog
--- lvm2-2.02.98/debian/changelog   2013-09-16 11:48:04.0 +0200
+++ lvm2-2.02.98/debian/changelog   2013-12-13 10:52:23.0 +0100
@@ -1,3 +1,13 @@
+lvm2 (2.02.98-6ubuntu2) trusty; urgency=medium
+
+  * Add enable-issue-discards.patch: Enable issue_discards option by default
+to trim SSDs when changing PVs. This option has no effect if the kernel or
+the drive does not support trimming, so it's safe to enable by default.
+[https://blueprints.launchpad.net/ubuntu/+spec/core-1311-ssd-trimming]
+(Closes: #717313)
+
+ -- Martin Pitt martin.p...@ubuntu.com  Fri, 13 Dec 2013 10:51:54 +0100
+
 lvm2 (2.02.98-6ubuntu1) saucy; urgency=low
 
   * Merge from Debian unstable, remaining changes:
diff -Nru lvm2-2.02.98/debian/patches/enable-issue-discards.patch 
lvm2-2.02.98/debian/patches/enable-issue-discards.patch
--- lvm2-2.02.98/debian/patches/enable-issue-discards.patch 1970-01-01 
01:00:00.0 +0100
+++ lvm2-2.02.98/debian/patches/enable-issue-discards.patch 2013-12-13 
10:51:47.0 +0100
@@ -0,0 +1,20 @@
+Description: Enable issue_discards option by default
+ to trim SSDs when changing PVs. This option has no effect if the kernel or the
+ drive does not support trimming, so it's safe to enable by default.
+ See also https://blueprints.launchpad.net/ubuntu/+spec/core-1311-ssd-trimming
+Author: Martin Pitt martin.p...@ubuntu.com
+Bug-Debian: http://bugs.debian.org/717313
+
+Index: lvm2-2.02.98/doc/example.conf.in
+===
+--- lvm2-2.02.98.orig/doc/example.conf.in  2013-12-13 10:43:03.0 
+0100
 lvm2-2.02.98/doc/example.conf.in   2013-12-13 10:49:09.471664140 +0100
+@@ -196,7 +196,7 @@
+ # to 1, discards will only be issued if both the storage and kernel 
provide
+ # support.
+ # 1 enables; 0 disables.
+-issue_discards = 0
++issue_discards = 1
+ }
+ 
+ # This section allows you to configure the way in which LVM selects
diff -Nru lvm2-2.02.98/debian/patches/series lvm2-2.02.98/debian/patches/series
--- lvm2-2.02.98/debian/patches/series  2013-09-16 11:22:10.0 +0200
+++ lvm2-2.02.98/debian/patches/series  2013-12-13 10:48:28.0 +0100
@@ -8,3 +8,4 @@
 dm-event-api.patch
 monitoring-default-off.patch
 missing-dmeventd.patch
+enable-issue-discards.patch


signature.asc
Description: Digital signature

Bug#717313: lvm2: Enable issue_discards = 1 automatically on non-rotational (SSD) disks?

[Petter Reinholdtsen]

Package: lvm2
Version: 2.02.95-7
Severity: wishlist

Hi.

According to several recipes on how to set up Linux on a SSD disk, LVM
need to be reconfigured to issue discard (aka TRIM) instructions to the
underlying storage device.  This is done by modifying /etc/lvm/lvm.conf
and setting issue_discards = 1 in the devices section.

Why not set the option automatically when the underlying device is not
rotational?  In my case, the LVM physical device is /dev/dm-0 (encrypted
device), and /sys/block/dm-0/queue/rotational return 0.  If LVM used the
rotational value to decide to set the issue_discard option, there would
be no need for manual configuration with LVM and SSD disks.

Is there some reason this isn't done already, except lack of time to
implement it?

-- 
Happy hacking
Petter Reinholdtsen


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org