Don't bypass cache in lfs_cache_prog() and lfs_cache_read()

[FreddieChopin]

In some cases specific alignment of buffer passed to underlying device
is required. For example SDMMC in STM32F7 (when used with DMA) requires
the buffers to be aligned to 16 bytes. If you enable data cache in
STM32F7, the alignment of buffer passed to any driver which uses DMA
should generally be at least 32 bytes.

While it is possible to provide sufficiently aligned "read", "prog" and
per-file caches to littlefs, the cases where caches are bypassed are
hard to control when littlefs is hidden under some additional layers.
For example if you couple littlefs with stdio and use it via FILE*,
then littlefs functions will operate on internal FIlE* buffer, usually
allocated dynamically, so in these specific cases - with insufficient
alignment (8 bytes on ARM Cortex-M).

The easy path was taken - remove all cases of cache bypassing.

Fixes #158

[FreddieChopin]

Another approach is possible, but it is considerably harder, uses more code and RAM.

lfs_config could be extended with additional field to specify required alignment. If these value is greater than __BIGGEST_ALIGNMENT__ (in case of gcc this is the alignment of memory provided by malloc()) additional code would have to be executed to manually align dynamically allocated buffers. This would also require extra fields for storing the unaligned pointers (to pass it to free()). User provided buffers should be checked for proper alignment with assertions. With all this in place, the condition for bypassing cache could be extended with test for sufficient alignment.

Please let me know if you would prefer this path - I will implement an alternative pull request in that case.

Important note - similar fix will be required for v2 branch too.

[FreddieChopin]

I've tried to understand why the assertion in lfs_cache_prog() fails now for read size == prog size == 1, but I really have no idea, maybe the test scenario needs some adjustments now or maybe there is some underlying problem which was hidden before or maybe my change is completely wrong... Any pointers appreciated.

[geky]

Hmm, lets rebase and see. Sorry if this messes with your branch.

[geky]

Well, I guess the problem went away. Unrelated question, do you think this sort of change should be on a minor release?

[FreddieChopin]

Unrelated question, do you think this sort of change should be on a minor release?

What do you mean exactly? Are you're asking whether you should do a new minor release right after this is merged? Or maybe you cannot decide between a minor and patch release? Or something else? (; As long as this gets released sooner than later I don't have any strong preferences (;

[geky]

Or maybe you cannot decide between a minor and patch release? Or something else? (; As long as this gets released sooner than later I don't have any strong preferences (;

Right now, every PR that lands on master is treated as either a patch, minor or major release. Minor releases are used to indicate new features. This PR is a bit ambiguous, but lets go with a patch release for now.

[geky]

Thanks for the pr!

[FreddieChopin]

Right now, every PR that lands on master is treated as either a patch, minor or major release.

Please take a look at how the description of the patch releases are generated. I assume this PR went into https://github.com/ARMmbed/littlefs/releases/tag/v2.0.3 , but there is actually no mention about it, just the most recent commit which deals with travis-ci related issues.

[geky]

So I played around with it a bit, but unfortunately as soon as you add text to tags in GitHub, they become big release notices which makes it hard to find the releases for the major/minor releases. IMO these are more important to find as they contain human written release notes.

They also contain a list of all commits since the last minor release, so when we make a minor release this commit will be found there.

[geky]

@FreddieChopin, after uses this process a bit longer, I see your point. It is very difficult to find which version a commit went into. I'm changing this here: #261

[FreddieChopin]

BTW after some more time I'm ready to admit that bypassing the cache should be reverted. It is actually not really a viable solution to expect every external library to deal with alignment or other low-level stuff like that.

Please revert yourself or I can prepare a pull-request - whatever is more convenient.

[geky]

That sounds like a plan. If you're able to create a PR that would help, otherwise I can revert it next minor release.

There's a number of performance issues that people have identified around the metadata fetching. The cache bypassing may help a little bit, but I think it still warrants more investigation.
#203 (comment)

Sorry I still haven't been able to get to the performance issues at all.

I'm planning on taking care of porting documentation next, but hopefully after that everything will be more or less settled and I'll finally be able to look into all of the performance issues.