rbf + startup issues - General Discussion and Assistance - CHDK Forum
« previous next »
Pages: [1] 2   Go Down

rbf + startup issues

  • 12 Replies
  • 8065 Views
*

Offline reyalp

  • ******
  • 14125
rbf + startup issues
« on: 03 / April / 2011, 17:17:32 »
Advertisements
This is related to http://chdk.setepontos.com/index.php?topic=650.msg63829#msg63829
and http://chdk.setepontos.com/index.php?topic=6179.0

I don't really want to continue the discussion in the patch thread and part of it is specific to the rbf code, so I'm starting a new thread...

While looking at this, I noticed that rbf_font.c uses read() with cacheable memory. This is unsafe and known to cause crashes and data corruption on some cameras. See http://chdk.wikia.com/wiki/CHDK_Coding_Guidelines#Memory_allocation (this use in rbf was also noted before http://chdk.setepontos.com/index.php?topic=6078.0 )

So I switched to use fread, and now fopen crashes in mounter.c. The cause isn't really clear, but this made me realize that the startup crash workaround only helps if CHDK calls open() directly, so switching to fopen will make the problem more likely.

Switching the font stuff to umalloc would not be great, I'd expect it to slow down OSD drawing a lot. I guess we could fiddle with the pointers later. Alternately, maybe our read/write wrappers should automatically fiddle with the pointers and do the appropriate CP15 calls to ensure the cashes and buffers are flushed.

Backing out the fread change, I noticed that my d10 seems get the FsIoNotify 451 assert if I start it with the USB cable connected. I think this is just due to timing and not an actual bug in the change.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: rbf + startup issues
« Reply #1 on: 03 / April / 2011, 18:42:18 »
Could this also be an issue for the configuration load & save as well.
If I recall correctly conf.c uses read & write on the conf data which will be in cached memory.
(Don't have the source with me now so I can't check it.)

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14125
Re: rbf + startup issues
« Reply #2 on: 03 / April / 2011, 18:54:00 »
Quote from: philmoz on 03 / April / 2011, 18:42:18
Could this also be an issue for the configuration load & save as well.
If I recall correctly conf.c uses read & write on the conf data which will be in cached memory.
(Don't have the source with me now so I can't check it.)

Phil.
I'm pretty sure I killed those back when we first figured this issue out. The only read and write I see are conf_restore and conf_save, which both use umalloc.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: rbf + startup issues
« Reply #3 on: 03 / April / 2011, 18:55:04 »
A thought - would a reasonable fix for this be to simply or the 'uncached' memory bit into the addresses passed into the 'read' and 'write' wrappers (inside the wrappers themselves)?

This way read and write work on uncached memory regardless of what the calling code is using.

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14125
Re: rbf + startup issues
« Reply #4 on: 03 / April / 2011, 19:02:31 »
Quote from: philmoz on 03 / April / 2011, 18:55:04
A thought - would a reasonable fix for this be to simply or the 'uncached' memory bit into the addresses passed into the 'read' and 'write' wrappers (inside the wrappers themselves)?

This way read and write work on uncached memory regardless of what the calling code is using.

Phil.
Unless you use the appropriate CP15 instructions to make sure everything is coherent, this doesn't solve the problem, e.g.
Code: [Select]
read(fd,&foo,sizeof(foo));
if(foo == 1)
...
if foo happened to be cached, the code wouldn't see the value from read. Similarly
Code: [Select]
foo = 1
write(fd,&foo,sizeof(foo));
will might not write 1. We had stuff like this happen using cached addresses.

Note this should theoretically apply to the raw stuff, but in that case the size of the raw buffer is so much bigger than the cache it all appears to get flushed out naturally.
« Last Edit: 03 / April / 2011, 22:23:19 by reyalp »
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: rbf + startup issues
« Reply #5 on: 03 / April / 2011, 23:00:01 »
Isn't the difference between the malloc and umalloc addresses just the uncached bit (as far as the calling program is concerned)?

