Re: [PATCH 0/11] LTTng-core (basic tracing infrastructure) 0.5.108

From: Ingo Molnar
Date: Thu Sep 14 2006 - 18:31:55 EST



* Michel Dagenais <michel.dagenais@xxxxxxxxxx> wrote:

> > the question is: what is more maintainance, hundreds of static
> > tracepoints (with long parameter lists) all around the (core) kernel, or
> > hundreds of detached dynamic rules that need an update every now and
> > then? [but of which most would still be usable even if some of them
> > "broke"] To me the answer is clear: having hundreds of tracepoints
> > _within_ the source code is higher cost. But please prove me wrong :-)
>
> Actually I rarely find that any of the 70 000 printk is such a huge
> nuisance to code readability. They may even help understand what is
> going on in a code area you are less familiar with.

i disagree. Consider the following example from LTT:

int sock_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct kiocb iocb;
struct sock_iocb siocb;
int ret;

trace_socket_sendmsg(sock, sock->sk->sk_family,
sock->sk->sk_type,
sock->sk->sk_protocol,
size);

init_sync_kiocb(&iocb, NULL);
iocb.private = &siocb;
ret = __sock_sendmsg(&iocb, sock, msg, size);
if (-EIOCBQUEUED == ret)
ret = wait_on_sync_kiocb(&iocb);
return ret;
}

what do the 5 extra lines introduced by trace_socket_sendmsg() tell us?
Nothing. They mostly just duplicate the information i already have from
the function declaration. They obscure the clear view of the function:

int sock_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct kiocb iocb;
struct sock_iocb siocb;
int ret;

init_sync_kiocb(&iocb, NULL);
iocb.private = &siocb;
ret = __sock_sendmsg(&iocb, sock, msg, size);
if (-EIOCBQUEUED == ret)
ret = wait_on_sync_kiocb(&iocb);
return ret;
}

the resulting visual and structural redundancy hurts.

Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/