This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Neels Hofmeyr nhofmeyr at sysmocom.deOn Tue, Mar 12, 2019 at 09:46:28PM +0100, Harald Welte wrote: > This is why I have been arguing in favor of something like a "packet dispatch > talloc context" for some time. Unfortuantely I didn't have any time to work > on this so far. > > The proposal is along those lines: > * we create a talloc context every time we drop out of the select() statement > * any msgb's allocated from routines that read from sockets / etc. are allocated > from that context > * just before we return into osmo_select_main() [or in the start of that function], > we free the entire talloc context, which will free any msgb's in it > * if anyone wants to take ownership of a msgb for a longer time (e.g. enqueue it > somewhere for delayed processing, ...) they need to re-parent the object away > from the volatile "packet dispathc talloc context" and move it into another > context. Talloc provides talloc_steal() or better talloc_move() for it. > > I would think this is an elegant solution. I fully agree, it's a great solution for two problems we have: msgb ownership as well as re-using foo_name() functions in the same print format. One limitation to the talloc_steal(): assume some code path already stole from the volatile context, then it would be a bit rude to have another code path steal it from the first code path again. So maybe it would be good to be able to determine whether a msgb is still volatile or already owned, so that implementations can decide to only steal from the volatile context. > We could lazily allocate whatever we wanted (like temporary string buffers, > etc.) and know that it will be safely released once the current select dispatch > ceases to exist. That is very desirable IMO. I've looked at some of your WIP patches in gerrit, and there placed a comment towards this. I would like to change the current string buffer returning functions under the hood without having to change the function signature -- after all, the callers don't need to care whether a buffer is static or from a volatile context. No caller anywhere is allowed to assume the returned string remains valid for a long time, so they do already strcpy() if they have to keep the returned string safe. The current patches look like we need to produce a lot of patches to use the volatile API... see https://gerrit.osmocom.org/#/c/libosmocore/+/13311/ > use the stack whenever we can One advantage of changing all string buffer functions to a volatile buffer would be that any static-buf-reuse bugs we might already have in our LOGP or printf() will be fixed implicitly, and will also be avoided for the future. I would very much like to not have to think about writing log statements so much, but make foo_name() invocations safe, period. After all, we don't log that much if LOGL_DEBUG is disabled, and for DEBUG switched on we are typically in a non-production environment. If dynamic allocations are a problem, I could even imagine some array of 10 or so string buffers that are always kept around to avoid re-allocating too much for logging strings. You catch my drift, I'm for changing the current implementation without needing explicit changes to calling code. I'm certain that you have good reasons to not want to do that, but are they worth the human time to use/maintain new API? > > This is the case with osmo_sccp_user_sap_down(). > > Let me know what you think of the above-mentioned proposal. It would make my other patch obsolete, in a good way. https://gerrit.osmocom.org/13277 osmo_sccp_user_sap_down2() > One might evene be a able to do some trickery with talloc_set_destructor() > to print a BIG FAT WARNING/ERROR message in case anyone ever tries to free > memory allocated from this context manually. It's not harmful to free volatile bits ahead of time though. It is even desirable. If I want to use the current osmo_sccp_user_sap_down() as-is, I could just allocate a volatile msgb and feed that down the code path that ends up in osmo_sccp_user_sap_down(). If the msgb reaches all the way there, it gets freed within osmo_sccp_user_sap_down(), otherwise the volatile context will clean up the leftovers from error handling. If we instead enforce that volatile contexts are not freed manually, then we would again need an osmo_sccp_user_sap_down2() API change; I assume that's not the only one. > However, I realize only now that you're referring not to the case of received message > buffers, but about locally-originated msgbs. But for them, too, the rule could > be simply something like this: > * allocate memory from the volatile context > * pass it into any library function > * if the library function transmits it right away, no need to free/steal > * if the library function enqueues it somewhere and doesn't immediately send > it over a socket or the like, it needs to move/steal it. exactly. For changing existing enqueueing functions without needing a new API version, above idea would come in handy: determine whether the msgb is in the / a volatile context or not. If it is, steal, otherwise act as before. > Thinking of the above, I think the 'being queued' case is more or less the normal > one, as we can never make the assumption that the underlying transport layer > is able to accept transmission of any data in a non-blocking way. So there will > always be some kind of write queue wherever we transmit. In some cases we actually memcpy(), as I think osmo_sccp_user_sap_down() does. Otherwise it wouldn't be able to msgb_free() before returning, which it does, right? > * local primitives that are simply sent to another layer in the local stack, > and which are released immediately after that stack has received them ^ yes, that's what osmo_sccp_user_sap_down() does, of course ~N -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20190319/bbe35db0/attachment.bin>