Message ID | 20111201165018.GA19451@tugrik.mns.mnsspb.ru |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
From: Kirill Smelkov <kirr@mns.spb.ru> Date: Thu, 1 Dec 2011 20:50:18 +0400 > One "regression" is it is now not possible to disable BQL at compile time, > because CONFIG_BQL can't be set to "n" via usual ways. > > Description and patch below. Thanks. It's intentional, and your patch will not be applied. The Kconfig entry and option is merely for expressing internal dependencies, not for providing a way to disable the code. -- 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 Thu, Dec 01, 2011 at 01:00:45PM -0500, David Miller wrote: > From: Kirill Smelkov <kirr@mns.spb.ru> > Date: Thu, 1 Dec 2011 20:50:18 +0400 > > > One "regression" is it is now not possible to disable BQL at compile time, > > because CONFIG_BQL can't be set to "n" via usual ways. > > > > Description and patch below. Thanks. > > It's intentional, and your patch will not be applied. > > The Kconfig entry and option is merely for expressing internal dependencies, > not for providing a way to disable the code. I'm maybe wrong somewhere - sorry then, but why there is e.g. static inline void netdev_tx_sent_queue(struct netdev_queue *dev_queue, unsigned int bytes) { +#ifdef CONFIG_BQL + dql_queued(&dev_queue->dql, bytes); + if (unlikely(dql_avail(&dev_queue->dql) < 0)) { + set_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state); + if (unlikely(dql_avail(&dev_queue->dql) >= 0)) + clear_bit(__QUEUE_STATE_STACK_XOFF, + &dev_queue->state); + } +#endif } and that netdev_tx_sent_queue() is called on every xmit. I wanted to save cycles on my small/slow hardware and compile BQL out in case I know the system is not going to use it. I'm netdev newcomer, so sorry if I ask naive questions, but what's wrong it? Thanks, Kirill -- 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
Le vendredi 02 décembre 2011 à 15:22 +0400, Kirill Smelkov a écrit : > On Thu, Dec 01, 2011 at 01:00:45PM -0500, David Miller wrote: > > From: Kirill Smelkov <kirr@mns.spb.ru> > > Date: Thu, 1 Dec 2011 20:50:18 +0400 > > > > > One "regression" is it is now not possible to disable BQL at compile time, > > > because CONFIG_BQL can't be set to "n" via usual ways. > > > > > > Description and patch below. Thanks. > > > > It's intentional, and your patch will not be applied. > > > > The Kconfig entry and option is merely for expressing internal dependencies, > > not for providing a way to disable the code. > > I'm maybe wrong somewhere - sorry then, but why there is e.g. > > static inline void netdev_tx_sent_queue(struct netdev_queue *dev_queue, > unsigned int bytes) > { > +#ifdef CONFIG_BQL > + dql_queued(&dev_queue->dql, bytes); > + if (unlikely(dql_avail(&dev_queue->dql) < 0)) { > + set_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state); > + if (unlikely(dql_avail(&dev_queue->dql) >= 0)) > + clear_bit(__QUEUE_STATE_STACK_XOFF, > + &dev_queue->state); > + } > +#endif > } > > > and that netdev_tx_sent_queue() is called on every xmit. > > I wanted to save cycles on my small/slow hardware and compile BQL out in > case I know the system is not going to use it. I'm netdev newcomer, so > sorry if I ask naive questions, but what's wrong it? > Its something like 4 or 5 instructions (granted your NIC is BQL enabled). Really nothing compared to thousand of instructions per packet spent in the stack and driver. And BQL will benefit especially on your small/slow hardware, even if you dont believe so or know yet. BQL is probably the major new netdev functionality of the year. Of course, you are free to patch your own kernel if you desperatly need to save a few cycles per packet. -- 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 Fri, Dec 02, 2011 at 12:57:51PM +0100, Eric Dumazet wrote: > Le vendredi 02 décembre 2011 à 15:22 +0400, Kirill Smelkov a écrit : > > On Thu, Dec 01, 2011 at 01:00:45PM -0500, David Miller wrote: > > > From: Kirill Smelkov <kirr@mns.spb.ru> > > > Date: Thu, 1 Dec 2011 20:50:18 +0400 > > > > > > > One "regression" is it is now not possible to disable BQL at compile time, > > > > because CONFIG_BQL can't be set to "n" via usual ways. > > > > > > > > Description and patch below. Thanks. > > > > > > It's intentional, and your patch will not be applied. > > > > > > The Kconfig entry and option is merely for expressing internal dependencies, > > > not for providing a way to disable the code. > > > > I'm maybe wrong somewhere - sorry then, but why there is e.g. > > > > static inline void netdev_tx_sent_queue(struct netdev_queue *dev_queue, > > unsigned int bytes) > > { > > +#ifdef CONFIG_BQL > > + dql_queued(&dev_queue->dql, bytes); > > + if (unlikely(dql_avail(&dev_queue->dql) < 0)) { > > + set_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state); > > + if (unlikely(dql_avail(&dev_queue->dql) >= 0)) > > + clear_bit(__QUEUE_STATE_STACK_XOFF, > > + &dev_queue->state); > > + } > > +#endif > > } > > > > > > and that netdev_tx_sent_queue() is called on every xmit. > > > > I wanted to save cycles on my small/slow hardware and compile BQL out in > > case I know the system is not going to use it. I'm netdev newcomer, so > > sorry if I ask naive questions, but what's wrong it? > > > > Its something like 4 or 5 instructions (granted your NIC is BQL > enabled). Really nothing compared to thousand of instructions per packet > spent in the stack and driver. > > And BQL will benefit especially on your small/slow hardware, even if you > dont believe so or know yet. > > BQL is probably the major new netdev functionality of the year. > > Of course, you are free to patch your own kernel if you desperatly need > to save a few cycles per packet. Thanks for describing this Eric. I'll try BQL and see how it goes as you suggest. Please note that the confusion here could be avoided if there was a tiny bit of documentation. Thanks again, Kirill -- 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/Kconfig b/net/Kconfig index 2d99873..c120631 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -240,7 +240,7 @@ config NETPRIO_CGROUP a per-interface basis config BQL - boolean + boolean "Byte Queue Limits" depends on SYSFS select DQL default y