From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesus Sanchez-Palencia Subject: Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc Date: Mon, 23 Apr 2018 11:21:34 -0700 Message-ID: <768e8da5-502e-d36f-0f32-9324eaca4a1d@intel.com> References: <20180307011230.24001-1-jesus.sanchez-palencia@intel.com> <20180307011230.24001-14-jesus.sanchez-palencia@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, vinicius.gomes@intel.com, richardcochran@gmail.com, intel-wired-lan@lists.osuosl.org, anna-maria@linutronix.de, henrik@austad.us, john.stultz@linaro.org, levi.pearson@harman.com, edumazet@google.com, willemb@google.com, mlichvar@redhat.com To: Thomas Gleixner Return-path: Received: from mga11.intel.com ([192.55.52.93]:5935 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932154AbeDWShy (ORCPT ); Mon, 23 Apr 2018 14:37:54 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: Hi Thomas, On 03/21/2018 06:46 AM, Thomas Gleixner wrote: > On Tue, 6 Mar 2018, Jesus Sanchez-Palencia wrote: >> +struct tbs_sched_data { >> + bool sorting; >> + int clockid; >> + int queue; >> + s32 delta; /* in ns */ >> + ktime_t last; /* The txtime of the last skb sent to the netdevice. */ >> + struct rb_root head; > > Hmm. You are reimplementing timerqueue open coded. Have you checked whether > you could reuse the timerqueue implementation? > > That requires to add a timerqueue node to struct skbuff > > @@ -671,7 +671,8 @@ struct sk_buff { > unsigned long dev_scratch; > }; > }; > - struct rb_node rbnode; /* used in netem & tcp stack */ > + struct rb_node rbnode; /* used in netem & tcp stack */ > + struct timerqueue_node tqnode; > }; > struct sock *sk; > > Then you can use timerqueue_head in your scheduler data and all the open > coded rbtree handling goes away. I just noticed that doing the above increases the size of struct sk_buff by 8 bytes - struct timerqueue_node is 32bytes long while struct rb_node is only 24bytes long. Given the feedback we got here before against touching struct sk_buff at all for non-generic use cases, I will keep the implementation of sch_tbs.c as is, thus keeping the open-coded version for now, ok? Thanks, Jesus (...) From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesus Sanchez-Palencia Date: Mon, 23 Apr 2018 11:21:34 -0700 Subject: [Intel-wired-lan] [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc In-Reply-To: References: <20180307011230.24001-1-jesus.sanchez-palencia@intel.com> <20180307011230.24001-14-jesus.sanchez-palencia@intel.com> Message-ID: <768e8da5-502e-d36f-0f32-9324eaca4a1d@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: Hi Thomas, On 03/21/2018 06:46 AM, Thomas Gleixner wrote: > On Tue, 6 Mar 2018, Jesus Sanchez-Palencia wrote: >> +struct tbs_sched_data { >> + bool sorting; >> + int clockid; >> + int queue; >> + s32 delta; /* in ns */ >> + ktime_t last; /* The txtime of the last skb sent to the netdevice. */ >> + struct rb_root head; > > Hmm. You are reimplementing timerqueue open coded. Have you checked whether > you could reuse the timerqueue implementation? > > That requires to add a timerqueue node to struct skbuff > > @@ -671,7 +671,8 @@ struct sk_buff { > unsigned long dev_scratch; > }; > }; > - struct rb_node rbnode; /* used in netem & tcp stack */ > + struct rb_node rbnode; /* used in netem & tcp stack */ > + struct timerqueue_node tqnode; > }; > struct sock *sk; > > Then you can use timerqueue_head in your scheduler data and all the open > coded rbtree handling goes away. I just noticed that doing the above increases the size of struct sk_buff by 8 bytes - struct timerqueue_node is 32bytes long while struct rb_node is only 24bytes long. Given the feedback we got here before against touching struct sk_buff at all for non-generic use cases, I will keep the implementation of sch_tbs.c as is, thus keeping the open-coded version for now, ok? Thanks, Jesus (...)