| Message ID | 1271602224.27235.199.camel@lb-tlvb-vladz |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: "Vladislav Zolotarov" <vladz@broadcom.com> Date: Sun, 18 Apr 2010 17:50:24 +0300 > The default was changed to save memory on 32bits systems. > > Author: Dmitry Kravkov <dmitry@broadcom.com> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com> > Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com> > Signed-off-by: Eilon Greenstein <eilong@broadcom.com> This is absolutely rediculious. There is no reason in the world to restrict multiqueue to only 64-bit systems. If there is a memory allocation issue, fix that! Is it vmalloc() space usage? You don't even describe what the hell the problem is, so nobody can figure out _WHY_ you're making this change. That's so incredibly frustrating because it makes it impossible for anyone to sanely evaluate the patch. The more I read of this patch series, the more I am incredibly disappointed with the poor quality of these changes. This is the worst patch series submitted by Broadcom engieers, ever! -- 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 Sun, 2010-04-18 at 21:10 -0700, David Miller wrote: > From: "Vladislav Zolotarov" <vladz@broadcom.com> > Date: Sun, 18 Apr 2010 17:50:24 +0300 > > > The default was changed to save memory on 32bits systems. > > > > Author: Dmitry Kravkov <dmitry@broadcom.com> > > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com> > > Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com> > > Signed-off-by: Eilon Greenstein <eilong@broadcom.com> > > This is absolutely rediculious. > > There is no reason in the world to restrict multiqueue to only 64-bit > systems. > > If there is a memory allocation issue, fix that! Is it vmalloc() space > usage? Yes - running low on vmalloc is the reason. This patch does not restrict the usage of multi queue on 32bits - it is just changing the default queues values from the number of available CPUs to 1. If the user will use another value, it will work as before. We encounter few issues were a 32bit kernel was installed on a multi-core platform and the driver allocated "too many" queues. One way to go is to limit the queue size and the other is the number of queues - leaving it as is caused issues due to running low on kernel virtual memory so a change was needed. Dave, what is your preference: - re-submitting this patch with a proper description? - limit the size of each queue? - other? Thanks, Eilon -- 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: "Eilon Greenstein" <eilong@broadcom.com> Date: Mon, 19 Apr 2010 09:06:21 +0300 > Yes - running low on vmalloc is the reason. This patch does not restrict > the usage of multi queue on 32bits - it is just changing the default > queues values from the number of available CPUs to 1. If the user will > use another value, it will work as before. Changing the default to 1 is equivalent to disabling multi-queue. > We encounter few issues were a 32bit kernel was installed on a > multi-core platform and the driver allocated "too many" queues. One way > to go is to limit the queue size and the other is the number of queues - > leaving it as is caused issues due to running low on kernel virtual > memory so a change was needed. Look into using an allocation strategy other than vmalloc(), in fact I can't see why you need to much memory that vmalloc() must be used in the first place. -- 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/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c index 2eb9a3b..a35def6 100644 --- a/drivers/net/bnx2x_main.c +++ b/drivers/net/bnx2x_main.c @@ -91,10 +91,17 @@ module_param(multi_mode, int, 0); MODULE_PARM_DESC(multi_mode, " Multi queue mode " "(0 Disable; 1 Enable (default))"); +#ifdef CONFIG_64BIT static int num_queues; module_param(num_queues, int, 0); MODULE_PARM_DESC(num_queues, " Number of queues for multi_mode=1" " (default is as a number of CPUs)"); +#else +static int num_queues = 1; +module_param(num_queues, int, 0); +MODULE_PARM_DESC(num_queues, " Number of queues for multi_mode=1" + " (default 1)"); +#endif static int disable_tpa; module_param(disable_tpa, int, 0);