Re: [PATCH 3/3] staging/rdma/hfi1: Method to toggle "fast ECN" detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 05, 2015 at 12:02:25PM -0500, ira.weiny wrote:
> On Thu, Nov 05, 2015 at 10:56:27AM +0300, Dan Carpenter wrote:
> > On Wed, Nov 04, 2015 at 09:10:11PM -0500, ira.weiny@xxxxxxxxx wrote:
> > > From: Vennila Megavannan <vennila.megavannan@xxxxxxxxx>
> > > 
> > > Add a module paramter to toggle prescan/Fast ECN Detection.
> > > 
> > > In addition change the PRESCAN_RXQ Kconfig default to "yes".
> > > 
> > > Reviewed-by: Arthur Kepner<arthur.kepner@xxxxxxxxx>
> > > Reviewed-by: Mike Marciniszyn<mike.marciniszyn@xxxxxxxxx>
> > > Signed-off-by: Vennila Megavannan<vennila.megavannan@xxxxxxxxx>
> > > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> > 
> > Hm...  In the original code we had the config entry but the code to
> > support this was actually disabled.  Now we are turning on the config
> > entry by default but the module parameter defaults to disabled.  I think
> > the documentation is a bit tricky because it should say that actually
> > enabling the config is not enough, it's still disabled.
> 
> We tried to make it clear but obviously missed the mark.
> 
> > 
> > Do we really need the config to be there?  Distros are going to enable it.
> > Who is it who wants to disable the config?
> 
> The config is maintained to be able to fully remove the code to obtain the
> maximum receive performance _if_ the customer knows that they will never need
> the prescanning.  This is the _ultimate_ in performance on this path.  We
> anticipate some HPC customers who build their own kernels may want this option.
> 
> >
> > Also is it better to
> > default to off or on for this code, what are the upsides and downsides
> > of that choice?
> > 
> 
> I'll update the commit message.  But will also explain here.
> 
> Enabling the code with the config option introduces a _slight_ performance
> impact but allows the user to determine if this congestion control fix should
> be active or not at run time.  You are correct that most distros will enable
> the config options (default Yes).  Therefore this becomes a system admin
> control item for the fabric being configured.  After testing we found that this
> was the best compromise for most systems in terms of performance and
> flexibility.  This is because prescanning is not required most of the time.
> Should a customer find that their topology and/or workload requires prescanning
> they can then enable this option at run time.  This will result in a
> significant reduction of performance at the node level but may for those
> customers result in better overall fabric/cluster performance.
> 
> So in conclusion we have 3 performance levels and the combination in this patch
> puts us at the "middle one".

Can't you just make this a dynamic thing, not a build-time thing?  No
one will rebuild their kernels (distros forbid this for obvious support
reasons), so making this work "automatically" is the best thing.  Second
best is having the ability to "tune" it by hand, but you really don't
want that.  Worse case is having a build-time option, which you chose :(

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux