> The comment isn't going to help anybody very much though, it would be > a better idea to improve on the comments on the vsm_overflow[ed] > counters where people can see them.
I have changed the patch and attached it again. Now the comment is trimmed down a bit, and with this the many typos are hopefully gone. As I say in the commit message, I plan to do a different patch for the documentation of the counters. I would appreciate it if I got a green light on the patch, or some feedback on anything else which should be changed before pushing. > It does not look like we emit a SLT_Error record when this happens, > I think we should. VSM_Alloc() looks like the best place for it, > but there is a chicken and egg issue since VSM_Alloc is also used > to allocate the VSL buffer. I think I understood this, but will not change anything about error reporting now, unless it is completely straightforward. I am just starting working with the code, and I do not want to introduce an infinite recursion during my first week at Varnish Software.
From ab18dcf1fd546ebcd0ef115b2556d66e287cb1d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l=20Hermunn=20Johansen?= <[email protected]> Date: Tue, 8 Mar 2016 11:09:20 +0100 Subject: [PATCH] Use calloc instead of malloc to allocate extra VSM space. With malloc we would read from uninitialized memory (implicitly with ++ or +=). Also expanded comment: When we need to allocate extra VSM space with malloc/calloc, the space will not be available to other processes. The problem can arise for setups where the value of the runtime parameter vsm_space is too low (usually left at its standard 1M), for example if the number of backends is very high. This can be detected through the counters vsm_overflow and vsm_overflowed, and the same counters indicates how much more vsm_space is needed. I plan to make a separate patch documenting this in the man page varnish-counters. --- bin/varnishd/common/common_vsm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/varnishd/common/common_vsm.c b/bin/varnishd/common/common_vsm.c index 94d583d..7fdc5f4 100644 --- a/bin/varnishd/common/common_vsm.c +++ b/bin/varnishd/common/common_vsm.c @@ -231,12 +231,12 @@ VSM_common_alloc(struct vsm_sc *sc, ssize_t size, } if (vr == NULL) { - /* - * No space in VSM, return malloc'd space + /* No space in VSM, return malloc'd space. The malloc'd + * space will not be availabe to other processes. */ ALLOC_OBJ(vr, VSM_RANGE_MAGIC); AN(vr); - vr->ptr = malloc(size); + vr->ptr = calloc(size, 1); AN(vr->ptr); vr->len = size; VTAILQ_INSERT_TAIL(&sc->r_bogus, vr, list); -- 2.1.4
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
