Camera crash on startup - investigation. - page 11 - General Discussion and Assistance - CHDK Forum  
« previous next »
Pages: 1 ... 6 7 8 9 10 [11] 12   Go Down

Camera crash on startup - investigation.

  • 112 Replies
  • 48104 Views
*

Offline srsa_4c

  • ******
  • 4451
Re: Camera crash on startup - investigation.
« Reply #100 on: 20 / March / 2014, 20:09:08 »
Advertisements
Quote from: philmoz on 20 / March / 2014, 19:47:04
Does _TakeSemaphore wait for the semaphore to be available (which would solve our problem), or does it fail
It's the TryTakeSemaphore variant that fails immediately if the semaphore is unavailable. Takesemaphore has a timeout parameter (which can be 0, IMHO meaning "wait indefinitely").

Quote
I can add TakeSemaphoreStrictly to the stubs easily enough - do you think it would it be better to call this and risk a DebugAssert?
The above should answer this question (no need).

Quote from: philmoz on 20 / March / 2014, 19:50:31
IMO the TakeSemaphore approach is probably the best long term solution.
I don't have a problem with this approach. Does that particular semaphore already exist at the time CHDK opens the first file?
Logged
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Camera crash on startup - investigation.
« Reply #101 on: 20 / March / 2014, 21:20:06 »
Quote from: srsa_4c on 20 / March / 2014, 20:09:08
Quote from: philmoz on 20 / March / 2014, 19:47:04
Does _TakeSemaphore wait for the semaphore to be available (which would solve our problem), or does it fail
It's the TryTakeSemaphore variant that fails immediately if the semaphore is unavailable. Takesemaphore has a timeout parameter (which can be 0, IMHO meaning "wait indefinitely").

Thanks, it should solve the problem nicely then.

Edit: It looks like using 0 as the timeout (2nd) parameter to TakeSemaphore means it doesn't wait - is this your understanding? Any thoughts on what value to use?

Quote
Quote from: philmoz on 20 / March / 2014, 19:50:31
IMO the TakeSemaphore approach is probably the best long term solution.
I don't have a problem with this approach. Does that particular semaphore already exist at the time CHDK opens the first file?

On the G12 and G1X it's initialised in the first function called from initFileModulesTask - so should be safe to use.

Phil.
« Last Edit: 20 / March / 2014, 22:27:42 by philmoz »
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

  • ******
  • 14080