If so why can't we use umalloc for allocating the font data (cTable data) and reading the font; but use the cTable address with the uncached bit removed for rendering the fonts? Access to the cTable data is encapsulated into a single function in the recent patch so this should be quite easy - we could even store two pointers (one cached, one uncached) for best performance.

This also raises the question of whether we need a version of umalloc/ufree for exmem (otherwise all the umalloc memory will be in the Canon heap instead of the exmem heap).

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14125
Re: rbf + startup issues
« Reply #6 on: 03 / April / 2011, 23:01:15 »
Figured out my problem with fopen: the conf code can pass an empty filename which makes dryos fopen assert rather than nicely returning NULL.

With the non fopen code, the FsIoNotify.c Line 451 assert went away with more pictures on the card... ???
Logged
Don't forget what the H stands for.
*

Offline reyalp

  • ******
  • 14125
Re: rbf + startup issues
« Reply #7 on: 03 / April / 2011, 23:07:50 »
Quote from: philmoz on 03 / April / 2011, 23:00:01
Isn't the difference between the malloc and umalloc addresses just the uncached bit (as far as the calling program is concerned)?
Yes, but as I said, you need to worry about coherency if you use both addresses. If read() loads directly into uncached memory, and then your code dereferences the cached address, you get whatever is in cache, not whats in RAM (edit: assuming there was something in cache for that address, of course.) There's no snooping.

If you use umalloc for the entire loading process, and then use the cached address later, that would probably be OK, just because a lot stuff happens between loading the font and actually trying to render characters.
Quote
This also raises the question of whether we need a version of umalloc/ufree for exmem (otherwise all the umalloc memory will be in the Canon heap instead of the exmem heap).
We don't use a whole lot of umalloc, and we tend not to hold onto it very long. I wouldn't worry at this point.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: rbf + startup issues
« Reply #8 on: 03 / April / 2011, 23:22:50 »
Quote from: reyalp on 03 / April / 2011, 23:07:50
If you use umalloc for the entire loading process, and then use the cached address later, that would probably be OK, just because a lot stuff happens between loading the font and actually trying to render characters.
It seems to work today for most people (with malloc) so I'd say the time between loading the font data and using it is long enough to avoid any issue.

The current rbf_font code only does one malloc and one read so changing this to umalloc; but using the cached address for access to the font data can't make it any worse :)

Quote
We don't use a whole lot of umalloc, and we tend not to hold onto it very long. I wouldn't worry at this point.

The font data is kept while CHDK is running - perhaps if exmem is enabled we can use umalloc and read then get another buffer using malloc (in exmem) and copy the data. The umalloc buffer can then be released.

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14125
Re: rbf + startup issues
« Reply #9 on: 03 / April / 2011, 23:44:24 »
Quote from: philmoz on 03 / April / 2011, 23:22:50
It seems to work today for most people (with malloc) so I'd say the time between loading the font data and using it is long enough to avoid any issue.
We do actually have some weird crashes in that area. They might all be the file handle thing but I'm not sure. Using read on a cached address is not necessarily the same as using an uncached one and twiddling the cache bit later.
Quote
The current rbf_font code only does one malloc and one read so changing this to umalloc; but using the cached address for access to the font data can't make it any worse :)
Agreed.

edit:
Actually the rbf code does a bunch of reads into struct font f... which is also cached.
Quote
The font data is kept while CHDK is running - perhaps if exmem is enabled we can use umalloc and read then get another buffer using malloc (in exmem) and copy the data. The umalloc buffer can then be released.
I wouldn't bother, compared to having all of CHDKs heap there it should be pretty minor. Especially if you put CHDK in exmem. If you want to spend time on it, a generic solution would seem a lot more worthwhile than special casing this one thing. Either an exmem umalloc/ufree, or some kind of safe read/write. Just my $0.02.
« Last Edit: 03 / April / 2011, 23:54:30 by reyalp »
Logged
Don't forget what the H stands for.
Pages: [1] 2   Go Up
« previous next »
 

Related Topics


SimplePortal © 2008-2014, SimplePortal