[JFFS2] GC patch for eCos port
Estelle HAMMACHE
estelle.hammache at st.com
Mon Oct 11 04:49:52 EDT 2004
Hi Mark,
isn't it cleaner (for compatibility with Linux) to modify
jffs2_gc_fetch_page so that it reads a
PAGE_CACHE_SIZE-aligned buffer ?
BTW I'd like to make the whole page caching thing optional for
a specific use (application needs to write structures to a
file atomically & structure size is _not_ a power of 2 so it
may overlap a page boundary)... I think eCos doesn't care
about page caches ?
Estelle
Mark Hamilton wrote:
>
> There looks to be a bug with the eCos port on how garbage collection is
> done. I sent a notice about this bug before but I didn't get a resolution.
> Another fellow came across the same bug and verified my fix. The bug seems
> specific to eCos because of how jffs2_gc_fetch_page was ported but the
> proposed fix is applied to gc.c. Since the gc.c is a JFFS2 core file, I
> guess this is the appropriate mailing list for posting the patch. The patch
> is attached.
>
> Here is the problem.
> ffs2_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));
>
> The patch uses a define(__ECOS) to fix this problem.
More information about the linux-mtd
mailing list