Re: [RFC DO NOT MERGE] treewide: use __xchg in most obvious places
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2023-01-10 11:09:25
Also in:
dri-devel, intel-gfx, linux-alpha, linux-arm-kernel, linux-m68k, linux-mips, linux-riscv, linux-s390, linux-sh, lkml, loongarch, sparclinux
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. ...
quoted hunk ↗ jump to hunk
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