On Sun, Dec 08, 2019 at 07:33:37PM -0800, Haren Myneni wrote:
> +static void notify_process(pid_t pid, u64 fault_addr)
> +{
> + int rc;
> + struct kernel_siginfo info;
> +
> + memset(&info, 0, sizeof(info));
> +
> + info.si_signo = SIGSEGV;
> + info.si_errno = EFAULT;
> + info.si_code = SEGV_MAPERR;
> + info.si_addr = (void *)fault_addr;
> + /*
> + * process will be polling on csb.flags after request is sent to
> + * NX. So generally CSB update should not fail except when an
> + * application does not follow the process properly. So an error
> + * message will be displayed and leave it to user space whether
> + * to ignore or handle this signal.
> + */
> + rcu_read_lock();
> + rc = kill_pid_info(SIGSEGV, &info, find_vpid(pid));
> + rcu_read_unlock();
> +
> + pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__, pid, rc);
> +}
I think you want to pass in the struct pid * here instead of looking
up again, given that..
> + if (tsk) {
> + if (tsk->flags & PF_EXITING)
> + task_exit = 1;
> + put_task_struct(tsk);
> + pid = vas_window_pid(window);
We already have the struct pid in the window structure here.
> + } else {
> + pid = window->tgid;
> +
> + rcu_read_lock();
> + tsk = find_task_by_vpid(pid);
> + if (!tsk) {
.. and could have easily stored on here. Or at least only do the
look up once, given that already looks it up.
> + /* Do not notify if the task is exiting. */
> + if (!task_exit) {
> + pr_err("Invalid CSB address 0x%p signalling pid(%d)\n",
> + csb_addr, pid);
> + notify_process(pid, (u64)csb_addr);
> + }
I suspect inlining notify_process and just existing early for the
task_exit case also makes the code a bit easier to follow.
