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

File creation freezes #717

Open
UfukAKKAYA opened this issue Aug 2, 2022 · 11 comments
Open

File creation freezes #717

UfukAKKAYA opened this issue Aug 2, 2022 · 11 comments
Labels
needs investigation no idea what is wrong

Comments

@UfukAKKAYA
Copy link

Hi dear littlefs developers.

I am creating a file in a folder. I am writing data to this file and I am able to close the file successfully. But my code sometimes freezes on "lfs_dir_orphaningcommit" function while creating the file.


uint8_t  lfs_read_buf[256];
uint8_t  lfs_prog_buf[256];

// configuration of the filesystem is provided by this struct
const struct lfs_config cfg = {
    // block device operations
    .read  = user_provided_block_device_read,
    .prog  = user_provided_block_device_prog,
    .erase = user_provided_block_device_erase,
    .sync  = user_provided_block_device_sync,

    // block device configuration
    .read_size = 256,
    .prog_size = 256,
    .block_size = 4096,
    .block_count = 8192,
    .cache_size = 256,
    .lookahead_size = 256,
    .block_cycles = 500,

   .read_buffer = lfs_read_buf,
   .write_buffer = lfs_prog_buf,

};
int fileCreate(char *fileName, uint8_t *data, int len)
{

	int err = lfs_file_open(&lfs, &file, "../123456789/file1234.f", LFS_O_RDWR | LFS_O_CREAT);
	if(err)	
		return err;
		
	err = lfs_file_rewind(&lfs, &file);	
	if(err)	
		return err;
	
	err = lfs_file_write(&lfs, &file, data, len);
	if(err)	
		return err;
		
	return lfs_file_close(&lfs, &file);
}

File Create


fileCreate("../123456789/file1234.f", writeBuf, strlen(writeBuf));

Code in infinite loop


static int lfs_dir_orphaningcommit(lfs_t *lfs, lfs_mdir_t *dir,
        const struct lfs_mattr *attrs, int attrcount) {
    // check for any inline files that aren't RAM backed and
    // forcefully evict them, needed for filesystem consistency
    for (lfs_file_t *f = (lfs_file_t*)lfs->mlist; f; f = f->next) {
        if (dir != &f->m && lfs_pair_cmp(f->m.pair, dir->pair) == 0 &&
                f->type == LFS_TYPE_REG && (f->flags & LFS_F_INLINE) &&
                f->ctz.size > lfs->cfg->cache_size) {
            int err = lfs_file_outline(lfs, f);
            if (err) {
                return err;
            }

            err = lfs_file_flush(lfs, f);
            if (err) {
                return err;
            }
        }
    }
    .
    . 
    .

}

I will be glad if you help.

@UfukAKKAYA
Copy link
Author

Debug View

Capture

@thuan1091996
Copy link

I'm having the same issue

@geky geky added the needs investigation no idea what is wrong label Sep 7, 2022
@midiwidi
Copy link

midiwidi commented Dec 2, 2022

I'm having the same problem.

