Re: [JFFS2] GC patch for eCos port

[Estelle HAMMACHE <estelle.hammache@st.com>]

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.