Re: [PATCH 10/13] powerpc/rtas: improve function information lookups
From: Nathan Lynch <hidden>
Date: 2022-11-28 21:58:16
Nick Child [off-list ref] writes:
On 11/18/22 09:07, Nathan Lynch wrote:quoted
+static int __init rtas_token_to_function_xarray_init(void) +{ + int err = 0; + + for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) { + const struct rtas_function *func = &rtas_function_table[i]; + const s32 token = func->token; + + if (token == RTAS_UNKNOWN_SERVICE) + continue; + + err = xa_err(xa_store(&rtas_token_to_function_xarray, + token, (void *)func, GFP_KERNEL)); + if (err) + break; + } + + return err; +} +arch_initcall(rtas_token_to_function_xarray_init); + +static const struct rtas_function *rtas_token_to_function(s32 token) +{ + const struct rtas_function *func; + + if (WARN_ONCE(token < 0, "invalid token %d", token)) + return NULL; + + func = xa_load(&rtas_token_to_function_xarray, (unsigned long)token); +Why typecast token here and not in xa_store?
No good reason. I'll add it to the xa_store() call site.
quoted
+static void __init rtas_function_table_init(void) +{ + struct property *prop; + + for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) { + struct rtas_function *curr = &rtas_function_table[i]; + struct rtas_function *prior; + int cmp; + + curr->token = RTAS_UNKNOWN_SERVICE; + + if (i == 0) + continue; + /* + * Ensure table is sorted correctly for binary search + * on function names. + */ + prior = &rtas_function_table[i - 1]; + + cmp = strcmp(prior->name, curr->name); + if (cmp < 0) + continue; + + if (cmp == 0) { + pr_err("'%s' has duplicate function table entries\n", + curr->name); + } else { + pr_err("function table unsorted: '%s' wrongly precedes '%s'\n", + prior->name, curr->name); + } + }Just a thought, would it be simpler to use sort()? you already have the cmp_func implemented for bsearch().
It's an option, but I think a tradeoff is that we would have to sacrifice some const-ness in the data structures (i.e. remove the const qualifier from struct rtas_function's fields). And the table has to be in *some* order, so it may as well be sorted by name from the start. That said, I don't love resorting to a boot-time check for this. We could sidestep the issue by generating the C code for the table and indexes at build time, but it's hard to justify the effort when the set of RTAS functions changes very slowly over time.
As for the series as a whole: I am no RTAS expert but was able to build, boot and mess around with new tracepoints without errors: Tested-by: Nick Child <nnac123@linux.ibm.com>
Thanks for testing and reviewing!