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

proposed fix for cryfs filesystem issue #1566

Closed
wants to merge 4 commits into from

Conversation

Michael-Gallo
Copy link
Contributor

For issue #1543

Works on my machine for getting rid of the high cpu usage.

I can not seem to find a way to detect a cryfs file system that doesn't rely on syscall

@Michael-Gallo Michael-Gallo reopened this Jan 4, 2024
Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Even though the root cause comes from cryfs and this is merely a workaround, I think it might be OK to merge this kind of patch, since the changes are relatively small. I am not sure what other scenarios result in a block count of 0 - if it does cause other issues we will have to revert this change, but hopefully that shouldn't be the case.

One other thing is that even with this workaround, because mtime is now ignored I believe there's no way for lf to automatically reload the directory if it changes (e.g. a new file is created), so the user will have to manually call reload.

joelim-work
joelim-work previously approved these changes Jan 6, 2024
Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for the patch, I will give my approval.

@joelim-work
Copy link
Collaborator

I have bad news, it looks like tmpfs filesystems (e.g. /tmp on my machine) also have a block count of 0. Disabling nav.checkDir in this case will mean that it's impossible for lf to update the directory contents from load, period timer, etc., and the only way I can think of is to perform a full reload manually, which is not ideal.

Unless there's some other way to distinguish cryfs mounts, I don't think this kind of patch can be merged, since it will fix this bug but end up causing other issues.

@joelim-work joelim-work dismissed their stale review January 7, 2024 12:53

Found issue with tmpfs filesystems

@Michael-Gallo Michael-Gallo deleted the cryfs-fix branch February 8, 2024 01:38
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.

2 participants