lwIP - A Lightweight TCP/IP stack - Bugs: bug #21433, Calling mem_free/pbuf_free from...
You are not allowed to post comments on this tracker with your current authentication level.
bug #21433: Calling mem_free/pbuf_free from interrupt context isn't safe
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Thu 25 Oct 2007 07:14:47 PM UTC | ||
Category: | pbufs | Severity: | 3 - Normal |
Item Group: | Change Request | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | CVS Head |
Jump to the original submission
Thu 10 Jul 2008 07:03:37 PM UTC, comment #26: |
Simon Goldschmidt <goldsimon> |
Mon 30 Jun 2008 04:05:49 PM UTC, comment #25: I must say I really don't care whether this is an option or not: I know what it does and wouldn't use mem_free/pbuf_free from interrupt context, anyway.
|
Simon Goldschmidt <goldsimon> |
Mon 30 Jun 2008 03:40:05 PM UTC, comment #24: I haven't tried out the code, but it looks great to me. Since you've tested it, I think this is fine to go in. Presumably there's a tweak to opt.h of course, and the description around that can probably be the documentation (since there's no other place to doc it, I think?).
|
Jonathan Larmour <jifl> |
Fri 27 Jun 2008 09:00:13 PM UTC, comment #23: Jonathan: I've changed mem_free/mem_malloc according to your idea from comment #10
|
Simon Goldschmidt <goldsimon> |
Fri 28 Mar 2008 02:18:01 PM UTC, comment #22: Oh I see what you're getting at. What I was envisaging was that the existing semaphore remained, but mem_free wouldn't use it.
|
Jonathan Larmour <jifl> |
Fri 28 Mar 2008 02:04:11 PM UTC, comment #21:
|
Simon Goldschmidt <goldsimon> |
Fri 28 Mar 2008 12:50:51 PM UTC, comment #20: Well, the point was to allow mem_free from other contexts such as interrupts. Allowing mem_alloc as well would indeed be a different kettle of fish. |
Jonathan Larmour <jifl> |
Fri 28 Mar 2008 11:39:42 AM UTC, comment #19: Actually, thinking more about your idea, I must say I like it!
|
Simon Goldschmidt <goldsimon> |
Fri 28 Mar 2008 09:21:29 AM UTC, comment #18: Just to be clear, I don't mind if you don't like the idea, as long as you don't like it only after we properly understand its merits and any drawbacks :-).
|
Jonathan Larmour <jifl> |
Fri 28 Mar 2008 09:19:03 AM UTC, comment #17:
|
Jonathan Larmour <jifl> |
Fri 28 Mar 2008 08:04:10 AM UTC, comment #16:
|
Simon Goldschmidt <goldsimon> |
Thu 27 Mar 2008 10:11:37 PM UTC, comment #15: I'm slightly unsure about referring to disabling interrupts all the time since on many embedded OSes this would actually disable tasking, which is slightly different.
|
Jonathan Larmour <jifl> |
Thu 27 Mar 2008 07:29:50 PM UTC, comment #14: I included a new option LWIP_USE_HEAP_FROM_INTERRUPT in opt.h (default = 0) that lets mem.c disable interrupts instead of using a semaphore (well documented in opt.h).
|
Simon Goldschmidt <goldsimon> |
Thu 01 Nov 2007 01:04:42 PM UTC, comment #13: OK, that sounds like a plan, and I agree with your a) b) c) requirements. |
Kieran Mansley <kieranm> |
Thu 01 Nov 2007 01:01:21 PM UTC, comment #12:
|
Simon Goldschmidt <goldsimon> |
Thu 01 Nov 2007 12:49:22 PM UTC, comment #11: I have to admit I'm a bit uneasy about increasing the amount of stuff that we support from interrupt context because in many cases doing this sort of thing from interrupt context is a bad idea. There are cases (perhaps like the one that prompted this) where it helps, but by adding support we make it tempting for people to write bad code.
|
Kieran Mansley <kieranm> |
Tue 30 Oct 2007 04:49:47 PM UTC, comment #10: plug_holes isn't a problem - for sure it would not be the largest part in a system that is run with interrupts off (certainly not if you have ISRs is doing things like pbuf_free!).
|
Jonathan Larmour <jifl> |
Tue 30 Oct 2007 04:07:43 PM UTC, comment #9:
|
EVS Hardware Dpt <vse> |
Tue 30 Oct 2007 03:15:04 PM UTC, comment #8: Hold on here, Simon is saying that using SYS_ARCH_PROTECT in mem_free is not good because it takes too long to walk the heap. I'm failing to see where the size of the heap (or number of allocations) is a factor. Maybe I'm just blind. Simon, can you tell me where this dependency is?
|
Jonathan Larmour <jifl> |
Tue 30 Oct 2007 09:03:41 AM UTC, comment #7: Hello,
|
EVS Hardware Dpt <vse> |
Sun 28 Oct 2007 09:30:49 PM UTC, comment #6:
|
Frédéric Bernon <fbernon> |
Fri 26 Oct 2007 03:51:31 PM UTC, comment #5: Personally, I don't know what to prefer here.
|
Simon Goldschmidt <goldsimon> |
Fri 26 Oct 2007 08:47:52 AM UTC, comment #4: Hello,
|
EVS Hardware Dpt <vse> |
Fri 26 Oct 2007 08:28:43 AM UTC, comment #3: I agree with Simon, the proposed patch is a good generic solution.
|
EVS Hardware Dpt <vse> |
Thu 25 Oct 2007 08:25:46 PM UTC, comment #2: Personally, I'm not sure I agree. This adds extra data structures, list processing code, and does so whether the driver model requires it or not, and I suspect many do not (mine doesn't, for example). Most importantly it does not help raw API users with NO_SYS==1.
|
Jonathan Larmour <jifl> |
Thu 25 Oct 2007 08:03:24 PM UTC, comment #1: My favourite solution for this is to create a new semaphore function 'sys_sem_tryget(sem)' which behaves much like 'sys_arch_mbox_tryfetch()' (check if the semaphore is available, return SYS_ARCH_TIMEOUT if not; don't block -> usable from interrupt context).
|
Simon Goldschmidt <goldsimon> |
Thu 25 Oct 2007 07:14:47 PM UTC, original submission:
As discussed on lwip-users on the 25th of october, PBUF_RAM pbufs may not be freed from interrupt context since mem_free is only protected by a semaphore (or not at all if NO_SYS==1) see http://lists.nongnu.org/archive/html/lwip-users/2007-10/msg00116.html and replies.
|
Simon Goldschmidt <goldsimon> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 8 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2008-07-10 | goldsimon | Status | Ready For Test | Fixed | |
Open/Closed | Open | Closed | |||
2008-06-27 | goldsimon | Attached File | - | Added mem.c_21433.patch, #15950 | |
2008-03-27 | goldsimon | Status | None | Ready For Test | |
Assigned to | None | goldsimon | |||
2008-03-04 | kieranm | Planned Release | 1.3.1 | ||
2007-10-26 | vse | Attached File | - | Added LockedAccess.c, #14216 | |
2007-10-25 | goldsimon | Attached File | - | Added mem_free_from_interrupt.patch, #14212 |
Powered by Savane 3.13-f8d8.
Corresponding source code
done.