Quoting Oren Laadan (orenl@xxxxxxxxxxx):
>
>
> Serge E. Hallyn wrote:
> > Quoting Matt Helsley (matthltc@xxxxxxxxxx):
> >>> @@ -401,6 +409,9 @@ char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt)
> >>> case 'E':
> >>> len += sprintf(format+len, "[%s]", "err %d");
> >>> break;
> >>> + case 'C': /* count of bytes read/written to checkpoint image */
> >>> + len += sprintf(format+len, "[%s]", "pos %d");
> >>> + break;
> >> Instead we could always output ckpt->total and then we wouldn't need %(C). I
> >> suspect it's such a useful piece of information that it'll be repeated
> >> in many/all format strings eventually.
> >
> > Yes, likewise %(T). If that's what we want to do.
>
> I agree. For the cases when there is not task, can put "none"
>
> >
> > Should we discuss here what we want an entry to look like? For both
> > ckpt_write_err (to the checkpoint image) and ckpt_error()?
> >
>
> Yes please !
Actually %T isn't the current task, right, so it shouldn't always be prepended?
It actually is only meaningful during checkpoint_task(), collect_objs(), and
__tree_count_tasks?
Ok, so how about:
1. ckpt_write_err() always also calls ckpt_error() (which in turn calls
ckpt_debug). Avoid duplication which exists in several places
right now.
2. We always prepend:
[current->pid]:[ctx->root_pid]:[ctx->active_pid]:[ctx->errno][ctx->total]
The %(X) expansions if specified come whereever they are in the fmt
string (which is what's happening now with my patchset).
Kind of long, but again this is for ckpt_error and ckpt_write_err, not for all
ckpt_debugs().
> >>> case 'O':
> >>> len += sprintf(format+len, "[%s]", "obj %d");
> >>> break;
> >>> @@ -435,6 +446,51 @@ char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt)
> >>> return format;
> >>> }
> >>>
> >>> +void ckpt_log_error(struct ckpt_ctx *ctx, char *fmt, ...)
> >>> +{
> >>> + mm_segment_t fs;
> >>> + struct file *file;
> >>> + int count;
> >>> + va_list ap, aq, az;
> >>> + char *format;
> >>> + char buf[200], *bufp = buf;
> >> I believe this buffer is too big for a kernel stack -- especially
> >> for ckpt_log_error() which might be invoked "deep" in
> >> the kernel stack.
> >
> > 200 bytes? Well, I guess I can try with 50 which still may often be
> > enough.
>
> How about using a dedicated buffer on @ctx for that ?
I was going to do that originally, but then thought back to your
comments about parallel checkpoint, and didn't feel like also adding
a spinlock.
Thoughts?
-serge
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
