Messages in this thread | | | Subject | Re: [RFC][PATCH 3/3] Process events biarch bug: New process events connector value | From | Matt Helsley <> | Date | Tue, 27 Jun 2006 14:39:42 -0700 |
| |
On Tue, 2006-06-27 at 12:14 -0700, Chandra Seetharaman wrote: > On Tue, 2006-06-27 at 04:49 -0700, Matt Helsley wrote: > > "Deprecate" existing Process Events connector interface and add a new one > > that works cleanly on biarch platforms. > > > > Any expansion of the previous event structure would break userspace's ability > > to workaround the biarch incompatibility problem. Hence this patch creates a > > new interface and generates events (for both when necessary). > > Is there a reason why the # of listeners part is removed (basically the > LISTEN/IGNORE) ? and why as part of this patch ?
Michael Kerrisk had some objections to LISTEN/IGNORE and I've been looking into making a connector function that would replace them. They exist primarily to improve performance by avoiding the memory allocation in cn_netlink_send() when there are no listeners.
> <snip> > > > @@ -158,16 +164,15 @@ static int cn_proc_watch_task(struct not > > void *t) > > { > > struct task_struct *task = t; > > struct cn_msg *msg; > > struct proc_event *ev; > > + struct proc_event_deprecated *ev_old; > > + struct timespec timestamp; > > __u8 buffer[CN_PROC_MSG_SIZE]; > > int rc = NOTIFY_OK; > > > > - if (atomic_read(&proc_event_num_listeners) < 1) > > - return rc; > > - > > msg = (struct cn_msg*)buffer; > > ev = (struct proc_event*)msg->data; > > switch (get_watch_event(val)) { > > case WATCH_TASK_CLONE: > > proc_fork_connector(task, ev); > > @@ -189,16 +194,26 @@ static int cn_proc_watch_task(struct not > > break; > > } > > if (rc != NOTIFY_OK) > > return rc; > > get_seq(&msg->seq, &ev->cpu); > > - ktime_get_ts(&ev->timestamp); /* get high res monotonic timestamp */ > > + ktime_get_ts(×tamp); /* get high res monotonic timestamp */ > > + ev->timestamp_ns = ((__u64)timestamp.tv_sec*(__u64)NSEC_PER_SEC) + (__u64)timestamp.tv_nsec; > > memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); > > msg->ack = 0; /* not used */ > > msg->len = sizeof(*ev); > > cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); > > /* If cn_netlink_send() fails, drop data */ > > + > > + if (atomic_read(&proc_event_num_old_listeners) < 1) > > + return rc; > > + ev_old = (struct proc_event_deprecated*)msg->data; > > + msg->len = sizeof(*ev_old); > > A comment saying the fields cpu, what, and ack are filled above and is > valid as is would help.
OK, good idea.
> > + memmove(&ev_old->event_data, &ev->event_data, sizeof(ev->event_data)); > > + memcpy(&ev_old->timestamp, ×tamp, sizeof(timestamp)); > > + cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); > > + /* If cn_netlink_send() fails, drop data */ > > return rc; > > } > > > > static struct notifier_block __read_mostly cn_proc_nb = { > > .notifier_call = cn_proc_watch_task, > > @@ -211,20 +226,27 @@ static struct notifier_block __read_most > > */ > > static int __init cn_proc_init(void) > > { > > int err; > > > > - if ((err = cn_add_callback(&cn_proc_event_id, "cn_proc", > > - &cn_proc_mcast_ctl))) { > > - printk(KERN_WARNING "cn_proc failed to register\n"); > > - goto out; > > - } > > + err = cn_add_callback(&cn_proc_event_deprecated_id, "cn_proc_old", > > + &cn_proc_mcast_old_ctl); > > + if (err) > > + goto error_old; > > > > + err = cn_add_callback(&cn_proc_event_id, "cn_proc", NULL); > > is this needed if you are not going to have the callback ?
I believe so. Evgeniy?
> > + if (err) > > + goto error; > should not try to cn_del_callback(&cn_proc_event_id) ?! (goto error_old; > perhaps)
Ack! Good catch.
Thanks, -Matt Helsley
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |