Re: [PATCH net-next v2] extend taskstats API to support networking accounts

[Li Yu <raise.sail@gmail.com>]

于 2012年02月24日 11:05, Ben Hutchings 写道:
> On Fri, 2012-02-24 at 10:43 +0800, Li Yu wrote:
>> This patch adds L7 traffic accounting in taskstats API, so
>> the iotop like applications can receive these statistics data.
>> In fact, I also have an iotop patch for this change.
>>
>> It ignores any protocol header overhead, so results of this
>> patch should be saw as the applications-aware data statistics
>> instead of traffic statistics on wire.
>>
>> The changes of v2 are:
>>
>> 1. Move up accounting source code, so they are not protocol-aware now.
>> 2. Skipping interrupt context accounting.
> [...]
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -1735,6 +1735,18 @@ static inline int skb_copy_to_page(struct sock
>> *sk, char __user *from,
>>    	return 0;
>>    }
>>
>> +static inline void task_net_accounting_rx(unsigned int len)
>> +{
>> +	if (!in_irq()&&  len>  0)
>> +		current->rx_bytes += len;
>> +}
>> +
>> +static inline void task_net_accounting_tx(unsigned int len)
>> +{
>> +	if (!in_irq()&&  len>  0)
>> +		current->tx_bytes += len;
>> +}
> [...]
>
> These are only called from system calls, so why are you checking for IRQ
> context?
>

Some caller are exported symbols or in exported symbols path, so
I think that we'd best that do not assume that such symbols only
work at process context, it may be used by some kernel modules in
softirq context.

However, faint, the 'len' should be signed int type.


>> @@ -558,7 +559,10 @@ static inline int __sock_sendmsg_nosec(struct kiocb
>> *iocb, struct socket *sock,
>>    	si->msg = msg;
>>    	si->size = size;
>>
>> -	return sock->ops->sendmsg(iocb, sock, msg, size);
>> +	ret = sock->ops->sendmsg(iocb, sock, msg, size);
>> +	task_net_accounting_tx(ret);
>> +
>> +	return ret;
>>    }
> [...]
>
> So what happens to the totals when sendmsg() returns an error?
>
> It seems to me that the parameter type for the accounting functions
> should be ssize_t.
>

En, I think that "int" is enough here at least, the return type of 
__sock_sendmsg_nosec() also is int ...

Thanks

Yu

> Ben.
>