On Mon, May 06, 2013 at 10:45:08AM +0200, Holger Hans Peter Freyther wrote:
Hi,
> with the BFIlevel set high enough (or another fix) on the BTS side,
> the stress test branch of OpenBSC and this shell script the issue
> should be re-producable.
it is a "genuine" leak and there are several issues.
1.) lapd_core be like the logging/msgb code and have a setter for
the talloc context. See the attached diff. (I used gdb to break
in malloc and typed bt and saw a ctx=0x8 passed to talloc).
2.) Consider attaching the log_info to the lapdm_channel. I say
consider as currently we are embedding the lapdm_channel in the
lchan struct. So this is something we can not do right now.
3.) common/rsl.c calls lapdm_channel_reset which will not
talloc_free the tx_hist. lapdm_channel_exit will do that. The
call to lapdm_channel_reset was introduced in 2011.
4.) lchan_activate will call lapdm_channel_init which will
end up call into lapd_dl_init which re-allocates the tx_hist.
5.) We should find a way to group all VTY allocations into
a single block (e.g. see if we can have a vector/cmd_desc
without a named context).
I will clean this up in the coming days
holger
diff --git a/src/common/rsl.c b/src/common/rsl.c
index f669099..2b359ce 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -777,7 +777,7 @@ static int rsl_rx_rf_chan_rel(struct gsm_lchan *lchan)
rc = bts_model_rsl_chan_rel(lchan);
- lapdm_channel_reset(&lchan->lapdm_ch);
+ lapdm_channel_exit(&lchan->lapdm_ch);
return rc;
}
diff --git a/src/osmo-bts-sysmo/l1_transp_hw.c b/src/osmo-bts-sysmo/l1_transp_hw.c
index da8ac3f..80f8b47 100644
--- a/src/osmo-bts-sysmo/l1_transp_hw.c
+++ b/src/osmo-bts-sysmo/l1_transp_hw.c
@@ -208,14 +208,13 @@ static int l1if_fd_cb(struct osmo_fd *ofd, unsigned int what)
struct msgb *msg[ARRAY_SIZE(iov)];
for (i = 0; i < ARRAY_SIZE(iov); ++i) {
- msg[i] = msgb_alloc_headroom(prim_size + 128, 128, "1l_fd");
+ msg[i] = msgb_alloc_headroom(SYSMOBTS_PRIM_SIZE, 128, "1l_fd");
msg[i]->l1h = msg[i]->data;
iov[i].iov_base = msg[i]->l1h;
- iov[i].iov_len = msgb_tailroom(msg[i]);
+ iov[i].iov_len = prim_size;
}
-
rc = readv(ofd->fd, iov, ARRAY_SIZE(iov));
count = rc / prim_size;
diff --git a/src/osmo-bts-sysmo/main.c b/src/osmo-bts-sysmo/main.c
index 595a6eb..7ccb0eb 100644
--- a/src/osmo-bts-sysmo/main.c
+++ b/src/osmo-bts-sysmo/main.c
@@ -214,6 +214,7 @@ static void signal_handler(int signal)
case SIGUSR1:
case SIGUSR2:
talloc_report_full(tall_bts_ctx, stderr);
+ talloc_report_full(NULL, stderr);
break;
default:
break;
@@ -251,6 +252,7 @@ int main(int argc, char **argv)
tall_bts_ctx = talloc_named_const(NULL, 1, "OsmoBTS context");
tall_msgb_ctx = talloc_named_const(tall_bts_ctx, 1, "msgb");
msgb_set_talloc_ctx(tall_msgb_ctx);
+ lapd_global_init(tall_bts_ctx);
bts_log_init(NULL);
diff --git a/include/osmocom/gsm/lapd_core.h b/include/osmocom/gsm/lapd_core.h
index c2fdc62..ed0398d 100644
--- a/include/osmocom/gsm/lapd_core.h
+++ b/include/osmocom/gsm/lapd_core.h
@@ -168,6 +168,8 @@ int lapd_set_mode(struct lapd_datalink *dl, enum lapd_mode mode);
int lapd_ph_data_ind(struct msgb *msg, struct lapd_msg_ctx *lctx);
int lapd_recv_dlsap(struct osmo_dlsap_prim *dp, struct lapd_msg_ctx *lctx);
+int lapd_global_init(void *ctx);
+
/*! @} */
#endif /* _OSMOCOM_LAPD_H */
diff --git a/src/gsm/lapd_core.c b/src/gsm/lapd_core.c
index f351308..029b6f9 100644
--- a/src/gsm/lapd_core.c
+++ b/src/gsm/lapd_core.c
@@ -292,9 +292,6 @@ void lapd_dl_init(struct lapd_datalink *dl, uint8_t k, uint8_t v_range,
"history range = %d\n", dl->v_range, dl->k, dl->range_hist);
lapd_dl_newstate(dl, LAPD_STATE_IDLE);
-
- if (!tall_lapd_ctx)
- tall_lapd_ctx = talloc_named_const(NULL, 1, "lapd context");
dl->tx_hist = (struct lapd_history *) talloc_zero_array(tall_lapd_ctx,
struct log_info, dl->range_hist);
}
@@ -2173,4 +2170,10 @@ int lapd_recv_dlsap(struct osmo_dlsap_prim *dp, struct lapd_msg_ctx *lctx)
return rc;
}
+int lapd_global_init(void *ctx)
+{
+ tall_lapd_ctx = talloc_named_const(ctx, 1, "lapd context");
+ return 0;
+}
+
/*! @} */