Re: Camera crash on startup - investigation.
« Reply #102 on: 20 / March / 2014, 22:49:30 »
Quote from: philmoz on 20 / March / 2014, 21:20:06
Edit: It looks like using 0 as the timeout (2nd) parameter to TakeSemaphore means it doesn't wait - is this your understanding? Any thoughts on what value to use?
My impression (like srsa's) was that 0 means forever, but that was just based on how it's used in the code. I would say wait forever (or a substantial time) is what we want. We make the file functions fail on a timeout or failure to take, but I don't see any reason it should assert.

The canon filewrite code (and most of the other stuff I looked at) uses 0, so if that means no wait, taking it may not help us.

The vxworks docs for semTake say -1 is forever and 0 is no wait, but I'm not sure this is the same as what we call TakeSemaphore.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Camera crash on startup - investigation.
« Reply #103 on: 20 / March / 2014, 23:13:56 »
Quote from: reyalp on 20 / March / 2014, 22:49:30
Quote from: philmoz on 20 / March / 2014, 21:20:06
Edit: It looks like using 0 as the timeout (2nd) parameter to TakeSemaphore means it doesn't wait - is this your understanding? Any thoughts on what value to use?
My impression (like srsa's) was that 0 means forever, but that was just based on how it's used in the code. I would say wait forever (or a substantial time) is what we want. We make the file functions fail on a timeout or failure to take, but I don't see any reason it should assert.

The canon filewrite code (and most of the other stuff I looked at) uses 0, so if that means no wait, taking it may not help us.

The vxworks docs for semTake say -1 is forever and 0 is no wait, but I'm not sure this is the same as what we call TakeSemaphore.

I think you're right - 0 means wait forever. The code is somewhat convoluted in the firmware :)

I'm working on finsig auto-detect of the 'filesem' value for DryOS cams, hope to post a patch soon.

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

  • ******
  • 14080
Re: Camera crash on startup - investigation.
« Reply #104 on: 20 / March / 2014, 23:38:15 »
Quote from: philmoz on 20 / March / 2014, 23:13:56
I think you're right - 0 means wait forever. The code is somewhat convoluted in the firmware :)
Yeah, if it doesn't I then I really don't understand a lot of the code that calls TakeSemaphore, but I'm going to try to make a test case anyway. There's quite a few places we should really use proper synchronization objects in the CHDK code.
Quote
I'm working on finsig auto-detect of the 'filesem' value for DryOS cams, hope to post a patch soon.
Thanks :)

edit:
I've verified TakeSemaphore works as expected on dryos (and judging from the vxworks firmware code, it must work the same way there)

wait = 0 is indefinite.
0 is returned if the semaphore is successfully acquired, 9 on timeout. The timeout is in ms.

Test patch attached.
« Last Edit: 21 / March / 2014, 01:15:46 by reyalp »
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Camera crash on startup - investigation.
« Reply #105 on: 21 / March / 2014, 01:15:14 »
Attached patch adds 'fileio_semaphore' detection to finsig_dryos.c, along with reyalps' open/close/write patch to use it.

The only DryOS camera not auto-detected was the S5IS - I've added this manually; but the code is quite different so if anyone has one to test that would be good.

Tested on G12, G1X and SX40.

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

  • ******
  • 14080
Re: Camera crash on startup - investigation.
« Reply #106 on: 21 / March / 2014, 02:03:24 »
Quote from: philmoz on 21 / March / 2014, 01:15:14
Tested on G12, G1X and SX40.
Works for me on elph130 and d10, appears to avoid the script autostart crash on d10.

If we can get a few more people to exercise it a bit without problems, I think it should go in the trunk.

It looks like the _Fut functions already do the same thing internally, so Lua IO should be OK.
Logged
Don't forget what the H stands for.
*

Offline waterwingz

  • ******
  • 12139
    • Waterwingz
Re: Camera crash on startup - investigation.
« Reply #107 on: 22 / March / 2014, 18:18:06 »
Quote from: philmoz on 21 / March / 2014, 01:15:14
Attached patch adds 'fileio_semaphore' detection to finsig_dryos.c, along with reyalps' open/close/write patch to use it.
Tested on A1200,  G10,  SX50, IXUS120_SD940.

All load and run without crashing.   DNG files looks good.  Script parameters save & load correctly.
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline reyalp

  • ******
  • 14080
Re: Camera crash on startup - investigation.
« Reply #108 on: 22 / March / 2014, 18:58:21 »
Here's a quick and dirty IO stress test. It can be applied with or without philmoz patch.

To use it, just run the build and turn on misc debug vals. The test runs at boot, and will blink the debug LED for a while. If anything fails, the FS line in misc debug vals will be non-zero, or the camera will crash.

The test repeatedly writes varying size files, reads them back and compares the results. Since it runs in it's own task at boot, it happens at the same time as a lot of other file activity from CHDK and the camera.

I get no errors or crashes with the semaphore patch on any camera.

On d10, I also ran it with the current current trunk and the pre-r3377 code with _Open.

The trunk runs without crashes or errors. The write failure discussed earlier should be detected and counted as an error if it was happening.

The old code crashes fairly frequently (but not 100% of the time) with the usual FsIoNotify crash.

Unless there are objections, I will check in the semaphore patch shortly.
Logged
Don't forget what the H stands for.
*

Offline reyalp

  • ******
  • 14080
Re: Camera crash on startup - investigation.
« Reply #109 on: 22 / March / 2014, 19:15:25 »
Checked in, trunk changeset 3392

It would be nice to get someone to test s5is if possible, but looking at the disassembly it seems like it should be OK.

If there are no complaints in the next few weeks, we can strip out the old CAM_STARTUP_CRASH_FILE_OPEN_FIX code and consider whether this should be backported to 1.2.

Based on my earlier testing, I don't think we need to do anything with the return value of TakeSemaphore.
Logged
Don't forget what the H stands for.
Pages: 1 ... 6 7 8 9 10 [11] 12   Go Up
« previous next »
 

Related Topics