| Message ID | 20190920133115.12802-1-uwe@kleine-koenig.org |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | dimlib: make DIMLIB a hidden symbol | expand |
On 9/20/2019 4:31 PM, Uwe Kleine-König wrote: > According to Tal Gilboa the only benefit from DIM comes from a driver > that uses it. So it doesn't make sense to make this symbol user visible, > instead all drivers that use it should select it (as is already the case > AFAICT). > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > --- > lib/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/lib/Kconfig b/lib/Kconfig > index cc04124ed8f7..9fe8a21fd183 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -555,8 +555,7 @@ config SIGNATURE > Implementation is done using GnuPG MPI library > > config DIMLIB > - bool "DIM library" > - default y > + bool > help > Dynamic Interrupt Moderation library. > Implements an algorithm for dynamically change CQ moderation values > There's a pending series using DIM which didn't add the select clause [1]. Arthur, FYI. Other than that LGTM. [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg314304.html
On 9/20/19 10:02 AM, Tal Gilboa wrote: > On 9/20/2019 4:31 PM, Uwe Kleine-König wrote: >> According to Tal Gilboa the only benefit from DIM comes from a driver >> that uses it. So it doesn't make sense to make this symbol user visible, >> instead all drivers that use it should select it (as is already the case >> AFAICT). >> >> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >> --- >> lib/Kconfig | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/lib/Kconfig b/lib/Kconfig >> index cc04124ed8f7..9fe8a21fd183 100644 >> --- a/lib/Kconfig >> +++ b/lib/Kconfig >> @@ -555,8 +555,7 @@ config SIGNATURE >> Implementation is done using GnuPG MPI library >> >> config DIMLIB >> - bool "DIM library" >> - default y >> + bool >> help >> Dynamic Interrupt Moderation library. >> Implements an algorithm for dynamically change CQ moderation values >> > There's a pending series using DIM which didn't add the select clause > [1]. Arthur, FYI. Other than that LGTM. That's easy enough to fix. > [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg314304.html for the patch: Acked-by: Randy Dunlap <rdunlap@infradead.org>
Hello, On 9/20/19 7:02 PM, Tal Gilboa wrote: > On 9/20/2019 4:31 PM, Uwe Kleine-König wrote: >> diff --git a/lib/Kconfig b/lib/Kconfig >> index cc04124ed8f7..9fe8a21fd183 100644 >> --- a/lib/Kconfig >> +++ b/lib/Kconfig >> @@ -555,8 +555,7 @@ config SIGNATURE >> Implementation is done using GnuPG MPI library >> >> config DIMLIB >> - bool "DIM library" >> - default y >> + bool >> help >> Dynamic Interrupt Moderation library. >> Implements an algorithm for dynamically change CQ moderation values >> > There's a pending series using DIM which didn't add the select clause > [1]. Arthur, FYI. Other than that LGTM. IMHO this should be fixed, as otherwise the config with the new code enabled and DIMLIB disabled will fail to build also without my change. Best regards Uwe
On Fri, 20 Sep 2019 15:31:15 +0200, Uwe Kleine-König wrote: > According to Tal Gilboa the only benefit from DIM comes from a driver > that uses it. So it doesn't make sense to make this symbol user visible, > instead all drivers that use it should select it (as is already the case > AFAICT). > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > --- > lib/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/lib/Kconfig b/lib/Kconfig > index cc04124ed8f7..9fe8a21fd183 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -555,8 +555,7 @@ config SIGNATURE > Implementation is done using GnuPG MPI library > > config DIMLIB > - bool "DIM library" > - default y > + bool > help > Dynamic Interrupt Moderation library. > Implements an algorithm for dynamically change CQ moderation values Hi Uwe! Looks like in the net tree there is a spelling mistake and moderation is spelled "modertion": https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/tree/lib/Kconfig#n562 I'm not seeing any patch to fix that anywhere, is it possible you have some local change in your tree?
Hello Jakub, On 9/22/19 5:06 AM, Jakub Kicinski wrote: > On Fri, 20 Sep 2019 15:31:15 +0200, Uwe Kleine-König wrote: >> According to Tal Gilboa the only benefit from DIM comes from a driver >> that uses it. So it doesn't make sense to make this symbol user visible, >> instead all drivers that use it should select it (as is already the case >> AFAICT). >> >> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >> --- >> lib/Kconfig | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/lib/Kconfig b/lib/Kconfig >> index cc04124ed8f7..9fe8a21fd183 100644 >> --- a/lib/Kconfig >> +++ b/lib/Kconfig >> @@ -555,8 +555,7 @@ config SIGNATURE >> Implementation is done using GnuPG MPI library >> >> config DIMLIB >> - bool "DIM library" >> - default y >> + bool >> help >> Dynamic Interrupt Moderation library. >> Implements an algorithm for dynamically change CQ moderation values > > Hi Uwe! Looks like in the net tree there is a spelling mistake and > moderation is spelled "modertion": > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/tree/lib/Kconfig#n562 > > I'm not seeing any patch to fix that anywhere, is it possible you have > some local change in your tree? I sent a patch[1] for that typo, but at that time wasn't aware that DIMLIB was relevant for net and so didn't Cc the netdev. Best regards Uwe [1] https://lore.kernel.org/lkml/20190919210314.22110-1-uwe@kleine-koenig.org/
From: Uwe Kleine-König <uwe@kleine-koenig.org> Date: Fri, 20 Sep 2019 15:31:15 +0200 > According to Tal Gilboa the only benefit from DIM comes from a driver > that uses it. So it doesn't make sense to make this symbol user visible, > instead all drivers that use it should select it (as is already the case > AFAICT). > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> Since this doesn't apply due to the moderation typo being elsewhere, I'd really like you to fix up this submission to properly be against 'net'. Thank you.
> -----Original Message----- > From: Tal Gilboa <talgi@mellanox.com> > Sent: Friday, September 20, 2019 8:02 PM > To: Uwe Kleine-König <uwe@kleine-koenig.org>; Saeed Mahameed > <saeedm@mellanox.com>; Kiyanovski, Arthur <akiyano@amazon.com> > Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org > Subject: Re: [PATCH] dimlib: make DIMLIB a hidden symbol > > On 9/20/2019 4:31 PM, Uwe Kleine-König wrote: > > According to Tal Gilboa the only benefit from DIM comes from a driver > > that uses it. So it doesn't make sense to make this symbol user visible, > > instead all drivers that use it should select it (as is already the case > > AFAICT). > > > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > > --- > > lib/Kconfig | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > index cc04124ed8f7..9fe8a21fd183 100644 > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -555,8 +555,7 @@ config SIGNATURE > > Implementation is done using GnuPG MPI library > > > > config DIMLIB > > - bool "DIM library" > > - default y > > + bool > > help > > Dynamic Interrupt Moderation library. > > Implements an algorithm for dynamically change CQ moderation > values > > > There's a pending series using DIM which didn't add the select clause > [1]. Arthur, FYI. Other than that LGTM. > > [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg314304.html Thanks Tal, I missed that.
Hi Uwe, Tal, On Sat, Sep 21, 2019 at 7:50 PM Uwe Kleine-König <uwe@kleine-koenig.org> wrote: > According to Tal Gilboa the only benefit from DIM comes from a driver > that uses it. So it doesn't make sense to make this symbol user visible, > instead all drivers that use it should select it (as is already the case > AFAICT). > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> Thanks for your patch! > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -555,8 +555,7 @@ config SIGNATURE > Implementation is done using GnuPG MPI library > > config DIMLIB > - bool "DIM library" > - default y > + bool > help > Dynamic Interrupt Moderation library. > Implements an algorithm for dynamically change CQ moderation values Thanks for fixing the first issue! The second issue is still present: NET_VENDOR_BROADCOM (which defaults to y, as all other NET_VENDOR_* symbols) should only be a gatekeeper for the various Broadcom network driver config options, and should not select DIMLIB. Cfr. my earlier complaint in https://lore.kernel.org/linux-rdma/alpine.DEB.2.21.1907021810220.13058@ramsan.of.borg/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/lib/Kconfig b/lib/Kconfig index cc04124ed8f7..9fe8a21fd183 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -555,8 +555,7 @@ config SIGNATURE Implementation is done using GnuPG MPI library config DIMLIB - bool "DIM library" - default y + bool help Dynamic Interrupt Moderation library. Implements an algorithm for dynamically change CQ moderation values
According to Tal Gilboa the only benefit from DIM comes from a driver that uses it. So it doesn't make sense to make this symbol user visible, instead all drivers that use it should select it (as is already the case AFAICT). Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> --- lib/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)