Skip to content

Add Dhara FTL Support to LittleFS#7184

Open
singh-aditya-04 wants to merge 9 commits intoSamsung:masterfrom
singh-aditya-04:littlefs_with_dhara
Open

Add Dhara FTL Support to LittleFS#7184
singh-aditya-04 wants to merge 9 commits intoSamsung:masterfrom
singh-aditya-04:littlefs_with_dhara

Conversation

@singh-aditya-04
Copy link
Contributor

1) Integrates Dhara FTL with LittleFS filesystem
2) Fix autoformat and memory corruption
3) Fix nand erase to continue if bad block
4) Reduces internal fragmentation of Littlefs with dhara
5) Adds metadata cache for improved read speeds

Add support to littlefs VFS layer to choose the driver based on
INODE. For MTD driver select the MTD defined functions otherwise
use block device.
Register dhara as block driver on top of MTD device.
This block device is used to mount littlfs on /mnt.
Dhara documentation shows that in case of successful read 0 should
be returned. But current TizeRT code returns numnber of read.
These changes modifies the format_filesystem function to add support for
formatting LittleFS over Dhara (MTD-based filesystem)
in addition to the existing block device formatting capabilities.
autoformat not working in case mount fails.
memory corruption due to wrong assignment for dhara.
causing corruption of dhara structure.
in case nand erase encounter bad block, return eagain and continue erasing other blocks.
Also fix return for nand_bread
Two changes are done for this to sync the behaviour between Dhara and littleFS
1) We change littlefs_blocksize from fs->geo.erasesize (128 KB) to fs->geo.blocksize(2 KB).
2) As we are reserving some sectors (pages) for dhara inside files
os/fs/driver/mtd/dhara/journal.c and os/fs/driver/mtd/dhara/map.c so
we are going to give total pages - reserved pages as available blocks
to Littlefs.

Signed-off-by: Aditya Singh <aditya.s4@samsung.com>
Signed-off-by: Aashish Lakhwara <aashish.l@samsung.com>
We have varified dhara itself manages bad block checking
so we do not need mtd to manage bad block checking as
it increases flash read operations.

Signed-off-by: Aditya Singh <aditya.s4@samsung.com>
Signed-off-by: Aashish Lakhwara <aashish.l@samsung.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of extern, you should define constant of ioctl, and use driver->u.i_bops->ioctl()

Copy link
Contributor

Choose a reason for hiding this comment

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

This need to be checked.
fs->geo.erasesize * fs->geo.neraseblocks is same as size of entire of flash (allocated for littlefs) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

prev is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

So erase size of MTD driver will be asigned.
But ftl will use blocksize of erase (Not erase internally). I mean lfs doesn't care flash is nor or nand. so if nand is attached, then blocksize will be used as a erasesize.
If I am right then geometry of ftl (include dhara) need to be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

if my comment is correct (https://github.com/Samsung/TizenRT/pull/7184/changes#r2910443698)
then this can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the PR please check

Copy link
Contributor

Choose a reason for hiding this comment

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

please put tab to each of code line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated Please check

Copy link
Contributor

Choose a reason for hiding this comment

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

what '64' means exactly? can we change constant value to some kind of formula?
(dev->geo->neraseblocks / dev->geo->blocksize I expect)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

64 represents pages in one bock , sure i will change constant value to some kind of formula

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated Please Check

Copy link
Contributor

Choose a reason for hiding this comment

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

if (page % 16 == 15)
But let's do not use '16' directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated please check

Copy link
Contributor

Choose a reason for hiding this comment

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

You reused this code from origin code of dhara. and I think this is implemented wrongly.
Because MTD_BREAD can be failed because of other issue, for example SPI.(of course w25n returns ERROR when status is ECC Fail. so I think we should put some TODO Comment here. I think kind of below logic is required

if (ret < 0) {
	dhara_insert_metadatacache(dev, mcache);
	if (check_ecc_check_logic) {
		dhara_set_error(err, DHARA_E_ECC);
	}
	return ret;
}

A Metadata Cache based on Most Recently Used Metadata Page
has been implemented inside file dhara.c that can be useful
for faster page reads.

Signed-off-by: Aditya Singh <aditya.s4@samsung.com>
Signed-off-by: Aashish Lakhwara <aashish.l@samsung.com>w
Copy link
Contributor

Choose a reason for hiding this comment

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

coding rule,

if (((p + 1) % page_per_chunk) == 0) {

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