Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable discard mount option by default in /etc/fstab #270

Closed
wants to merge 1 commit into from

Conversation

adrelanos
Copy link
Contributor

This is for easier trim support in VMs to release the free space on the host operating system after deleting files inside the VM image.

@adrelanos
Copy link
Contributor Author

Do you know if this causes any issues for any of the file systems? I am only using ext4.

 ext*|exfat|fat|jfs|nilfs2|vfat)

Ideally file systems not supporting it would simply ignore it.

Otherwise this would need a different implementation doing this only where known to be functional, for ext4 with others happy to send patches to opt this in for other file systems.

@mika
Copy link
Member

mika commented Jan 29, 2024

AFAICS all those file systems seem to support discard, so I'd give this a try.

@zeha @jkirk what do you think?

@zeha
Copy link
Member

zeha commented Jan 29, 2024

I'm generally ok with enabling trim by default, but:

  1. IIRC adding discard to mount options is somewhat deprecated and instead regular fstrim is suggested and iirc enabled by default nowadays
  2. it shouldn't be done in the block that is responsible for errors=remount-ro,
  3. xfs should also get it, if done at all

@jkirk
Copy link
Contributor

jkirk commented Jan 29, 2024

I am not so sure about changing the default mount options.

ext4(5) states:

   discard/nodiscard
          Controls whether ext4 should issue discard/TRIM commands to the underlying block device when blocks are freed. 
          This is useful for SSD devices and sparse/thinly-provisioned LUNs,
          but it is off by default until sufficient testing has been done.

xfs(5) states:

   discard|nodiscard
          Enable/disable the issuing of commands to let the block
          device reclaim space freed by the filesystem.  This is
          useful for SSD devices, thinly provisioned LUNs and
          virtual machine images, but may have a performance impact.

          Note: It is currently recommended that you use the fstrim
          application to discard unused blocks rather than the
          discard mount option because the performance impact of
          this option is quite severe.  For this reason, nodiscard
          is the default.

I could not find a man page for mount.exfat.
mount.exfat(8) from exfat-fuse does not mention discard and mount(8) does not mention exfat at all.

mount(8) does not mention discard for jfs.

mount.nilfs2(8) mentions discard, but AFAICS nilfs-tools is needed for mkfs.nilfs2; shouldn't it be installed via install_fs_tools()?

And finally fstrim(8)

Running fstrim frequently, or even using mount -o discard, might
negatively affect the lifetime of poor-quality SSD devices. For
most desktop and server systems a sufficient trimming frequency
is once a week. Note that not all devices support a queued trim,
so each trim command incurs a performance penalty on whatever
else might be trying to use the disk at the time.

If at all, I'd vote to make discard optional.

@mika
Copy link
Member

mika commented Jan 29, 2024

Re filesystems: I looked into the linux source itself, the manpages don't seem to be fully up2dated, AFAICS.

But as @zeha wrote:

IIRC adding discard to mount options is somewhat deprecated and instead regular fstrim is suggested and iirc enabled by default nowadays

And as stated by @jkirk:

If at all, I'd vote to make discard optional.

I agree, I don't think that's a sane default we should enable by default. And if anyone wants that, this should be taken care of through a custom hook script or alike.

ACK?

@adrelanos
Copy link
Contributor Author

Yes. Thank you for your consideration!

Closing.

@adrelanos adrelanos closed this Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants