-
Notifications
You must be signed in to change notification settings - Fork 9
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
Tune + fix /usr/sbin/mount-sd.sh #19
base: master
Are you sure you want to change the base?
Conversation
We trigger indexing anyway on line 63, so no need to delay the mounting and annoy users.
This tells the tracker to re-index, so it knows about the removed files. Also it removes the temporary mount directory as it's no longer needed (in fact line 63 checks that the directory hasn't been deleted, so this is just right).
This is needed for the next commit to be golden.
No need to trigger indexing on line 36: At that time there won't be any new files to track.
Please submit one PR per topic. |
The reason it is done like this is not annoy users. Tracker has removable volume support watching based on Glib2 and /proc/mounts and /dev/mmcblk1 is added to the watched volumes (https://github.com/mer-packages/glib2/blob/master/rpm/0001-Add-dev-mmcblk-to-the-list-of-devices-to-be-detected.patch). When a removable volume is removed (but the mount point has to stay around) the filesystem tree is marked as not available and when it is reinserted the tree is marked as available and quickly scanned for changes. This is much faster than a full reindex. As for the TRIM option, it would be nice to handle it in a separate pull request with links to documentation on how it helps (benchmarks etc.. if possible) |
@foolab The reason I did it bundled was that for example V10lator@e361899 depends on V10lator@2811de0 - so individual requests wouldn't work here. @iamer I know it's not done to annoy users, that was a joke. :) I see the theory behind fast reindexing but with the current implementation it's broken none the less: https://github.com/nemomobile/sd-utils/blob/master/scripts/mount-sd.sh#L49 triggers a re-index on the empty directory no matter if it was mounted before or not, so https://github.com/nemomobile/sd-utils/blob/master/scripts/mount-sd.sh#L63 will do a full reindex every time. Please correct me if my understanding is wrong. If needed I'll split that. Can't promise that the pull requests will stay auto-mergable that way through. As for benchmarks and other documentation: I don't have any. The reason behind enabling it is that it's already enabled for vfat and exfat. So I thought enabling it for more FSes is the next thing and benchmarks etc. have already been done on your end. |
Lines https://github.com/nemomobile/sd-utils/blob/master/scripts/mount-sd.sh#L28 to 32 will bail out if the device is already mounted. So the rest of the add block will only be reached if the volume was actually really just mounted :) On my Fedora man mount says for ext4: for btrfs: I am not so sure about turning it on right now. |
We talked about re-mounting (pulling card out, then back in) so it must be unmounted at line 28? Or is there some magic I don't understand that doesn't unmount a card when you pull it out of the device (but when does it get unmounted then?) ? discard works the same on all filesystems: When a file gets deleted (or any other action frees space) TRIM commands are send for the freed blocks, so the underlying FTL (flash translation layer) also knows these blocks are free and is able to optimize wear-leveling. Now again: It's already enabled for vfat and exfat. If the used FTL has performance issues with TRIM this should never have happened in the first place, so why exactly is vftat/exfat allowed to TRIM but other FSes are not? //EDIT: But I agree with the mount documentation: A COW FS like btrfs will trigger TRIM issues at the FTL way more often than "traditional" FSes like vfat/ext4. |
As far as I know mounting a filesystem with the trim mount option (so that it issues trim commands in real time) is usually discouraged as it can incur significant performance penalties. It is usually advised to rather add a cron job that periodically runs fstrim on the ssd & thin-LV mount points. This results in the trims being batched, which should improve performance (the trim operation should be shorter). For more information, see: Also - do SD cards actually support trim ? Results from a quick Google search seem to indicate that they don't. |
While officially not supported there's community support: https://openrepos.net/content/v10lator/f2fs Also exfat and ntfs aren't officially supported but handled here, so I can't see anything wrong with adding f2fs.
suggests that TRIM is supported. I wanted to confirm (see http://lightrush.ndoytchev.com/random-1/checkiftrimonext4isenabledandworking ) but hdparm --read-sector fails:
So the only one who's able to confirm is Jolla Oy I guess? For performance: Again: This hardly depends on the FTL. On my SSDs I'm using queued TRIM without any performance impacts, for example. On the Jolla Smartphone trimming 36.8 GB (done with "fstrim -v /media/sdcard/d742de61-e9be-4618-ad04-40e7e66d7b40/") took around a second, so I don't think you'll notice any impact. //EDIT: I googled and readed docs and... It seems you're right: SD cards do not support TRIM. None the less you should use it on them cause the Linux kernel is smart enough to translate the TRIM (SCSI command) commands to ERASE (SDHCI command) commands for SD cards. ;) //EDIT²: See also https://en.wikipedia.org/wiki/Trim_%28computing%29#SD.2FMMC |
@V10lator Just an (off-topic to this pull request) note about queued trim - some drives claim to support it but will actually trash your data if you use it, for example: |
@M4rtinK Thanks for the heads up but I know about that. My drive is safe (thanks to a firmware update, that's me: https://bugzilla.kernel.org/show_bug.cgi?id=71371#c34 ). :) |
To detect and fix corruption.
Almost 100 downloads at https://openrepos.net/content/v10lator/bleeding-edge-sd-utils with zero bug reports so far. |
# ext and btrfs are both able to handly TRIM. Add more to the list if needed. | ||
ext4|btrfs|f2fs) | ||
MOUNT_OPTS+=",discard" | ||
;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identation is a bit off here.
Other than the indentation to me this looks ok. |
And personally I would leave the trim still out as there is change that it can cause issues eventhough it shouldn't but there is change and dataloss is always bad even if the likelyhood is very low. |
@saukko I think then it should be disabled for fat, too, as it makes no sense to enable discard only for a single FS. Also I'm using this with f2fs and it works like a charm even with heavy sd usage (I have a lot of symlinks / bind mounts to reduce internal flash usage as much as possible). |
See the commits for details.