From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751511AbdFFUld (ORCPT ); Tue, 6 Jun 2017 16:41:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55172 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751440AbdFFUlc (ORCPT ); Tue, 6 Jun 2017 16:41:32 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B350CC0467D7 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=bcodding@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com B350CC0467D7 From: "Benjamin Coddington" To: "Jeff Layton" Cc: bfields@fieldses.org, "Alexander Viro" , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] fs/locks: Remove fl_nspid Date: Tue, 06 Jun 2017 16:41:29 -0400 Message-ID: <062370B1-1664-446F-8FDA-49289D47265E@redhat.com> In-Reply-To: <7ADF33E5-AD07-4D57-A38E-AB5FD81636FB@redhat.com> References: <1496772023.2807.14.camel@redhat.com> <1496773509.2807.16.camel@redhat.com> <7ADF33E5-AD07-4D57-A38E-AB5FD81636FB@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; format=flowed X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 06 Jun 2017 20:41:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6 Jun 2017, at 14:57, Benjamin Coddington wrote: > On 6 Jun 2017, at 14:25, Jeff Layton wrote: > >> On Tue, 2017-06-06 at 14:00 -0400, Jeff Layton wrote: >>> On Tue, 2017-06-06 at 13:19 -0400, Benjamin Coddington wrote: >>>> Since commit c69899a17ca4 "NFSv4: Update of VFS byte range lock >>>> must be >>>> atomic with the stateid update", NFSv4 has been inserting locks in >>>> rpciod >>>> worker context. The result is that the file_lock's fl_nspid is the >>>> kworker's pid instead of the original userspace pid. >>>> >>>> The fl_nspid is only used to represent the namespaced virtual pid >>>> number >>>> when displaying locks or returning from F_GETLK. There's no reason >>>> to set >>>> it for every inserted lock, since we can usually just look it up >>>> from >>>> fl_pid. So, instead of looking up and holding struct pid for every >>>> lock, >>>> let's just look up the virtual pid number from fl_pid when it is >>>> needed. >>>> That means we can remove fl_nspid entirely. >>>> >>> >>> With this set, I think we ought to codify that the stored pid must >>> be >>> relative >> >> ...to the init_pid_ns. Let's make that clear in the comments for >> filesystem authors. > > OK, but I think you mean fl_pid should always be current->tgid or the > pid as > it is in init_pid_ns. We translate that pid into the virtual pid of > the > process doing F_GETLK or reading /proc/locks. And in that sense, we shouldn't have to add a comment, because the expected behavior is the same as it is today -- namely that fl_pid is current->tgid, or the real pid number, which is the pid number in init_pid_ns. So, unless you feel strongly that this should be explained in a comment, I think I'll resend this without additional comments after making the change to use find_pid_ns(). Ben