d4rken-org / sdmaid

SD Maid is an Android app that helps you manage files and apps.
https://play.google.com/store/apps/details?id=eu.thedarken.sdm
1.51k stars 745 forks source link

Wrong allocated file size (invalid blocks*blocksize calculation) #830

Closed rishabhkohli closed 7 years ago

rishabhkohli commented 7 years ago

The Storage Analyzer reports the file size as about 1/8th (bit-byte conversion issue?) the actual size. I couldn't find it having been reported before. Is this limited to me? I can provide screenshots, if required.

d4rken commented 7 years ago

Can you show me some samples?

SD Maid uses the size on disk value and formats it via Androids shorthand file size format method.

rishabhkohli commented 7 years ago

screenshot_20170407-181534 screenshot_20170407-181543 screenshot_20170407-181550 screenshot_20170407-181529

d4rken commented 7 years ago

Is this on SD Maid v4.6.5?

What sizes does SD Maids explorer show?

rishabhkohli commented 7 years ago

4.6.4. But I have noticed this bug for a while now. Just never got around to reporting it. I will report again after the update.

The explorer showed something quite interesting. screenshot_20170407-182823 What are the two different sizes?

rishabhkohli commented 7 years ago

Just checked. No update available after 4.6.4.

d4rken commented 7 years ago

v4.6.5 is beta atm, but it doesn't matter in this case.

The first size is allocated size in the filesystem (size on storage) (e.g. how many blocks). The second size is the file size as reported by the file itself.

They usually differ when a file is a sparse file: https://en.wikipedia.org/wiki/Sparse_file

(I need to explain this in the wiki, thx for reminding me)

I think there is an issue with determining the size on disk. I checked the code and SD Maid assumes a blocksize of 512 for the blockcount value we get from stat. I would suspect that the blockcount value from stat is too low.

Seeing as the factor really is roughly 8, it's likely giving the blockcount for a blocksize of 4096Byte instead of 512Byte.

Why though... What binary is SD Maid using? Check in SD Maids overview.

rishabhkohli commented 7 years ago

screenshot_20170407-185538

That sounds interesting. What is stat, though?

d4rken commented 7 years ago

Hm using SD Maids own toybox should be ok... stat is an applet that SD Maid uses (in this case from the file you can see in your screenshot) to access file stats, see: https://linux.die.net/man/2/stat

Did you make any fancy file system changes on your device?

Btw updated the wiki: https://github.com/d4rken/sdmaid-public/wiki/Explorer#file-size

Can you run these command from a shell, file doesn't really matter, just something on your internal sdcard.

/data/data/eu.thedarken.sdm/files/toybox_sdm stat /storage/emulated/0/TitaniumBackup/com.ubercab-20170316-151754.tar.gz
stat /storage/emulated/0/TitaniumBackup/com.ubercab-20170316-151754.tar.gz

The first one uses SD Maids stat the second one the systems native stat applet. First command might need to run with root to work.

rishabhkohli commented 7 years ago

Ah. That stat. That just didn't click in my mind. I don't work with Linux enough these days, I guess. :P

screenshot_20170407-191626

I did change the filesystem to F2FS from ext4 a few months back. Could that be it?

d4rken commented 7 years ago

I did change the filesystem to F2FS from ext4 a few months back. Could that be it?

Not sure.

From your screenshot we can see that:

So 13563 4096 Byte = 55554048 Byte is a lot closer the the actual file size, than 13563 512 Byte = 6944256 Byte.

But the last time this came up (calculating allocated size), see here, it turned out that the blockcount from stat is always in 512 Byte chunks. Some more nice infos here.

When you browse with SD Maids exploret to /system , /cache or /data, is there also this discrepancy in allocated vs filesize for all files?

rishabhkohli commented 7 years ago

screenshot_20170407-202006 screenshot_20170407-201950 screenshot_20170407-201916

Cache and system are both ext4. Only data partition is F2FS.

I'll read up on your links.

d4rken commented 7 years ago

Can you install a busybox of your choice and run the stat applet from that (direct reference busybox stat) on one of the problematic files, i.e. the /storage/emulated/0/TitaniumBackup/com.ubercab-20170316-151754.tar.gz from previously?

rishabhkohli commented 7 years ago

screenshot_20170407-205904

d4rken commented 7 years ago

Currently running out of ideas. It all looks OK, except for the too small allocated size.

rishabhkohli commented 7 years ago

Yeah, me as well. I read up on the links. The filled some gaps in my knowledge. But no insight as to why it is wrong here. And especially why it isn't wrong for more people. I don't have another rooted device to play around with.

d4rken commented 7 years ago

So far it seems F2FS related, but I'm out of my depth here. I've posted to the toybox mailing list, asking for help.

http://lists.landley.net/pipermail/toybox-landley.net/2017-April/008919.html

rishabhkohli commented 7 years ago

