Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
From: Evgeniy PolyakovDate: Mon, 30 Apr 2018 18:01:30 +0300 > Stefan, hi > > Sorry for delay. > > 26.04.2018, 15:04, "Stefan Strogin" : >> Hi David, Evgeniy, >> >> Sorry to bother you, but could you please comment about the UAPI change and >> the patch? > > With 4-bytes pid_t everything looks fine, and I do not know arch where pid is > larger currently, so it looks safe. > > David, please pull it into your tree, or should it go via different path? > > Acked-by: Evgeniy Polyakov After this much time it needs to be resubmitted.
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
From: Evgeniy Polyakov Date: Mon, 30 Apr 2018 18:01:30 +0300 > Stefan, hi > > Sorry for delay. > > 26.04.2018, 15:04, "Stefan Strogin" : >> Hi David, Evgeniy, >> >> Sorry to bother you, but could you please comment about the UAPI change and >> the patch? > > With 4-bytes pid_t everything looks fine, and I do not know arch where pid is > larger currently, so it looks safe. > > David, please pull it into your tree, or should it go via different path? > > Acked-by: Evgeniy Polyakov After this much time it needs to be resubmitted.
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Stefan, hi Sorry for delay. 26.04.2018, 15:04, "Stefan Strogin": > Hi David, Evgeniy, > > Sorry to bother you, but could you please comment about the UAPI change and > the patch? With 4-bytes pid_t everything looks fine, and I do not know arch where pid is larger currently, so it looks safe. David, please pull it into your tree, or should it go via different path? Acked-by: Evgeniy Polyakov >> I don't see how it breaks UAPI. The point is that structures >> coredump_proc_event and exit_proc_event are members of *union* >> event_data, thus position of the existing data in the structure is >> unchanged. Furthermore, this change won't increase size of struct >> proc_event, because comm_proc_event (also a member of event_data) is >> of bigger size than the changed structures. >> >> If I'm wrong, could you please explain what exactly will the change >> break in UAPI? >> >> On 30/03/18 19:59, David Miller wrote: >>> From: Stefan Strogin >>> Date: Thu, 29 Mar 2018 17:12:47 +0300 >>> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h index 68ff25414700..db210625cee8 100644 --- a/include/uapi/linux/cn_proc.h +++ b/include/uapi/linux/cn_proc.h @@ -116,12 +116,16 @@ struct proc_event { struct coredump_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } coredump; struct exit_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; __u32 exit_code, exit_signal; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } exit; } event_data; >>> >>> I don't think you can add these members without breaking UAPI.
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Stefan, hi Sorry for delay. 26.04.2018, 15:04, "Stefan Strogin" : > Hi David, Evgeniy, > > Sorry to bother you, but could you please comment about the UAPI change and > the patch? With 4-bytes pid_t everything looks fine, and I do not know arch where pid is larger currently, so it looks safe. David, please pull it into your tree, or should it go via different path? Acked-by: Evgeniy Polyakov >> I don't see how it breaks UAPI. The point is that structures >> coredump_proc_event and exit_proc_event are members of *union* >> event_data, thus position of the existing data in the structure is >> unchanged. Furthermore, this change won't increase size of struct >> proc_event, because comm_proc_event (also a member of event_data) is >> of bigger size than the changed structures. >> >> If I'm wrong, could you please explain what exactly will the change >> break in UAPI? >> >> On 30/03/18 19:59, David Miller wrote: >>> From: Stefan Strogin >>> Date: Thu, 29 Mar 2018 17:12:47 +0300 >>> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h index 68ff25414700..db210625cee8 100644 --- a/include/uapi/linux/cn_proc.h +++ b/include/uapi/linux/cn_proc.h @@ -116,12 +116,16 @@ struct proc_event { struct coredump_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } coredump; struct exit_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; __u32 exit_code, exit_signal; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } exit; } event_data; >>> >>> I don't think you can add these members without breaking UAPI.
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Hi David, Evgeniy, Sorry to bother you, but could you please comment about the UAPI change and the patch? Thanks, Jesper. -- Stefan On 05/04/18 12:07, Jesper Derehag wrote: > Unless David comes back with something I have (also) missed regarding uapi > breakage, this looks good to me. > > /Jesper > > > Från: Stefan Strogin> Skickat: den 2 april 2018 17:18 > Till: David Miller > Kopia: z...@ioremap.net; net...@vger.kernel.org; > linux-kernel@vger.kernel.org; xe-linux-exter...@cisco.com; > jdere...@hotmail.com; matt.hels...@gmail.com; mini...@googlemail.com > Ämne: Re: [PATCH] connector: add parent pid and tgid to coredump and exit > events > > Hi David, > > I don't see how it breaks UAPI. The point is that structures > coredump_proc_event and exit_proc_event are members of *union* > event_data, thus position of the existing data in the structure is > unchanged. Furthermore, this change won't increase size of struct > proc_event, because comm_proc_event (also a member of event_data) is > of bigger size than the changed structures. > > If I'm wrong, could you please explain what exactly will the change > break in UAPI? > > > On 30/03/18 19:59, David Miller wrote: >> From: Stefan Strogin >> Date: Thu, 29 Mar 2018 17:12:47 +0300 >> >>> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h >>> index 68ff25414700..db210625cee8 100644 >>> --- a/include/uapi/linux/cn_proc.h >>> +++ b/include/uapi/linux/cn_proc.h >>> @@ -116,12 +116,16 @@ struct proc_event { >>> struct coredump_proc_event { >>> __kernel_pid_t process_pid; >>> __kernel_pid_t process_tgid; >>> +__kernel_pid_t parent_pid; >>> +__kernel_pid_t parent_tgid; >>> } coredump; >>> >>> struct exit_proc_event { >>> __kernel_pid_t process_pid; >>> __kernel_pid_t process_tgid; >>> __u32 exit_code, exit_signal; >>> +__kernel_pid_t parent_pid; >>> +__kernel_pid_t parent_tgid; >>> } exit; >>> >>> } event_data; >> >> I don't think you can add these members without breaking UAPI. >> >
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Hi David, Evgeniy, Sorry to bother you, but could you please comment about the UAPI change and the patch? Thanks, Jesper. -- Stefan On 05/04/18 12:07, Jesper Derehag wrote: > Unless David comes back with something I have (also) missed regarding uapi > breakage, this looks good to me. > > /Jesper > > > Från: Stefan Strogin > Skickat: den 2 april 2018 17:18 > Till: David Miller > Kopia: z...@ioremap.net; net...@vger.kernel.org; > linux-kernel@vger.kernel.org; xe-linux-exter...@cisco.com; > jdere...@hotmail.com; matt.hels...@gmail.com; mini...@googlemail.com > Ämne: Re: [PATCH] connector: add parent pid and tgid to coredump and exit > events > > Hi David, > > I don't see how it breaks UAPI. The point is that structures > coredump_proc_event and exit_proc_event are members of *union* > event_data, thus position of the existing data in the structure is > unchanged. Furthermore, this change won't increase size of struct > proc_event, because comm_proc_event (also a member of event_data) is > of bigger size than the changed structures. > > If I'm wrong, could you please explain what exactly will the change > break in UAPI? > > > On 30/03/18 19:59, David Miller wrote: >> From: Stefan Strogin >> Date: Thu, 29 Mar 2018 17:12:47 +0300 >> >>> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h >>> index 68ff25414700..db210625cee8 100644 >>> --- a/include/uapi/linux/cn_proc.h >>> +++ b/include/uapi/linux/cn_proc.h >>> @@ -116,12 +116,16 @@ struct proc_event { >>> struct coredump_proc_event { >>> __kernel_pid_t process_pid; >>> __kernel_pid_t process_tgid; >>> +__kernel_pid_t parent_pid; >>> +__kernel_pid_t parent_tgid; >>> } coredump; >>> >>> struct exit_proc_event { >>> __kernel_pid_t process_pid; >>> __kernel_pid_t process_tgid; >>> __u32 exit_code, exit_signal; >>> +__kernel_pid_t parent_pid; >>> +__kernel_pid_t parent_tgid; >>> } exit; >>> >>> } event_data; >> >> I don't think you can add these members without breaking UAPI. >> >
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Hi David, I don't see how it breaks UAPI. The point is that structures coredump_proc_event and exit_proc_event are members of *union* event_data, thus position of the existing data in the structure is unchanged. Furthermore, this change won't increase size of struct proc_event, because comm_proc_event (also a member of event_data) is of bigger size than the changed structures. If I'm wrong, could you please explain what exactly will the change break in UAPI? On 30/03/18 19:59, David Miller wrote: > From: Stefan Strogin> Date: Thu, 29 Mar 2018 17:12:47 +0300 > >> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h >> index 68ff25414700..db210625cee8 100644 >> --- a/include/uapi/linux/cn_proc.h >> +++ b/include/uapi/linux/cn_proc.h >> @@ -116,12 +116,16 @@ struct proc_event { >> struct coredump_proc_event { >> __kernel_pid_t process_pid; >> __kernel_pid_t process_tgid; >> +__kernel_pid_t parent_pid; >> +__kernel_pid_t parent_tgid; >> } coredump; >> >> struct exit_proc_event { >> __kernel_pid_t process_pid; >> __kernel_pid_t process_tgid; >> __u32 exit_code, exit_signal; >> +__kernel_pid_t parent_pid; >> +__kernel_pid_t parent_tgid; >> } exit; >> >> } event_data; > > I don't think you can add these members without breaking UAPI. >
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
Hi David, I don't see how it breaks UAPI. The point is that structures coredump_proc_event and exit_proc_event are members of *union* event_data, thus position of the existing data in the structure is unchanged. Furthermore, this change won't increase size of struct proc_event, because comm_proc_event (also a member of event_data) is of bigger size than the changed structures. If I'm wrong, could you please explain what exactly will the change break in UAPI? On 30/03/18 19:59, David Miller wrote: > From: Stefan Strogin > Date: Thu, 29 Mar 2018 17:12:47 +0300 > >> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h >> index 68ff25414700..db210625cee8 100644 >> --- a/include/uapi/linux/cn_proc.h >> +++ b/include/uapi/linux/cn_proc.h >> @@ -116,12 +116,16 @@ struct proc_event { >> struct coredump_proc_event { >> __kernel_pid_t process_pid; >> __kernel_pid_t process_tgid; >> +__kernel_pid_t parent_pid; >> +__kernel_pid_t parent_tgid; >> } coredump; >> >> struct exit_proc_event { >> __kernel_pid_t process_pid; >> __kernel_pid_t process_tgid; >> __u32 exit_code, exit_signal; >> +__kernel_pid_t parent_pid; >> +__kernel_pid_t parent_tgid; >> } exit; >> >> } event_data; > > I don't think you can add these members without breaking UAPI. >
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
From: Stefan StroginDate: Thu, 29 Mar 2018 17:12:47 +0300 > diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h > index 68ff25414700..db210625cee8 100644 > --- a/include/uapi/linux/cn_proc.h > +++ b/include/uapi/linux/cn_proc.h > @@ -116,12 +116,16 @@ struct proc_event { > struct coredump_proc_event { > __kernel_pid_t process_pid; > __kernel_pid_t process_tgid; > + __kernel_pid_t parent_pid; > + __kernel_pid_t parent_tgid; > } coredump; > > struct exit_proc_event { > __kernel_pid_t process_pid; > __kernel_pid_t process_tgid; > __u32 exit_code, exit_signal; > + __kernel_pid_t parent_pid; > + __kernel_pid_t parent_tgid; > } exit; > > } event_data; I don't think you can add these members without breaking UAPI.
Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
From: Stefan Strogin Date: Thu, 29 Mar 2018 17:12:47 +0300 > diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h > index 68ff25414700..db210625cee8 100644 > --- a/include/uapi/linux/cn_proc.h > +++ b/include/uapi/linux/cn_proc.h > @@ -116,12 +116,16 @@ struct proc_event { > struct coredump_proc_event { > __kernel_pid_t process_pid; > __kernel_pid_t process_tgid; > + __kernel_pid_t parent_pid; > + __kernel_pid_t parent_tgid; > } coredump; > > struct exit_proc_event { > __kernel_pid_t process_pid; > __kernel_pid_t process_tgid; > __u32 exit_code, exit_signal; > + __kernel_pid_t parent_pid; > + __kernel_pid_t parent_tgid; > } exit; > > } event_data; I don't think you can add these members without breaking UAPI.
[PATCH] connector: add parent pid and tgid to coredump and exit events
The intention is to get notified of process failures as soon as possible, before a possible core dumping (which could be very long) (e.g. in some process-manager). Coredump and exit process events are perfect for such use cases (see 2b5faa4c553f "connector: Added coredumping event to the process connector"). The problem is that for now the process-manager cannot know the parent of a dying process using connectors. This could be useful if the process-manager should monitor for failures only children of certain parents, so we could filter the coredump and exit events by parent process and/or thread ID. Add parent pid and tgid to coredump and exit process connectors event data. Signed-off-by: Stefan Strogin--- drivers/connector/cn_proc.c | 4 include/uapi/linux/cn_proc.h | 4 2 files changed, 8 insertions(+) diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index a782ce87715c..ed5e42461094 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -262,6 +262,8 @@ void proc_coredump_connector(struct task_struct *task) ev->what = PROC_EVENT_COREDUMP; ev->event_data.coredump.process_pid = task->pid; ev->event_data.coredump.process_tgid = task->tgid; + ev->event_data.coredump.parent_pid = task->real_parent->pid; + ev->event_data.coredump.parent_tgid = task->real_parent->tgid; memcpy(>id, _proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ @@ -288,6 +290,8 @@ void proc_exit_connector(struct task_struct *task) ev->event_data.exit.process_tgid = task->tgid; ev->event_data.exit.exit_code = task->exit_code; ev->event_data.exit.exit_signal = task->exit_signal; + ev->event_data.exit.parent_pid = task->real_parent->pid; + ev->event_data.exit.parent_tgid = task->real_parent->tgid; memcpy(>id, _proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h index 68ff25414700..db210625cee8 100644 --- a/include/uapi/linux/cn_proc.h +++ b/include/uapi/linux/cn_proc.h @@ -116,12 +116,16 @@ struct proc_event { struct coredump_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } coredump; struct exit_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; __u32 exit_code, exit_signal; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } exit; } event_data; -- 2.11.0
[PATCH] connector: add parent pid and tgid to coredump and exit events
The intention is to get notified of process failures as soon as possible, before a possible core dumping (which could be very long) (e.g. in some process-manager). Coredump and exit process events are perfect for such use cases (see 2b5faa4c553f "connector: Added coredumping event to the process connector"). The problem is that for now the process-manager cannot know the parent of a dying process using connectors. This could be useful if the process-manager should monitor for failures only children of certain parents, so we could filter the coredump and exit events by parent process and/or thread ID. Add parent pid and tgid to coredump and exit process connectors event data. Signed-off-by: Stefan Strogin --- drivers/connector/cn_proc.c | 4 include/uapi/linux/cn_proc.h | 4 2 files changed, 8 insertions(+) diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index a782ce87715c..ed5e42461094 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -262,6 +262,8 @@ void proc_coredump_connector(struct task_struct *task) ev->what = PROC_EVENT_COREDUMP; ev->event_data.coredump.process_pid = task->pid; ev->event_data.coredump.process_tgid = task->tgid; + ev->event_data.coredump.parent_pid = task->real_parent->pid; + ev->event_data.coredump.parent_tgid = task->real_parent->tgid; memcpy(>id, _proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ @@ -288,6 +290,8 @@ void proc_exit_connector(struct task_struct *task) ev->event_data.exit.process_tgid = task->tgid; ev->event_data.exit.exit_code = task->exit_code; ev->event_data.exit.exit_signal = task->exit_signal; + ev->event_data.exit.parent_pid = task->real_parent->pid; + ev->event_data.exit.parent_tgid = task->real_parent->tgid; memcpy(>id, _proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h index 68ff25414700..db210625cee8 100644 --- a/include/uapi/linux/cn_proc.h +++ b/include/uapi/linux/cn_proc.h @@ -116,12 +116,16 @@ struct proc_event { struct coredump_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } coredump; struct exit_proc_event { __kernel_pid_t process_pid; __kernel_pid_t process_tgid; __u32 exit_code, exit_signal; + __kernel_pid_t parent_pid; + __kernel_pid_t parent_tgid; } exit; } event_data; -- 2.11.0