On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:
This patch tries to show usability of __xchg helper.
It is not intended to be merged, but I can convert
it to proper patchset if necessary.
There are many more places where __xchg can be used.
This demo shows the most spectacular cases IMHO:
- previous value is returned from function,
- temporary variables are in use.
As a result readability is much better and diffstat is quite
nice, less local vars to look at.
In many cases whole body of functions is replaced
with __xchg(ptr, val), so as further refactoring the whole
function can be removed and __xchg can be called directly.
...
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long orig_ret_vaddr;
-
- orig_ret_vaddr = regs->ARM_lr;
- /* Replace the return addr with trampoline addr */
- regs->ARM_lr = trampoline_vaddr;
- return orig_ret_vaddr;
+ return __xchg(®s->ARM_lr, trampoline_vaddr);
}
If it's not a callback, the entire function can be killed.
And this is a good example of the function usage.
OTOH, these places might have a side effect (if it's in deep CPU
handlers), means we need to do this carefully.
...
static inline void *qed_chain_produce(struct qed_chain *p_chain)
{
- void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
+ void *p_prod_idx, *p_prod_page_idx;
if (is_chain_u16(p_chain)) {
if ((p_chain->u.chain16.prod_idx &
@@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain *p_chain)
p_chain->u.chain32.prod_idx++;
}
- p_ret = p_chain->p_prod_elem;
- p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
- p_chain->elem_size);
-
- return p_ret;
+ return __xchg(&p_chain->p_prod_elem,
+ (void *)(((u8 *)p_chain->p_prod_elem) + p_chain->elem_size));
Wondering if you still need a (void *) casting after the change. Ditto for the
rest of similar cases.
}
...
Btw, is it done by coccinelle? If no, why not providing the script?
--
With Best Regards,
Andy Shevchenko
