| Message ID | 20081210093532.GA7926@ff.dom.local |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
Jarek Poplawski wrote: > On Wed, Dec 10, 2008 at 01:14:31AM -0800, David Miller wrote: > > >> I understand it's logical steps, but you could just do 1 and >> 2 in the same patch and it'd be OK. >> > > OK, so I withdraw 1 and 4 too, and let's say this is No 7 (2in1). > Just FYI, I'm travelling today, so I'll need until tommorrow to review this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick, I know you said you were travelling and very busy, but Jarek has been waiting patiently for you to review these two newly respun patches. These are just rotting in my patch queue and I'd like to do something with them. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 16, 2008 at 03:57:22PM -0800, David Miller wrote: > > Patrick, I know you said you were travelling and very busy, > but Jarek has been waiting patiently for you to review > these two newly respun patches. > > These are just rotting in my patch queue and I'd like to > do something with them. I guess Patrick has some doubts, but since these patches are not critical, let's say I'll resend them within a few weeks. Thanks, Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jarek Poplawski <jarkao2@gmail.com> Date: Wed, 17 Dec 2008 07:03:03 +0000 > On Tue, Dec 16, 2008 at 03:57:22PM -0800, David Miller wrote: > > > > Patrick, I know you said you were travelling and very busy, > > but Jarek has been waiting patiently for you to review > > these two newly respun patches. > > > > These are just rotting in my patch queue and I'd like to > > do something with them. > > I guess Patrick has some doubts, but since these patches are not > critical, let's say I'll resend them within a few weeks. Fair enough, I'll mark them deferred on patchwork -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller wrote: > From: Jarek Poplawski <jarkao2@gmail.com> > Date: Wed, 17 Dec 2008 07:03:03 +0000 > >> On Tue, Dec 16, 2008 at 03:57:22PM -0800, David Miller wrote: >>> Patrick, I know you said you were travelling and very busy, >>> but Jarek has been waiting patiently for you to review >>> these two newly respun patches. >>> >>> These are just rotting in my patch queue and I'd like to >>> do something with them. >> I guess Patrick has some doubts, but since these patches are not >> critical, let's say I'll resend them within a few weeks. > > Fair enough, I'll mark them deferred on patchwork Sorry, I dropped the ball on this one. I still think scheduling a work-queue or something else running in process context to kick the queue once the scheduler had a chance to run would be a better solution. But Jarek's patches are an improvement to the current situation, so no objections from me. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 12, 2009 at 07:56:37AM +0100, Patrick McHardy wrote: > David Miller wrote: >> From: Jarek Poplawski <jarkao2@gmail.com> >> Date: Wed, 17 Dec 2008 07:03:03 +0000 >> >>> On Tue, Dec 16, 2008 at 03:57:22PM -0800, David Miller wrote: >>>> Patrick, I know you said you were travelling and very busy, >>>> but Jarek has been waiting patiently for you to review >>>> these two newly respun patches. >>>> >>>> These are just rotting in my patch queue and I'd like to >>>> do something with them. >>> I guess Patrick has some doubts, but since these patches are not >>> critical, let's say I'll resend them within a few weeks. >> >> Fair enough, I'll mark them deferred on patchwork > > Sorry, I dropped the ball on this one. I still think scheduling > a work-queue or something else running in process context to > kick the queue once the scheduler had a chance to run would > be a better solution. But Jarek's patches are an improvement > to the current situation, so no objections from me. > Thanks for the review Patrick. As I wrote before, I'm not against using a workqueue here: it's logically better, but I still think this place is rather exception, so I'm not convinced we should care so much adding better solution, but also some overhead when cancelling this workqueue. But if it really bothers you, please confirm, and I'll do it. BTW, I wonder if adding the old "too many events" warning back wouldn't be more useful here. David, I'm not sure you can still track these patches, so I'll soon resend them (2 patches: #7/6 and 8/6). Thanks, Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jarek Poplawski wrote: > On Mon, Jan 12, 2009 at 07:56:37AM +0100, Patrick McHardy wrote: >> Sorry, I dropped the ball on this one. I still think scheduling >> a work-queue or something else running in process context to >> kick the queue once the scheduler had a chance to run would >> be a better solution. But Jarek's patches are an improvement >> to the current situation, so no objections from me. >> > > Thanks for the review Patrick. As I wrote before, I'm not against > using a workqueue here: it's logically better, but I still think > this place is rather exception, so I'm not convinced we should > care so much adding better solution, but also some overhead when > cancelling this workqueue. But if it really bothers you, please > confirm, and I'll do it. It doesn't bother me :) I just think its the technical better and also most likely code-wise cleaner solution to this problem. Cancellation wouldn't be necessary since an unnecessary netif_schedule() doesn't really matter. It you don't mind adding the workqueue, I certainly would prefer it, but I'm also fine with this patch. I don't have a HTB setup or a testcase for this specific case, otherwise I'd simply do it myself. > BTW, I wonder if adding the old "too many > events" warning back wouldn't be more useful here. It would be good to notify the user and also have some indication for this case when looking into bug reports. A counter or a (single) printk would both be fine. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 12, 2009 at 10:10:47AM +0000, Jarek Poplawski wrote: ... > David, I'm not sure you can still track these patches, so I'll > soon resend them (2 patches: #7/6 and 8/6). Maybe I should have mention this: there is nothing more except these 2 patches to merge from this thread. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jarek Poplawski <jarkao2@gmail.com> Date: Mon, 12 Jan 2009 10:10:47 +0000 > David, I'm not sure you can still track these patches, so I'll > soon resend them (2 patches: #7/6 and 8/6). Thanks, please do that. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 12, 2009 at 02:32:50AM -0800, David Miller wrote: > From: Jarek Poplawski <jarkao2@gmail.com> > Date: Mon, 12 Jan 2009 10:10:47 +0000 > > > David, I'm not sure you can still track these patches, so I'll > > soon resend them (2 patches: #7/6 and 8/6). > > Thanks, please do that. I hope you've received them. Thanks, Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jarek Poplawski <jarkao2@gmail.com> Date: Mon, 12 Jan 2009 10:59:03 +0000 > On Mon, Jan 12, 2009 at 02:32:50AM -0800, David Miller wrote: > > From: Jarek Poplawski <jarkao2@gmail.com> > > Date: Mon, 12 Jan 2009 10:10:47 +0000 > > > > > David, I'm not sure you can still track these patches, so I'll > > > soon resend them (2 patches: #7/6 and 8/6). > > > > Thanks, please do that. > > I hope you've received them. I did, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 12, 2009 at 11:22:47AM +0100, Patrick McHardy wrote: ... > It doesn't bother me :) I just think its the technical better > and also most likely code-wise cleaner solution to this problem. > Cancellation wouldn't be necessary since an unnecessary > netif_schedule() doesn't really matter. Hmm... Do you mean during destroying... It's probably not very long, but deleting and creating qdiscs especially for some virtual devices can take longer I guess. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jarek Poplawski wrote: > On Mon, Jan 12, 2009 at 11:22:47AM +0100, Patrick McHardy wrote: > ... >> It doesn't bother me :) I just think its the technical better >> and also most likely code-wise cleaner solution to this problem. >> Cancellation wouldn't be necessary since an unnecessary >> netif_schedule() doesn't really matter. > > Hmm... Do you mean during destroying... It's probably not very long, > but deleting and creating qdiscs especially for some virtual devices > can take longer I guess. I was referring to your statement "but also some overhead when cancelling this workqueue" and assumed you meant during packet processing. In the destruction path it really doesn't matter at all. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 5070643..9ca8a26 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -685,8 +685,8 @@ static psched_time_t htb_do_events(struct htb_sched *q, int level) if (cl->cmode != HTB_CAN_SEND) htb_add_to_wait_tree(q, cl, diff); } - /* too much load - let's continue on next jiffie */ - return q->now + PSCHED_TICKS_PER_SEC / HZ; + /* too much load - let's continue on next jiffie (including above) */ + return q->now + 2 * PSCHED_TICKS_PER_SEC / HZ; } /* Returns class->node+prio from id-tree where classe's id is >= id. NULL @@ -873,7 +873,7 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch) } else event = q->near_ev_cache[level]; - if (event && next_event > event) + if (next_event > event) next_event = event; m = ~q->row_mask[level];