static int lfs_dir_orphaningcommit(lfs_t *lfs, lfs_mdir_t *dir,
        const struct lfs_mattr *attrs, int attrcount) {
    // check for any inline files that aren't RAM backed and
    // forcefully evict them, needed for filesystem consistency
    for (lfs_file_t *f = (lfs_file_t*)lfs->mlist; f; f = f->next)

f->next points back to f and therefore we are stuck in an endless loop.

@thuan1091996
Copy link

I'm having the same problem.

static int lfs_dir_orphaningcommit(lfs_t *lfs, lfs_mdir_t *dir,
        const struct lfs_mattr *attrs, int attrcount) {
    // check for any inline files that aren't RAM backed and
    // forcefully evict them, needed for filesystem consistency
    for (lfs_file_t *f = (lfs_file_t*)lfs->mlist; f; f = f->next)

f->next points back to f and therefore we are stuck in an endless loop.

I spotted that when opening the same file multiple times, then this issue came in

@midiwidi
Copy link

midiwidi commented Dec 2, 2022

I came back here to report exactly that. Debugging the problem today revealed that I had synced the file but forgot to close it. When I opened it again I got "Assertion at ../littlefs/lfs.c:5478"

image

and on the next sync I ran into the problem with the endless loop described above.

@demetriopitasi
Copy link

We are too experiencing issues with lfs_dir_orphaningcommit too.

We're still analyzing the issue, but from a preliminary analysis it seems to be possible to call the lfs_pair_cmp within lfs_dir_orphaningcommit with f->m.pair set to NULL resulting in an hard fault.

static int lfs_dir_orphaningcommit(lfs_t *lfs, lfs_mdir_t *dir,
        const struct lfs_mattr *attrs, int attrcount) {
    // check for any inline files that aren't RAM backed and
    // forcefully evict them, needed for filesystem consistency
    for (lfs_file_t *f = (lfs_file_t*)lfs->mlist; f; f = f->next) {
        if (dir != &f->m && lfs_pair_cmp(f->m.pair, dir->pair) == 0 &&

#define LFS_VERSION 0x00020005
#define LFS_VERSION_MAJOR (0xffff & (LFS_VERSION >> 16))
#define LFS_VERSION_MINOR (0xffff & (LFS_VERSION >> 0))

We're currently testing the following workaround

static inline int lfs_pair_cmp(
        const lfs_block_t paira[2],
        const lfs_block_t pairb[2]) {

    if(paira==NULL||pairb==NULL)
        return 0;

    return !(paira[0] == pairb[0] || paira[1] == pairb[1] ||
             paira[0] == pairb[1] || paira[1] == pairb[0]);
}

#ifndef LFS_READONLY
static inline bool lfs_pair_sync(
        const lfs_block_t paira[2],
        const lfs_block_t pairb[2]) {

    if(paira==NULL||pairb==NULL)
        return 0;

    return (paira[0] == pairb[0] && paira[1] == pairb[1]) ||
           (paira[0] == pairb[1] && paira[1] == pairb[0]);
}
#endif

@geky
Copy link
Member

geky commented Jan 16, 2024

Hi all, thanks for opening/appending this issue.

This is a symptom of unbalanced open/close calls. Though it should be caught earlier than lfs_dir_orphaningcommit if you have asserts enabled.

littlefs uses an invasive linked-list to keep track of open files. Anytime open is called twice on the same file, littlefs tries to enroll the file in the linked-list twice, but this doesn't work since there's only one next pointer. Things break, and usually the linked-list ends up pointing to itself, creating an infinite loop.

The key thing is that every successful open call needs to be followed by a close call. This is to keep the linked-list up to date, but also for freeing any malloced resources.

One tricky thing is that this is a requirement even if a later file operation fails. So the example posted by @UfukAKKAYA should probably be written like the following:

int fileCreate(char *fileName, uint8_t *data, int len)
{
    int err = lfs_file_open(&lfs, &file, "../123456789/file1234.f", LFS_O_RDWR | LFS_O_CREAT);
    if(err)    
        return err;
        
    err = lfs_file_rewind(&lfs, &file);    
    if(err)    
        goto failed;
    
    err = lfs_file_write(&lfs, &file, data, len);
    if(err)    
        goto failed;
        
    return lfs_file_close(&lfs, &file);

failed:;
    lfs_file_close(&lfs, &file);
    return err;
}

Though it's unclear if this is actually @UfukAKKAYA's issue, since an unrelated error is required for the original code to break.

When an error occurs during a file operation a flag is set internally so that lfs_file_close will not touch disk and will not error.


I've created a PR to tighten the bounds of the open-list assert, in case any functions are missing this assert: #921, though this still requires asserts to be enabled to do anything.


EDIT: Corrected error handling when lfs_file_open itself errors. If lfs_file_open errors, the file is not "open" and should not be closed.

@geky
Copy link
Member

geky commented Jan 16, 2024

I just realized open-list related issues could also be caused by moving the lfs_file_t struct.

A side-effect of the invasive linked-list is that the lfs_file_t struct can't be moved. Moving the struct causes the linked-list to point to now-invalid memory.

I don't think this will often lead to an infinite loop, but it could cause other problems.

I'm not sure there's a way to check for this in an assert...

@Alvipe
Copy link

Alvipe commented Feb 1, 2024

Hi @geky. First of all, I want to thank you for your amazing work. Coming from SPIFFS, which gets corrupted so easily in the event of sudden power losses, the robustness of LittleFS is truly a blessing.

I am not sure about what you mention that every open call must be followed by a close call, even in case of error. I've tested this, trying to open a non-existent file (which returned a -2 error core, as expected) and then closing it. This has caused the following assert in line 6080 of lfs.c (in lfs_file_close) to fail (my code has the LFS_ASSERT macro defined):

LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));

I have tested this with version 2.9.

@geky
Copy link
Member

geky commented Feb 2, 2024

Oh, good catch! I made a mistake.

I am not sure about what you mention that every open call must be followed by a close call, even in case of error.

This should have been phrased as "Every successful open call must be followed by a close call, even if a later file operation errors."

Given that an open call can fail to even allocate resources (LFS_ERR_NOMEM), this is necessary behavior for the API.

Coming from SPIFFS

To be fair, it's a hard problem, and every layer needs to be power-loss aware, both up into the application and down into the bd driver.

@geky
Copy link
Member

geky commented Feb 2, 2024

I've edited the above comment so it should now be correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation no idea what is wrong
Projects
None yet
Development

No branches or pull requests

6 participants