RFC: talloc contexts / automatic free at select loop

[Harald Welte]

Hi all,

this idea has been floating around for quite some time, and I finally took
some time for a proposed implementation:  The introduction of a "volatile"
talloc context which one can use to allocate whatever temporary things from,
and which would automatically be free'd once we leave the select dispatch.

The current proposal is in https://gerrit.osmocom.org/#/c/libosmocore/+/13312
and I'm happy to receive any review.

How this works:

* within a new osmo_select_main_ctx() function, we create a temporary talloc
  context before calling the filedescriptor call-back functions, and we
  free that after the call-backs have returned

* any of the code running from within the select loop dispatch (which for
  "normal" osmocom CNI projects is virtually everything) can use that
  temporary context for allocations.  There's a OTC_SELECT #define for
  convenience. So you could do something like talloc_zero(OTC_SELECT, ...)
  which would be automatically free'd after the current select loop
  iteration.

Where is this useful?  There's at least two common use cases:

* allocation of message buffers without having to worry about msgb ownership
* various temporary buffers e.g. for something-to-string conversions where
  currently we use library-internal static buffers with all their known problems
  (not being able to use them twice within a single printf() statement, not
  being thread-safe, ...)

To Neels' disappointment, this is not all automatic. You

a) have to call the _c suffix of the respective function, e.g. osmo_hexdump_c()
   instead of osmo_hexdump() with one extra initial argument:
   OTC_SELECT. There's also msgb_alloc_c(), msgb_copy_c() and the like,
   allowing msgb allocation from a context of your choice, as opposed
   to the library-internal msgb_tall_ctx that we had so far.

b) have to use osmo_select_main_ctx() instead of osmo_select_main().  This is
   an arbitrary limitation for optimization that Pau requested, to make sure
   applications that don't want this can avoid all the extra talloc+free at
   every select iteration.  This is debatable, and we can make it automatic
   in osmo_select_main().  It's probably worth a benchmark how expensive
   that 'empty' allocation + free is.

However, I think that's a rather "OK" price to pay.  Converting the existing
callers can be more or less done with "sed" (yes, spatch is of course better).

While introducing this feature, I also tried to address two other topics, which
are not strictly related.  However, ignoring those two now would mean
we'd have API/ABI breaks if we'd care about them in the future.  Hence I
think we should resolve them right away:

1) introduce another context: OTC_GLOBAL.  This is basically a library-internal
   replacement for the "g_tall_ctx" that we typically generate as one of the
   first things in every application.  You can use it anywhere where you'd want
   to allocate something form the global talloc context

2) make those contexts thread-local. As you may know, talloc is not thread
   safe. So you cannot allocate/free from a single context on multiple
   threds.  However, it is safe to have separate talloc contexts on each
   thread. Making the new OTC_* contexts per-thread, we are enabling the
   [limited] use of this functionality in multi-threaded programs.  This
   is of course very far from making libosmocore thread-safe.  There are
   many library-internal data structures like timers, file descriptors,
   the VTY, ... which are absolutely not thread-safe.  However, I think
   it's a step in the right direction and I don't expect any performance
   impact for single-threaded programs by marking the contexts as
   __thread.

-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)