On Mon, 23 Apr 2018, Jesus Sanchez-Palencia wrote:
> 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?
The size of sk_buff is 216 and the size of sk_buff_fclones is 440
bytes. The sk_buff and sk_buff_fclones kmem_caches use objects sized 256
and 512 bytes because the kmem_caches are created with SLAB_HWCACHE_ALIGN.
So adding 8 bytes to spare duplicated code will not change the kmem_cache
object size and I really doubt that anyone will notice.
Thanks,
tglx
