suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

philmoz

3450

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #70 on: 22 / October / 2017, 04:44:34 »

Advertisements

Updated patch based on reyalps feedback and help.
- fixed the issue with the bad 'largest_block' value
- increased timeout to 1s
- added a second 'magic' value at the end of the memory block
- check the 'magic' values in suba_getmeminfo, assert if not correct.

Phil.

suba_sem9.patch (40.29 kB - downloaded 22 times.)

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)

reyalp

14080

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #71 on: 22 / October / 2017, 17:27:30 »

Thanks Phil. Checking the new patch now.

Regarding

Quote from: philmoz on 22 / October / 2017, 03:06:05

Quote
suba_init enforces size > 2048. Conceivably, I think CHDK in ARAM could leave less than that (but probably shouldn't, since minor changes might push it over the edge) We could skip the ARAM heap in that case.
Yes, I added that to avoid creating small allocators if CHDK almost filled ARAM.

The build currently fails if CHDK is set to load in ARAM but doesn't fit, so having it crash on startup if it's just a bit smaller may not be helpful. Alternately, we could silently disable it or change the compile time check.

I don't think we are at much risk of hitting this case at the moment because all the cameras with CHDK in ARAM are ones with the larger ARAM size, but diskboot.bin has actually gotten small enough that it could just barely fit in the small ARAM now. (in my build environment sx160 diskboot.bin is 0x217c0, ARAM is 0x22000)

Logged

Don't forget what the H stands for.

reyalp

14080

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #72 on: 22 / October / 2017, 18:42:39 »

Tested all my ARAM/EXMEM enabled cams with both the busy and memory stress tests. Looks ready to go to me.

Logged

Don't forget what the H stands for.

philmoz

3450

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #73 on: 22 / October / 2017, 23:00:12 »

Quote from: reyalp on 22 / October / 2017, 17:27:30

Thanks Phil. Checking the new patch now.

Regarding
Quote from: philmoz on 22 / October / 2017, 03:06:05
Quote
suba_init enforces size > 2048. Conceivably, I think CHDK in ARAM could leave less than that (but probably shouldn't, since minor changes might push it over the edge) We could skip the ARAM heap in that case.
Yes, I added that to avoid creating small allocators if CHDK almost filled ARAM.
The build currently fails if CHDK is set to load in ARAM but doesn't fit, so having it crash on startup if it's just a bit smaller may not be helpful. Alternately, we could silently disable it or change the compile time check.

Good point, didn't consider that would be the result. This test probably belongs in memmgnt.c when ARAM in setup.

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)

philmoz

3450

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #74 on: 24 / October / 2017, 05:56:30 »

Added to SVN in revision 4923 (3 arg DebugAssert support) & 4924 (new suba.c version).

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)

philmoz

3450

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #75 on: 27 / October / 2017, 18:00:14 »

Quote from: reyalp on 22 / October / 2017, 02:50:34

Should this be backported to 1.4? It's a significant crash bug, which argues for yes, but the suba re-write makes me a bit nervous, and there will be some conflicts involving the ports that aren't in 1.4.

The SVN updates (4919-4921,4923 & 4924) can be applied to 1.4 without any issues.
After manually adding the CreateBinarySemaphore stub for the SX280 all cameras build.
Tested on G12 and no apparent problems.

Do you think it's worth committing to 1.4?

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)

reyalp

14080

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #76 on: 27 / October / 2017, 21:55:22 »

Quote from: philmoz on 27 / October / 2017, 18:00:14

Do you think it's worth committing to 1.4?

Yeah, I'm in favor of having it in 1.4.

Logged

Don't forget what the H stands for.

philmoz

3450

Re: suba threading bug (was Re: 3D scanner with multiple IXUS160 cameras)

« Reply #77 on: 29 / October / 2017, 02:35:36 »

Quote from: reyalp on 27 / October / 2017, 21:55:22

Quote from: philmoz on 27 / October / 2017, 18:00:14
Do you think it's worth committing to 1.4?
Yeah, I'm in favor of having it in 1.4.

Added on SVN revision 4925.

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)