From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95F02C54FD0 for ; Mon, 27 Apr 2020 11:57:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7353F20644 for ; Mon, 27 Apr 2020 11:57:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726899AbgD0L47 (ORCPT ); Mon, 27 Apr 2020 07:56:59 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:42662 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726260AbgD0L46 (ORCPT ); Mon, 27 Apr 2020 07:56:58 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jT2Nc-0005LR-I5; Mon, 27 Apr 2020 05:56:56 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jT2Nb-000812-3T; Mon, 27 Apr 2020 05:56:56 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Thomas Gleixner Cc: LKML , Linux FS Devel , Alexey Dobriyan , Alexey Gladkov , Andrew Morton , Alexey Gladkov , Linus Torvalds , Oleg Nesterov , "Paul E. McKenney" References: <20200419141057.621356-1-gladkov.alexey@gmail.com> <87ftcv1nqe.fsf@x220.int.ebiederm.org> <87wo66vvnm.fsf_-_@x220.int.ebiederm.org> <20200424173927.GB26802@redhat.com> <87mu6ymkea.fsf_-_@x220.int.ebiederm.org> <87h7x6mj6h.fsf_-_@x220.int.ebiederm.org> <87368ps1ql.fsf@nanos.tec.linutronix.de> Date: Mon, 27 Apr 2020 06:53:42 -0500 In-Reply-To: <87368ps1ql.fsf@nanos.tec.linutronix.de> (Thomas Gleixner's message of "Mon, 27 Apr 2020 11:43:14 +0200") Message-ID: <87zhaxi1q1.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1jT2Nb-000812-3T;;;mid=<87zhaxi1q1.fsf@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+es43Xm7YLhAh5+wwr1PPIMlgGfoou5is= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3 1/6] posix-cpu-timers: Always call __get_task_for_clock holding rcu_read_lock X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thomas Gleixner writes: > ebiederm@xmission.com (Eric W. Biederman) writes: > >> This allows the getref flag to be removed and the callers can >> than take a task reference if needed. > > That changelog lacks any form of information why this should be > changed. I can see the point vs. patch 2, but pretty please put coherent > explanations into each patch. Well excess flags bad. But in this case I can do even better. The code no longer takes a reference on task_struct so this patch removes unnecessary code. I will see if I can say that better. >> Signed-off-by: "Eric W. Biederman" >> --- >> kernel/time/posix-cpu-timers.c | 41 +++++++++++++++++----------------- >> 1 file changed, 21 insertions(+), 20 deletions(-) >> >> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c >> index 2fd3b3fa68bf..eba41c70f0f0 100644 >> --- a/kernel/time/posix-cpu-timers.c >> +++ b/kernel/time/posix-cpu-timers.c >> @@ -86,36 +86,34 @@ static struct task_struct *lookup_task(const pid_t pid, bool thread, >> } >> >> static struct task_struct *__get_task_for_clock(const clockid_t clock, >> - bool getref, bool gettime) >> + bool gettime) >> { >> const bool thread = !!CPUCLOCK_PERTHREAD(clock); >> const pid_t pid = CPUCLOCK_PID(clock); >> - struct task_struct *p; >> >> if (CPUCLOCK_WHICH(clock) >= CPUCLOCK_MAX) >> return NULL; >> >> - rcu_read_lock(); >> - p = lookup_task(pid, thread, gettime); >> - if (p && getref) >> - get_task_struct(p); >> - rcu_read_unlock(); >> - return p; >> + return lookup_task(pid, thread, gettime); >> } >> >> static inline struct task_struct *get_task_for_clock(const clockid_t clock) >> { >> - return __get_task_for_clock(clock, true, false); >> + return __get_task_for_clock(clock, false); >> } >> >> static inline struct task_struct *get_task_for_clock_get(const clockid_t clock) >> { >> - return __get_task_for_clock(clock, true, true); >> + return __get_task_for_clock(clock, true); >> } >> >> static inline int validate_clock_permissions(const clockid_t clock) >> { >> - return __get_task_for_clock(clock, false, false) ? 0 : -EINVAL; >> + int ret; > > New line between declarations and code please. > >> + rcu_read_lock(); >> + ret = __get_task_for_clock(clock, false) ? 0 : -EINVAL; >> + rcu_read_unlock(); >> + return ret; >> } > > Thanks, > > tglx