Don't populate the read-only array attr on the stack at run time, instead make it static const.
Signed-off-by: Colin Ian King coking@nvidia.com --- drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP; - u8 attr[] = { + static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
-Sumit
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
- u8 attr[] = {
- static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
-- 2.51.0
This looks like a micro-optimization, const makes the lookup array explicitly immutable, and static keeps it out of the stack frame, avoiding per-call initialization.
Is there a style preference for read only lookup arrays here, e.g. Should these variables remain local but not static, or should they be moved to file scope static const?
On Mon, Jan 12, 2026 at 3:26 AM Sumit Garg sumit.garg@kernel.org wrote:
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
-Sumit
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
u8 attr[] = {
static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,-- 2.51.0
I added this change for several reasons, it's read-only so making it const is always sensible. Making it static means we don't get the compiler to generate object code that stores the array on the stack (each time it's called) and then index into it, instead the data is put into a read-only data section at compile timne and the object code just fetched data from this array.
Basically, if it's read-only it's good to make it const and it's not in an __init section then you may as well make it static const. It's scoped inside the function, even though it's in the data section.
________________________________ From: ryan foster foster.ryan.r@gmail.com Sent: 12 January 2026 14:46 To: Sumit Garg sumit.garg@kernel.org Cc: Colin King coking@nvidia.com; Jens Wiklander jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org op-tee@lists.trustedfirmware.org; kernel-janitors@vger.kernel.org kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org Subject: Re: [PATCH][next] optee: make read-only array attr static const
This looks like a micro-optimization, const makes the lookup array explicitly immutable, and static keeps it out of the stack frame, avoiding per-call initialization.
Is there a style preference for read only lookup arrays here, e.g. Should these variables remain local but not static, or should they be moved to file scope static const?
On Mon, Jan 12, 2026 at 3:26 AM Sumit Garg sumit.garg@kernel.org wrote:
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
-Sumit
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
u8 attr[] = {
static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,-- 2.51.0
On Mon, Jan 12, 2026 at 03:24:49PM +0000, Colin King wrote:
I added this change for several reasons, it's read-only so making it const is always sensible. Making it static means we don't get the compiler to generate object code that stores the array on the stack (each time it's called) and then index into it, instead the data is put into a read-only data section at compile timne and the object code just fetched data from this array.
Basically, if it's read-only it's good to make it const and it's not in an __init section then you may as well make it static const. It's scoped inside the function, even though it's in the data section.
That's fair. I don't have a strict preference here. Feel free to add:
Reviewed-by: Sumit Garg sumit.garg@oss.qualcomm.com
-Sumit
From: ryan foster foster.ryan.r@gmail.com Sent: 12 January 2026 14:46 To: Sumit Garg sumit.garg@kernel.org Cc: Colin King coking@nvidia.com; Jens Wiklander jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org op-tee@lists.trustedfirmware.org; kernel-janitors@vger.kernel.org kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org Subject: Re: [PATCH][next] optee: make read-only array attr static const
This looks like a micro-optimization, const makes the lookup array explicitly immutable, and static keeps it out of the stack frame, avoiding per-call initialization.
Is there a style preference for read only lookup arrays here, e.g. Should these variables remain local but not static, or should they be moved to file scope static const?
On Mon, Jan 12, 2026 at 3:26 AM Sumit Garg sumit.garg@kernel.org wrote:
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
-Sumit
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
u8 attr[] = {
static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,-- 2.51.0
On Fri, Jan 16, 2026 at 10:14 AM Sumit Garg sumit.garg@kernel.org wrote:
On Mon, Jan 12, 2026 at 03:24:49PM +0000, Colin King wrote:
I added this change for several reasons, it's read-only so making it const is always sensible. Making it static means we don't get the compiler to generate object code that stores the array on the stack (each time it's called) and then index into it, instead the data is put into a read-only data section at compile timne and the object code just fetched data from this array.
Basically, if it's read-only it's good to make it const and it's not in an __init section then you may as well make it static const. It's scoped inside the function, even though it's in the data section.
That's fair. I don't have a strict preference here. Feel free to add:
Reviewed-by: Sumit Garg sumit.garg@oss.qualcomm.com
I'm picking up this.
Thanks, Jens
-Sumit
From: ryan foster foster.ryan.r@gmail.com Sent: 12 January 2026 14:46 To: Sumit Garg sumit.garg@kernel.org Cc: Colin King coking@nvidia.com; Jens Wiklander jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org op-tee@lists.trustedfirmware.org; kernel-janitors@vger.kernel.org kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org Subject: Re: [PATCH][next] optee: make read-only array attr static const
This looks like a micro-optimization, const makes the lookup array explicitly immutable, and static keeps it out of the stack frame, avoiding per-call initialization.
Is there a style preference for read only lookup arrays here, e.g. Should these variables remain local but not static, or should they be moved to file scope static const?
On Mon, Jan 12, 2026 at 3:26 AM Sumit Garg sumit.garg@kernel.org wrote:
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
-Sumit
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
u8 attr[] = {
static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,-- 2.51.0
On Mon, Jan 12, 2026 at 04:56:10PM +0530, Sumit Garg wrote:
On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
Is there any value add to do this? AFAIK, the static local variables aren't preffered.
I have never heard that static local variables aren't prefered. I imagine that because they're often not thread safe? But if they're const thread safeness isn't an problem. Generally, we prefer the scope as local as possible.
regards, dan carpenter
On Fri, 9 Jan 2026 15:44:42 +0000 Colin Ian King coking@nvidia.com wrote:
Don't populate the read-only array attr on the stack at run time, instead make it static const.
For a 4 byte array initialising on stack is likely to generate faster code. In particular it avoid a very likely data-cache miss for the rodata. The on-stack array access might even shorter code than accessing a global array on some architectures. The compiler might, of course, decide to unroll the loop and not instantiate the array at all (in either case). Indeed, just unrolling the loop as: if (params[0].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT) goto bad; if (params[1].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT) goto bad; if (params[2].attr != TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT) goto bad; if (params[3].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT) goto bad; is likely to be pretty near optimal. (Collecting the bytes and doing a single 32bit compare may be better.)
Of course this won't be a massively hot path - so the code choice won't make a measurable difference.
David
Signed-off-by: Colin Ian King coking@nvidia.com
drivers/tee/optee/rpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 97fc5b14db0c..1758eb7e6e8b 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP;
- u8 attr[] = {
- static const u8 attr[] = { TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT, TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
op-tee@lists.trustedfirmware.org
-
Colin Ian King -
Colin King
-
Dan Carpenter -
David Laight -
Jens Wiklander -
ryan foster -
Sumit Garg