How can it be F2FS related? The data partition showed the correct sizes in explorer. The last screenshot in my post with the partitions. The entire partition is F2FS.

d4rken commented 7 years ago

Right, good point. I've been staring at the screen for too long.

/data is F2FS, which includes /data/app which we have seen above on your screenshot, shows sane values.

/storage/emulated/0 is FUSE?

What sizes do you see when you browse the folder directly (not through FUSE), i.e /data/media/0/TitaniumBackup?

rishabhkohli commented 7 years ago

I always thought that the media partition was simply symlinked for legacy purposes. It was interesting to read about FUSE.

SD Maid reads the proper sizes through /data/media/0. As far as I understand it, FUSE is mounted by the kernel, right? So, could the kernel be providing bad values somewhere?

d4rken commented 7 years ago

AFAIK /data/media/0 holds the files with their original permissions and meta data and for legacy reasons this is then FUSE mounted to /storage/emulated/0 to provide the public storage that behaves similar to the mounted sdcards in earlier Android versions.

As far as I understand it, FUSE is mounted by the kernel, right? So, could the kernel be providing bad values somewhere?

Possibly, at least there are options like this that point to the possibility.

'blksize=N' Set the block size for the filesystem. The default is 512. This option is only valid for 'fuseblk' type mounts.

Looking deeper, it seems that the problematic blockcount value comes directly from the system see st_blocks here. It also mentions again that it's in units of 512B.

Seeing as the "wrong" value comes directly from the system call, I'd say this is neither a bug in SD Maid nor in toybox, but rather a system bug or config issue. Would you agree?

That would leave the question whether SD Maid can reasonably do something about this.

rishabhkohli commented 7 years ago

It would seem so. I will try an AOSP/LOS based ROM tomorrow or day after to see if it makes a difference. Will report back if it is successful.

As far as what SD Maid can do, I haven't read enough about storage to know if it is possible, but how feasible would it be to read the file size directly from the file?

d4rken commented 7 years ago

As far as what SD Maid can do, I haven't read enough about storage to know if it is possible, but how feasible would it be to read the file size directly from the file?

We already know the file size, see the other value displayed by the explorer.

With stuff like comparing sizes, deleting, apps sizes, we want to know how much space is occupied on storage, i.e. the allocated size or occupied size (terminology is confusing).

We could delete a 10GB file and only free 1MB (e.g. sparse file), if we then use and display 10GB, that would be pretty missleading, which is why SD Maid prefers allocated size over file size in most tools.

d4rken commented 7 years ago

Got a response on the mailing list. We were on the right track. The stat call returns the wrong value.

http://lists.landley.net/pipermail/toybox-landley.net/2017-April/008925.html

This was only fixed quite recently, see: https://android-review.googlesource.com/#/q/I1c9e16604ba580a8cdefa17f02dcc489d7351aed The best way to fix this would be to update your kernel.

I can't really do much from my end, but I could provide an option to change the storage analyzers sort mode from using "allocated size" to "file size"? Would that be worth it though? Is this a useful option? Seems after all this would just be a semi workaround for this bug here.

For further discussion of that: #831

rishabhkohli commented 7 years ago

I updated the kernel. To one that specifically mentioned having merged the sdcardfs fixes. https://kernels.franco-lnx.net/OnePlus3/7.1.1/appfiles/changelog.xml

But the file size was still wrong. I think it could be because toybox still has that hard coded 512. Could it?

I guess an option to choose the sorting mode would be good enough as a fix. It is not too critical of a bug anyway.

d4rken commented 7 years ago

I updated the kernel. To one that specifically mentioned having merged the sdcardfs fixes. https://kernels.franco-lnx.net/OnePlus3/7.1.1/appfiles/changelog.xml

Hm not sure why this isn't fixed in the newer kernel, a bit out of my depth... maybe you could ask the kernel maintainer?

I think it could be because toybox still has that hard coded 512. Could it?

No, a hardcoded 512 is ok, because the linux kernel docs specifically states that the blockcount value is always in units of 512.

rishabhkohli commented 7 years ago

Fixed the reported file sizes by disabling sdcardfs in build.prop. It is the replacement for FUSE in Android O. OnePlus and some other manufacturers have been using it since earlier.

d4rken commented 7 years ago

I guess that works too as solution 😁 Still curious why this isn't fixed by using the latest kernel, maybe there are ROM changes required too.

rishabhkohli commented 7 years ago

I talked to the kernel dev. He said he wasn't sure if he had merged that particular patch. I'm waiting for his next release.

rishabhkohli commented 7 years ago

Final update. A kernel with the relevant patches fixed the issue.

d4rken commented 7 years ago

Awesome :)

lbcaizyt commented 6 years ago

The same problem also appears on my mobile phone.

lbcaizyt commented 6 years ago

My phone is Honor 6X. The ROM is EMUI 5.0.1 based on Android 7.0.