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.

Sign up for GitHub

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

Jump to bottom

enable fstrim on SSDs #116

Merged
merged 1 commit into from Jul 24, 2019
Merged

enable fstrim on SSDs #116

merged 1 commit into from Jul 24, 2019

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Jul 23, 2019

This will run TRIM once a week using a systemd timer. Running TRIM regularly
will improves the performance and increases the SSDs lifespan. Since it is based
on the utillinux no additional package is required.

@Mic92
enable fstrim on SSDs
e1a10b1 
This will run TRIM once a week using a systemd timer. Running TRIM regularly
will improves the performance and increases the SSDs lifespan. Since it is based
on the utillinux no additional package is required.
@Mic92 Mic92 requested a review from lukateras July 23, 2019 08:07
@zarelit
Copy link
Member

zarelit commented Jul 23, 2019

Does this work with encrypted drives? IIRC TRIM needs allow-discards to be enabled for it to work with LUKS devices

@Mic92
Copy link
Member Author

Mic92 commented Jul 24, 2019

If it should be enabled for cryptsetup is up to the user in my opinion. It might have some security implications.

@lukateras
Copy link
Member

This does cancel out any potential for deniable encryption, but this certainly is not something that can be applied to a / LUKS partition. As-is, fstrim in this PR will be a non-op on LUKS partitions. To fix that, user will have to open their partition with cryptsetup --allow-discards --persistent open /dev/sdaX root once.

I guess we could alternatively set rd.luks.options=discard in boot.kernelParams, but that would result in enabling TRIM for all LUKS partitions, including external ones AFAIK, which are much more likely to benefit from deniable encryption (in particular, I think DeLUKS might be affected).

lukateras
lukateras approved these changes Jul 24, 2019
View reviewed changes
Copy link
Member

@lukateras lukateras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks so much for adding this in :-)

@lukateras lukateras merged commit 34f24f2 into master Jul 24, 2019
@lukateras lukateras deleted the fstrim branch July 24, 2019 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukateras lukateras lukateras approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mic92 @zarelit @lukateras