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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
