This is the mail archive of the
ecos-discuss@sources.redhat.com
mailing list for the eCos project.
Re: JFFS2 fragment corruption during gc?
- From: "Mark Hamilton" <mark_lee_hamilton at sbcglobal dot net>
- To: "Alf Nilsson" <alf dot nilsson at abem dot se>,"eCos Disuss" <ecos-discuss at ecos dot sourceware dot org>
- Date: Tue, 5 Oct 2004 08:31:09 -0700
- Subject: Re: [ECOS] JFFS2 fragment corruption during gc?
- References: <4162B787.7040405@abem.se>
Alf,
I've seen this problem as well. I sent an email to the ecos-discussion
group a few weeks back asking if my fix made sense. From what I can tell,
there is a bug with the eCos port of the garbage collection. The eCos group
directed me to the JFFS mailing list. I sent my email there but never
received a response.
Below is my original email which includes my fix. I would appreciate it if
you could tell me if this solves your problem.
I'm using the JFFS file system and I'm having problems with files being
truncated during garbage collection. I believe I've tracked down the
problem. I'm hoping someone can give me positive feedback on my fix. The
problem is in this snippet of code below:
Jffs2_gc_fetch_page reads 4K of data into a static buffer. The static buffer
is hidden in the jffs2_gc_fetch_page function. The problem is when the
writebuf pointer is calculated. The offset is used again to reference into
the pg_ptr. You can image when start is equal to 4K that writebuf will
extend beyond the end of the pg_ptr valid memory. Offset is set to start
just before the while loop.
I made a comment below with what I think the fix should be.
Am I missing something?
pg_ptr = jffs2_gc_fetch_page(c, f, start, &pg);
if (IS_ERR(pg_ptr)) {
printk(KERN_WARNING "read_cache_page() returned error:
%ld\n",
PTR_ERR(pg_ptr));
return PTR_ERR(pg_ptr);
}
offset = start;
while(offset < orig_end) {
uint32_t datalen;
uint32_t cdatalen;
char comprtype = JFFS2_COMPR_NONE;
ret = jffs2_reserve_space_gc(c, sizeof(ri) +
JFFS2_MIN_DATA_LEN, &phys_ofs, &alloclen);
if (ret) {
printk(KERN_WARNING "jffs2_reserve_space_gc of
%zd bytes for
garbage_collect_dnode failed: %d\n",
sizeof(ri)+ JFFS2_MIN_DATA_LEN,
ret);
break;
}
cdatalen = min_t(uint32_t, alloclen - sizeof(ri),
end - offset);
datalen = end - offset;
// This looks to be wrong.
writebuf = pg_ptr + (offset & (PAGE_CACHE_SIZE -1));
// I think it should be.
writebuf = pg_ptr + ((offset -start) &
(PAGE_CACHE_SIZE -1));
----- Original Message -----
From: "Alf Nilsson" <alf.nilsson@abem.se>
To: "eCos Disuss" <ecos-discuss@ecos.sourceware.org>
Sent: Tuesday, October 05, 2004 8:02 AM
Subject: [ECOS] JFFS2 fragment corruption during gc?
> Hi all,
>
> We've noticed some odd behaviors from the JFFS2 file system and would
> like to know if anyone else has had the same type of problems.
> Our problem is that when we fill up the file system until it returns
> ENOSPC and then start to remove files, the remaining files seem to be
> corrupted.
> The files are the correct size, but it seems like some fragments are
> cleared and set to zero within the file.
> We've run tests with the D2 option enabled, but this generates a rather
> large amount of debug info, but I can make is accessible in case anyone
> is interested.
>
> I have attached a source code file which causes the error. Depending on
> which configuration that is used it takes a different amount of time
> before it fails.
> In my current repository the fileio2.c file resides in my
> fs/jffs2/current/tests directory, with a slightly altered cdl. In case
> It would be preferred to send this test as a patch instead let me know.
>
> To make the test run without failing lower the value of FILE_LIMIT, it
> seems like if the file system isn't filled up completely the corruption
> does not occur.
>
> We have noticed this behavior on 2 different flash circuits and on the
> synthetic target.
> Our setup has been without compression and CMODE set to NONE default.
>
> The test is based upon writing files that are 75% of the block size, and
> filling the file system completely at least 10 times.
>
> I've also attached my exported settings from the ecos.ecc file, in case
> anyone want to view which settings I've been using.
>
> Thanks,
> Alf Nilsson
>
>
----------------------------------------------------------------------------
----
> cdl_savefile_version 1;
> cdl_savefile_command cdl_savefile_version {};
> cdl_savefile_command cdl_savefile_command {};
> cdl_savefile_command cdl_configuration { description hardware template
package };
> cdl_savefile_command cdl_package { value_source user_value wizard_value
inferred_value };
> cdl_savefile_command cdl_component { value_source user_value wizard_value
inferred_value };
> cdl_savefile_command cdl_option { value_source user_value wizard_value
inferred_value };
> cdl_savefile_command cdl_interface { value_source user_value wizard_value
inferred_value };
>
> cdl_configuration eCos {
> description "" ;
> hardware linux ;
> template default ;
> package -hardware CYGPKG_HAL_SYNTH current ;
> package -hardware CYGPKG_HAL_SYNTH_I386 current ;
> package -hardware CYGPKG_DEVS_FLASH_SYNTH current ;
> package -hardware CYGPKG_DEVS_ETH_ECOSYNTH current ;
> package -hardware CYGPKG_DEVS_WATCHDOG_SYNTH current ;
> package -template CYGPKG_HAL current ;
> package -template CYGPKG_IO current ;
> package -template CYGPKG_IO_SERIAL current ;
> package -template CYGPKG_INFRA current ;
> package -template CYGPKG_KERNEL current ;
> package -template CYGPKG_MEMALLOC current ;
> package -template CYGPKG_ISOINFRA current ;
> package -template CYGPKG_LIBC current ;
> package -template CYGPKG_LIBC_I18N current ;
> package -template CYGPKG_LIBC_SETJMP current ;
> package -template CYGPKG_LIBC_SIGNALS current ;
> package -template CYGPKG_LIBC_STARTUP current ;
> package -template CYGPKG_LIBC_STDIO current ;
> package -template CYGPKG_LIBC_STDLIB current ;
> package -template CYGPKG_LIBC_STRING current ;
> package -template CYGPKG_LIBC_TIME current ;
> package -template CYGPKG_LIBM current ;
> package -template CYGPKG_IO_WALLCLOCK current ;
> package -template CYGPKG_ERROR current ;
> package CYGPKG_FS_JFFS2 current ;
> package CYGPKG_IO_FILEIO current ;
> package CYGPKG_IO_FLASH current ;
> package CYGPKG_LINUX_COMPAT current ;
> package CYGPKG_CRC current ;
> };
>
> cdl_option CYGDBG_KERNEL_DEBUG_GDB_THREAD_SUPPORT {
> inferred_value 0
> };
>
> cdl_option CYGBLD_ISO_CTYPE_HEADER {
> inferred_value 1 <cyg/libc/i18n/ctype.inl>
> };
>
> cdl_option CYGBLD_ISO_ERRNO_CODES_HEADER {
> inferred_value 1 <cyg/error/codes.h>
> };
>
> cdl_option CYGBLD_ISO_ERRNO_HEADER {
> inferred_value 1 <cyg/error/errno.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_FILETYPES_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_STREAMS_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_FILEOPS_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_FILEACCESS_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_FORMATTED_IO_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_CHAR_IO_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_DIRECT_IO_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_FILEPOS_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDIO_ERROR_HEADER {
> inferred_value 1 <cyg/libc/stdio/stdio.h>
> };
>
> cdl_option CYGBLD_ISO_STDLIB_STRCONV_HEADER {
> inferred_value 1 <cyg/libc/stdlib/atox.inl>
> };
>
> cdl_option CYGBLD_ISO_STDLIB_ABS_HEADER {
> inferred_value 1 <cyg/libc/stdlib/abs.inl>
> };
>
> cdl_option CYGBLD_ISO_STDLIB_DIV_HEADER {
> inferred_value 1 <cyg/libc/stdlib/div.inl>
> };
>
> cdl_option CYGBLD_ISO_STRERROR_HEADER {
> inferred_value 1 <cyg/error/strerror.h>
> };
>
> cdl_option CYGBLD_ISO_STRTOK_R_HEADER {
> inferred_value 1 <cyg/libc/string/string.h>
> };
>
> cdl_option CYGBLD_ISO_STRING_LOCALE_FUNCS_HEADER {
> inferred_value 1 <cyg/libc/string/string.h>
> };
>
> cdl_option CYGBLD_ISO_STRING_BSD_FUNCS_HEADER {
> inferred_value 1 <cyg/libc/string/bsdstring.h>
> };
>
> cdl_option CYGBLD_ISO_STRING_MEMFUNCS_HEADER {
> inferred_value 1 <cyg/libc/string/string.h>
> };
>
> cdl_option CYGBLD_ISO_STRING_STRFUNCS_HEADER {
> inferred_value 1 <cyg/libc/string/string.h>
> };
>
> cdl_option CYGBLD_ISO_C_TIME_TYPES_HEADER {
> inferred_value 1 <cyg/libc/time/time.h>
> };
>
> cdl_option CYGBLD_ISO_C_CLOCK_FUNCS_HEADER {
> inferred_value 1 <cyg/libc/time/time.h>
> };
>
> cdl_option CYGBLD_ISO_SIGNAL_NUMBERS_HEADER {
> inferred_value 1 <cyg/libc/signals/signal.h>
> };
>
> cdl_option CYGBLD_ISO_SIGNAL_IMPL_HEADER {
> inferred_value 1 <cyg/libc/signals/signal.h>
> };
>
> cdl_option CYGBLD_ISO_SETJMP_HEADER {
> inferred_value 1 <cyg/libc/setjmp/setjmp.h>
> };
>
> cdl_option CYGBLD_ISO_DIRENT_HEADER {
> inferred_value 1 <cyg/fileio/dirent.h>
> };
>
> cdl_option CYGBLD_ISO_OPEN_MAX_HEADER {
> inferred_value 1 <cyg/fileio/limits.h>
> };
>
> cdl_option CYGBLD_ISO_NAME_MAX_HEADER {
> inferred_value 1 <cyg/fileio/limits.h>
> };
>
> cdl_option CYGNUM_LIBC_MAIN_DEFAULT_STACK_SIZE {
> inferred_value 16384
> };
>
> cdl_option CYGOPT_FS_JFFS2_COMPRESS_ZLIB {
> inferred_value 0
> };
>
> cdl_option CYGOPT_FS_JFFS2_COMPRESS_CMODE {
> user_value NONE
> };
>
> cdl_component CYGPKG_IO_FILEIO_INODE {
> inferred_value 1
> };
>
> cdl_component CYGPKG_IO_FLASH_BLOCK_DEVICE {
> inferred_value 1
> };
>
> cdl_option CYGNUM_IO_FLASH_BLOCK_OFFSET_1 {
> user_value 0x00000000
> };
>
> cdl_option CYGNUM_IO_FLASH_BLOCK_LENGTH_1 {
> user_value 0x00080000
> };
>
> cdl_option CYGNUM_FLASH_SYNTH_BLOCKSIZE {
> user_value 8192
> };
>
> cdl_option CYGNUM_FLASH_SYNTH_NUMBLOCKS {
> user_value 1024
> };
>
>
>
----------------------------------------------------------------------------
----
> #include <pkgconf/hal.h>
> #include <pkgconf/kernel.h>
> #include <pkgconf/io_fileio.h>
> #include <pkgconf/io_flash.h>
>
> #include <cyg/kernel/ktypes.h> // base kernel types
> #include <cyg/infra/cyg_trac.h> // tracing macros
> #include <cyg/infra/cyg_ass.h> // assertion macros
> #include <cyg/io/flash.h>
> #include <cyg/io/io.h>
> #include <cyg/io/config_keys.h>
>
> #include <unistd.h>
> #include <fcntl.h>
> #include <sys/stat.h>
> #include <errno.h>
> #include <string.h>
> #include <dirent.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #include <cyg/fileio/fileio.h>
>
> #include <cyg/infra/testcase.h>
> #include <cyg/infra/diag.h> // HAL polled output
>
> #include <pkgconf/fs_jffs2.h> // Address of JFFS2
>
>
//==========================================================================
>
> #define FILE_LIMIT (200)
> #define ERROR 1
> #define SUCCESS 0
>
>
//==========================================================================
>
> #define SHOW_RESULT( _fn, _res ) \
> {snprintf( cyg_test_text, sizeof( cyg_test_text ), \
> #_fn "() returned %d %s", (int)_res, _res<0?strerror(errno):""); \
> CYG_TEST_INFO( cyg_test_text );}
>
>
>
//==========================================================================
>
> static const char *test_file_folder = "/test_files";
> static char *dest;
> static char *pattern_buffer;
> static int current_file_num = 1;
> static int oldest_file_num = 1;
> static int file_size;
> static char cyg_test_text[200];
> static int out_of_space = 0;
>
>
//==========================================================================
>
> static void create_test_pattern( char *buffer, size_t s );
> static void create_filename( char*buffer, size_t size, int i );
> static int verify_files( const char *path,size_t size );
> static void remove_file( int i );
> static int need_space( void );
>
>
//==========================================================================
>
> static void create_test_pattern( char *buffer, size_t s )
> {
> memset( buffer, '8', s );
> }
>
>
//==========================================================================
>
> static void create_filename( char*buffer, size_t size, int i )
> {
> snprintf( buffer, size, "%s/%i", test_file_folder, i );
> }
>
>
//==========================================================================
>
> static int verify_files( const char *path, size_t size)
> {
> DIR *dirp;
> int fd;
> struct dirent *entry;
> ssize_t read_size;
> int error = 0;
> char absolute_filename[50];
> int err;
>
> dirp = opendir( path );
> if( dirp )
> {
> entry = readdir( dirp );
> while( entry )
> {
> if( strcmp( entry->d_name, "." ) != 0 &&
> strcmp( entry->d_name, ".." ) != 0 )
> {
> create_filename( absolute_filename,
> sizeof( absolute_filename ),
> atoi( entry->d_name ));
>
> fd = open( absolute_filename, O_RDONLY );
> if( fd != -1 )
> {
> int wrong = 0;
> read_size = read( fd, dest, size );
> if( read_size != -1 )
> {
> int j;
> for(j=0;j<read_size;j++)
> {
> if( pattern_buffer[j] != dest[j] )
> wrong++;
> }
> if( wrong )
> error = 1;
> if( !( read_size == size || read_size==0 ) )
> {
> snprintf( cyg_test_text, sizeof( cyg_test_text ), "Bad size %s %d",
> entry->d_name, (int)read_size );
> CYG_TEST_INFO( cyg_test_text );
>
> }
> }
> else
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( read, read_size );
> error = 1;
> }
>
> err = close( fd );
> if( err == -1 )
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( close, err );
> }
> if( error )
> {
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "Compare of file : %s failed", entry->d_name );
> CYG_TEST_INFO( cyg_test_text );
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "Total %d of %d bytes differs", wrong, (int)read_size );
> CYG_TEST_INFO( cyg_test_text );
> break;
> }
> }
> else
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( open, fd );
> err = 1;
> break;
> }
> }
> entry = readdir( dirp );
> }
> err = closedir( dirp );
> if( err == -1 )
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( closedir, err );
> }
> }
>
> if( error )
> return ERROR;
>
> return SUCCESS;
> }
>
>
//==========================================================================
>
> static void remove_file( int i )
> {
> int err;
> char filenamebuff[50];
> create_filename(filenamebuff,
> sizeof(filenamebuff),
> i );
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "Remove: %s", filenamebuff );
> CYG_TEST_INFO( cyg_test_text );
> memset( cyg_test_text, 0, sizeof( cyg_test_text ) );
>
> err = unlink( filenamebuff );
> if( err == -1 )
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( unlink, err );
> }
>
> if( verify_files( test_file_folder, file_size ) != SUCCESS )
> CYG_TEST_FAIL_FINISH("File verify failed");
> }
>
>
//==========================================================================
>
> static int create_file( int i, const char *data, size_t count )
> {
> int fd;
> int err = 0;
> int err_check;
> char filename[50];
>
> create_filename( filename, sizeof( filename ), i );
>
> fd = open( filename, O_WRONLY | O_CREAT );
> if (fd==-1)
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( open, fd );
> if( verify_files( test_file_folder, count ) != SUCCESS )
> CYG_TEST_FAIL_FINISH("File verify failed");
> return -1;
> }
>
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "Created file %s", filename );
> CYG_TEST_INFO( cyg_test_text );
> memset( cyg_test_text, 0, sizeof( cyg_test_text ) );
>
> if( verify_files( test_file_folder, count ) != SUCCESS)
> CYG_TEST_FAIL_FINISH("File verify failed");
>
> if( fd != -1 )
> {
> err = write( fd, data, count );
> if( err != count )
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( write, err );
> err = -1;
> }
>
> err_check = close( fd );
> if( err_check == -1 )
> {
> if( errno == ENOSPC ) out_of_space = 1;
> SHOW_RESULT( close, err_check );
> }
>
> if( err != count )
> remove_file( i );
> }
>
> if( verify_files( test_file_folder, count ) != SUCCESS )
> CYG_TEST_FAIL_FINISH("File verify failed");
>
> return err;
> }
>
>
//==========================================================================
>
> static int need_space( void )
> {
> if( out_of_space )
> {
> out_of_space = 0;
> return true;
> }
> if( ( current_file_num - oldest_file_num ) >= FILE_LIMIT )
> return true;
>
> return false;
> }
>
>
//==========================================================================
> // main
>
> int main( int argc, char **argv )
> {
> int err;
> Cyg_ErrNo cyg_err;
> cyg_io_handle_t flash_handle;
> cyg_io_flash_getconfig_blocksize_t blocksize;
> size_t len;
> int max_num_of_files;
>
> CYG_TEST_INIT();
>
> // --------------------------------------------------------------
>
> cyg_err = cyg_io_lookup( CYGDAT_IO_FLASH_BLOCK_DEVICE_NAME_1,
> &flash_handle );
> if( cyg_err != ENOERR )
> {
> CYG_TEST_FAIL_FINISH( "Could not read flash device" );
> }
>
> len = sizeof(blocksize);
> memset( &blocksize, 0, len );
> cyg_err = cyg_io_get_config( flash_handle,
> CYG_IO_GET_CONFIG_FLASH_BLOCKSIZE, &blocksize, &len);
> if( cyg_err != ENOERR )
> {
> CYG_TEST_FAIL_FINISH("Could not read flash blocksize" );
> }
>
> file_size = ( blocksize.block_size / 4 ) * 3;
>
> max_num_of_files = ( CYGNUM_IO_FLASH_BLOCK_LENGTH_1 / file_size ) *
10;
>
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "File size = %d, Max files = %d", file_size, max_num_of_files );
> CYG_TEST_INFO( cyg_test_text );
>
> dest = malloc( file_size );
> pattern_buffer = malloc( file_size );
>
> // --------------------------------------------------------------
>
> err = mount( CYGDAT_IO_FLASH_BLOCK_DEVICE_NAME_1, "/", "jffs2" );
> if( err < 0 ) SHOW_RESULT( mount, err );
>
> err = chdir( "/" );
> if( err < 0 ) SHOW_RESULT( chdir, err );
>
> err = mkdir( test_file_folder, 0 );
> if( err < 0 ) SHOW_RESULT( mkdir, err );
>
> // --------------------------------------------------------------
>
> // Init buffer
> create_test_pattern( pattern_buffer, file_size );
>
> while( max_num_of_files-- )
> {
> int first_file_num = current_file_num++;
>
> while( create_file( first_file_num, pattern_buffer, file_size ) == -1 )
> {
> if( need_space() )
> remove_file( oldest_file_num++ );
> }
> if( need_space() )
> {
> remove_file( oldest_file_num++ );
> }
> }
>
> free( dest );
> free( pattern_buffer );
>
> if( max_num_of_files > 0 )
> {
> snprintf( cyg_test_text, sizeof( cyg_test_text ),
> "File count was not reached %d files left", max_num_of_files );
> CYG_TEST_FAIL_FINISH( cyg_test_text );
> }
>
> err = chdir( "/" );
> if( err < 0 ) SHOW_RESULT( chdir, err );
>
> err = umount( "/" );
> if( err < 0 ) SHOW_RESULT( umount, err );
>
> CYG_TEST_PASS_FINISH("fileio2");
> }
>
>
----------------------------------------------------------------------------
----
> --
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss