[PATCH 3/3] lib/core/fiber: Put watermarks into stack to track its usage
Cyrill Gorcunov
gorcunov at gmail.com
Wed Mar 13 17:00:54 MSK 2019
On Wed, Mar 13, 2019 at 04:52:04PM +0300, Vladimir Davydov wrote:
> >
> > +static bool
> > +stack_has_wmark(void *addr)
>
> Please add brief comments to all the functions you add.
ok
> > +
> > + /*
> > + * When dropping pages make sure the page
> > + * containing overflow mark is untouched.
> > + * Same time no need to unmap the page which
> > + * carries "shrink" wmark, since we're updating
> > + * this page anyway.
> > + */
> > + if (stack_direction < 0) {
> > + end = page_align_down(fiber->stack_shrink_wmark);
> > + start = page_align_up(fiber->stack_overflow_wmark);
> > + madvise(start, end - start, MADV_DONTNEED);
> > + } else {
> > + start = page_align_up(fiber->stack_shrink_wmark);
> > + end = page_align_down(fiber->stack_overflow_wmark);
> > + madvise(start, end - start, MADV_DONTNEED);
> > + }
>
> Nit: you could move madvise() out of the if-else block.
indeed, thanks!
> > +
> > + if (!stack_has_wmark(fiber->stack_overflow_wmark)) {
> > + say_warn("stack usage is close to the limit of %zu bytes",
> > + (size_t)FIBER_STACK_SIZE_DEFAULT);
>
> IMO warning only when we are close to the limit isn't very useful.
> Let's keep track of the max stack size instead and print it whenever
> it's increased.
Could you please elaborate. I don't understand what you mean by
keep tracking of max stack size. You propose to remember somewhere
if this mark has been vanished?
> > + fiber->wmark_inpage_offset = ((rand() % 128) + 8) & ~7;
>
> Please define a constant for this.
ok
> > + fiber->stack_shrink_wmark -= 16 << 12;
>
> Ditto.
ok
> > @@ -927,8 +1090,12 @@ cord_create(struct cord *cord, const char *name)
> > /* Record stack extents */
> > tt_pthread_attr_getstack(cord->id, &cord->sched.stack,
> > &cord->sched.stack_size);
> > + cord->sched.stack_overflow_wmark = cord->sched.stack;
> > + cord->sched.stack_shrink_wmark = cord->sched.stack;
>
> I don't think we need to set the watermarks for the sched fiber,
> even if ASAN is on.
Won't they left unitialized then? The only reason I put nils here
is to force them have known values even if they are unused.
Cyrill
More information about the Tarantool-patches
mailing list