Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper

[Mike Rapoport <rppt@linux.vnet.ibm.com>]

On Mon, Oct 30, 2017 at 07:51:42PM +1100, Balbir Singh wrote:
> On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > There are several functions that do find_task_by_vpid() followed by
> > get_task_struct(). We can use a helper function instead.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
> 
> I did a quick grep and found other similar patterns in

(reordered the file list a bit)

> kernel/events/core.c,
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
> mm/mempolicy.c,

Those and mm/migrate.c indeed have a similar pattern, but they all do

	task = pid ? find_task_by_vpid(pid) : current;

And I don't see an elegant way to use find_get_task_by_vpid() in this case.

> kernel/kcmp.c,

kcmp gets both tasks between rcu_read_lock/unlock and I think it's better
to keep it this way.

> kernel/sys.c,

There is no get_task_struct() after find_task_by_vpid(), unless I've missed
something

> kernel/time/posix-cpu-timers.c,

Here the task is selected with more complex logic than just
find_task_by_vpid() 

> mm/process_vm_access.c,

Converted in the patch

> security/yama/yama_lsm.c,
> arch/ia64/kernel/perfmon.c

I've missed these two, indeed.

The arch/ia64/kernel/perfmon.c even still uses read_lock(&tasklist) rather
than rcu_read_lock()...
 
> Balbir Singh.
> 

-- 
Sincerely yours,
Mike.