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=-3.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 1F0C2C433E1 for ; Tue, 18 Aug 2020 00:08:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EFB7F20789 for ; Tue, 18 Aug 2020 00:08:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597709332; bh=jNXhqR+ASYKuYB0jNz/5hl9gOu/ZDZOHBscoCNS1+tM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=BhaxAD8SccsXNcOfVDNAqckIzImoKa5f0wqx51Q/7CdbIvQEViPEbCjgdDsQmGhsL 85AoeITuXSnJ87c7qUGbAuokOA9Vu4I98q0c22i4iX78r/XWiYYm8sQxQ/lJLZYYFe luArySh14Yvp25aSXjQFAfgHFkZCpS8OeFlbzKhA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726697AbgHRAIv (ORCPT ); Mon, 17 Aug 2020 20:08:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58268 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726473AbgHRAIt (ORCPT ); Mon, 17 Aug 2020 20:08:49 -0400 Received: from mail-lf1-x143.google.com (mail-lf1-x143.google.com [IPv6:2a00:1450:4864:20::143]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30D57C061342 for ; Mon, 17 Aug 2020 17:08:47 -0700 (PDT) Received: by mail-lf1-x143.google.com with SMTP id h8so9279455lfp.9 for ; Mon, 17 Aug 2020 17:08:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7PUubmKG0qCUbLC0lny1sEZMsh6oDctj94eQbjDTgwk=; b=Cc93yVMbx3RvLnq5PLotBg+roGGu69+/Dx9uDZdnFaQvTrkIzdLU5lhrEDOPiq2+fN 5E+MIu8qQiaA07jjpGceLS5/FUXItwiiQf3jrnqL+6nxCyP1OYWmoOUBc1tR+82IoQ3t pB1HfO6+D8i21tTtZuIWUIjxWkjP3RCPMcq/k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7PUubmKG0qCUbLC0lny1sEZMsh6oDctj94eQbjDTgwk=; b=jIs1r9evR+44RAbAZ8/VyQYNpS6JutRgWqqx3Q7SwD4BUFmfFVH2yqRL9iGvLopPTX lwR45qlcpt+vFiafg3OqhT9BPgnN2AGfoX/8aPz6GlX5en4o4pRUlPQNeOIYbeL8hifp ULPix16z4WHNWOp4oksFqqpOcFOnB3hhiygT3MOl0AZ8YUZWHTILmwKV78PiXke6oI/s NguOz7QKfi9wVmzFDPKt9Jkb4Nc2CM7CVsAHAMCI82dzE0D+FvlOv1Akf7XMSvQ8a40N f8cbxL2bI5u+7nSvtr21P/kM3eW44j4AmcpooY8LES+au6ehkMfcVxiZiTBwsaTeOxxm 4wig== X-Gm-Message-State: AOAM530A7ODozDBko8TSNiSdkLOC/tmnEB5Y9TEfzgx5p3UTauGQYtVQ 5xzrMOMxHxr6PnPM3mtejIBYtHa3uDKe6w== X-Google-Smtp-Source: ABdhPJylLBiCu5wUtDB/iHx2DY1wEEcukgboW4GbKZwOPl4bdmV5o0/bOsjoZaCOt5DWeEqxZhx3Qg== X-Received: by 2002:a19:c197:: with SMTP id r145mr8436276lff.41.1597709325512; Mon, 17 Aug 2020 17:08:45 -0700 (PDT) Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com. [209.85.167.46]) by smtp.gmail.com with ESMTPSA id h23sm5153153lji.139.2020.08.17.17.08.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Aug 2020 17:08:44 -0700 (PDT) Received: by mail-lf1-f46.google.com with SMTP id v15so9259331lfg.6 for ; Mon, 17 Aug 2020 17:08:43 -0700 (PDT) X-Received: by 2002:a05:6512:3b7:: with SMTP id v23mr8518837lfp.10.1597709323252; Mon, 17 Aug 2020 17:08:43 -0700 (PDT) MIME-Version: 1.0 References: <87ft8l6ic3.fsf@x220.int.ebiederm.org> <20200817220425.9389-12-ebiederm@xmission.com> In-Reply-To: <20200817220425.9389-12-ebiederm@xmission.com> From: Linus Torvalds Date: Mon, 17 Aug 2020 17:08:27 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 12/17] proc/fd: In fdinfo seq_show don't use get_files_struct To: "Eric W. Biederman" Cc: Linux Kernel Mailing List , linux-fsdevel , criu@openvz.org, bpf , Alexander Viro , Christian Brauner , Oleg Nesterov , Cyrill Gorcunov , Jann Horn , Kees Cook , =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= , Jeff Layton , Miklos Szeredi , Matthew Wilcox , "J. Bruce Fields" , Matthew Wilcox , Trond Myklebust , Chris Wright , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 17, 2020 at 3:11 PM Eric W. Biederman wrote: > > Instead hold task_lock for the duration that task->files needs to be > stable in seq_show. The task_lock was already taken in > get_files_struct, and so skipping get_files_struct performs less work > overall, and avoids the problems with the files_struct reference > count. Hmm. Do we even need that task_lock() at all? Couldn't we do this all with just the RCU lock held for reading? As far as I can tell, we don't really need the task lock. We don't even need the get/pid_task_struct(). Can't we just do rcu_read_lock(); task = pid_task(proc_pid(inode), PIDTYPE_PID); if (task) { unsigned int fd = proc_fd(m->private); struct file *file = fcheck_task(task, fd); if (file) .. do the seq_printf .. and do it all with no refcount games or task locking at all? Anyway, I don't dislike your patch per se, I just get the feeling that you could go a bit further in that cleanup.. And it's quite possible that I'm wrong, and you can't just use the RCU lock, but as far as I can tell, both the task lookup and the file lookup *already* really both depend on the RCU lock anyway, so the other locking and reference counting games really do seem superfluous. Please just send me a belittling email telling me what a nincompoop I am if I have missed something. Linus