Patchwork [1/1] Add config option for batched hcalls

login
register
mail settings
Submitter Will Schmidt
Date Sept. 24, 2010, 9:44 p.m.
Message ID <1285364655.3016.25.camel@lexx>
Download mbox | patch
Permalink /patch/65699/
State Rejected
Headers show

Comments

Will Schmidt - Sept. 24, 2010, 9:44 p.m.
Add a config option for the (batched) MULTITCE and BULK_REMOVE h-calls.

By default, these options are on and are beneficial for performance and
throughput reasons.   If disabled, the code will fall back to using less
optimal TCE and REMOVE hcalls.   The ability to easily disable these
options is useful for some of the PREEMPT_RT related investigation and
work occurring on Power.


Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>
cc: Anton Blanchard <anton@samba.org>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

---
 /* Build up the firmware features bitmask using the contents of
Olof Johansson - Sept. 26, 2010, 3:49 a.m.
On Fri, Sep 24, 2010 at 04:44:15PM -0500, Will Schmidt wrote:
> 
> Add a config option for the (batched) MULTITCE and BULK_REMOVE h-calls.
> 
> By default, these options are on and are beneficial for performance and
> throughput reasons.   If disabled, the code will fall back to using less
> optimal TCE and REMOVE hcalls.   The ability to easily disable these
> options is useful for some of the PREEMPT_RT related investigation and
> work occurring on Power.

Hi,

I can see why it's useful to enable and disable, but these are all
runtime-checked, wouldn't it be more useful to add a bootarg to handle
it instead of adding some new config options that pretty much everyone
will always go with the defaults on?

The bits are set early, but from looking at where they're used, there
doesn't seem to be any harm in disabling them later on when a bootarg
is convenient to parse and deal with?

It has the benefit of easier on/off testing, if that has any value for
production debug down the road.


-Olof
Will Schmidt - Sept. 27, 2010, 8:06 p.m.
On Sat, 2010-09-25 at 22:49 -0500, Olof Johansson wrote:
> On Fri, Sep 24, 2010 at 04:44:15PM -0500, Will Schmidt wrote:
> > 
> > Add a config option for the (batched) MULTITCE and BULK_REMOVE h-calls.
> > 
> > By default, these options are on and are beneficial for performance and
> > throughput reasons.   If disabled, the code will fall back to using less
> > optimal TCE and REMOVE hcalls.   The ability to easily disable these
> > options is useful for some of the PREEMPT_RT related investigation and
> > work occurring on Power.
> 
> Hi,
> 
> I can see why it's useful to enable and disable, but these are all
> runtime-checked, wouldn't it be more useful to add a bootarg to handle
> it instead of adding some new config options that pretty much everyone
> will always go with the defaults on?
> 
> The bits are set early, but from looking at where they're used, there
> doesn't seem to be any harm in disabling them later on when a bootarg
> is convenient to parse and deal with?
> 
> It has the benefit of easier on/off testing, if that has any value for
> production debug down the road.

Hi Olof, 
  Thats a good idea, let me poke at this a bit more, see if I can get
bootargs for this.  

Thanks, 
-Will

> 
> 
> -Olof
>

Patch

diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f0e6f28..0b5e6a9 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -81,3 +81,23 @@  config DTL
 	  which are accessible through a debugfs file.
 
 	  Say N if you are unsure.
+
+config BULK_REMOVE
+	bool "Enable BULK_REMOVE"
+	depends on PPC_PSERIES
+	default y
+	help
+	  Enable the BULK_REMOVE option for the hash page code.
+	  This relies on a "hcall-bulk" firmware feature, and
+	  should be enabled for performance throughput.
+
+config MULTITCE
+	bool "Enable MultiTCE"
+	depends on PPC_PSERIES
+	default y
+	help
+	  Enable the Multi-TCE code, allowing a single hcall to
+	  update multiple TCE entries at one time.  This relies
+	  on a "hcall-multi-tce" firmware feature, and should be
+	  enabled for performance throughput.
+
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 0a4d8c..4327064 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -51,9 +51,13 @@  firmware_features_table[FIRMWARE_MAX_FEATURES] = {
 	{FW_FEATURE_VIO,		"hcall-vio"},
 	{FW_FEATURE_RDMA,		"hcall-rdma"},
 	{FW_FEATURE_LLAN,		"hcall-lLAN"},
+#if defined(CONFIG_BULK_REMOVE)
 	{FW_FEATURE_BULK_REMOVE,	"hcall-bulk"},
+#endif
 	{FW_FEATURE_XDABR,		"hcall-xdabr"},
+#if defined(CONFIG_MULTITCE)
 	{FW_FEATURE_MULTITCE,		"hcall-multi-tce"},
+#endif
 	{FW_FEATURE_SPLPAR,		"hcall-splpar"},
 };