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=-2.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 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 6878FC43331 for ; Wed, 13 Nov 2019 13:59:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2CAAE222CD for ; Wed, 13 Nov 2019 13:59:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="L4pEOF8g" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2CAAE222CD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B92756B0005; Wed, 13 Nov 2019 08:59:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B42E46B0006; Wed, 13 Nov 2019 08:59:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A57DD6B000A; Wed, 13 Nov 2019 08:59:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0078.hostedemail.com [216.40.44.78]) by kanga.kvack.org (Postfix) with ESMTP id 8FA766B0005 for ; Wed, 13 Nov 2019 08:59:55 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 1B38A2465 for ; Wed, 13 Nov 2019 13:59:55 +0000 (UTC) X-FDA: 76151412750.30.event51_568367edbdc43 X-HE-Tag: event51_568367edbdc43 X-Filterd-Recvd-Size: 5279 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 Nov 2019 13:59:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=Mr4RY3jvcUlwtWVilQSRD7aASsaH0ZVmNXKgXD4ar2I=; b=L4pEOF8gToeI1wxRP1AwsUyj7 it0aZ/H3gzMqN/frlWv7pf8ACU3+ysRk7aQMNDAwKSfgsnYZtpbRLVOgl2xWx2oZDjs6GDaFt9WvR l/NQ2gO/K3Ot7KXxQGR4VKhxlIaN3GUEGwzrCHmhIUE4AAL0RvwNFG3C6vKao2bg2rryraoP9pDjJ M9IDd6BxvnT2iTiJ3v7dvNhUY1+or3v58FJ4rYsLWxgRZsp54JWPrtILV8uaQiYhqnc+PE8/Af1RU FAMu2LxxYuyc1s+r4BStFsKVk8JJ0/AZtijke8xPvXEVjlaYoNKs52mvNP7KuysuicWSP/O+gJLCs 9wgg5VPgA==; Received: from hch by bombadil.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1iUtBY-0001WY-NU; Wed, 13 Nov 2019 13:59:52 +0000 Date: Wed, 13 Nov 2019 05:59:52 -0800 From: Christoph Hellwig To: Jason Gunthorpe Cc: linux-mm@kvack.org, Jerome Glisse , Ralph Campbell , John Hubbard , Felix.Kuehling@amd.com, linux-rdma@vger.kernel.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, Alex Deucher , Ben Skeggs , Boris Ostrovsky , Christian =?iso-8859-1?Q?K=F6nig?= , David Zhou , Dennis Dalessandro , Juergen Gross , Mike Marciniszyn , Oleksandr Andrushchenko , Petr Cvek , Stefano Stabellini , nouveau@lists.freedesktop.org, xen-devel@lists.xenproject.org, Christoph Hellwig , Jason Gunthorpe , Philip Yang Subject: Re: [PATCH v3 02/14] mm/mmu_notifier: add an interval tree notifier Message-ID: <20191113135952.GB20531@infradead.org> References: <20191112202231.3856-1-jgg@ziepe.ca> <20191112202231.3856-3-jgg@ziepe.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191112202231.3856-3-jgg@ziepe.ca> User-Agent: Mutt/1.12.1 (2019-06-15) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > +int mmu_interval_notifier_insert(struct mmu_interval_notifier *mni, > + struct mm_struct *mm, unsigned long start, > + unsigned long length, > + const struct mmu_interval_notifier_ops *ops); > +int mmu_interval_notifier_insert_locked( > + struct mmu_interval_notifier *mni, struct mm_struct *mm, > + unsigned long start, unsigned long length, > + const struct mmu_interval_notifier_ops *ops); Very inconsistent indentation between these two related functions. > + /* > + * The inv_end incorporates a deferred mechanism like > + * rtnl_unlock(). Adds and removes are queued until the final inv_end > + * happens then they are progressed. This arrangement for tree updates > + * is used to avoid using a blocking lock during > + * invalidate_range_start. Nitpick: That comment can be condensed into one less line: /* * The inv_end incorporates a deferred mechanism like rtnl_unlock(). * Adds and removes are queued until the final inv_end happens then * they are progressed. This arrangement for tree updates is used to * avoid using a blocking lock during invalidate_range_start. */ > + /* > + * TODO: Since we already have a spinlock above, this would be faster > + * as wake_up_q > + */ > + if (need_wake) > + wake_up_all(&mmn_mm->wq); So why is this important enough for a TODO comment, but not important enough to do right away? > + * release semantics on the initialization of the mmu_notifier_mm's > + * contents are provided for unlocked readers. acquire can only be > + * used while holding the mmgrab or mmget, and is safe because once > + * created the mmu_notififer_mm is not freed until the mm is > + * destroyed. As above, users holding the mmap_sem or one of the > + * mm_take_all_locks() do not need to use acquire semantics. > */ Some spaces instead of tabs here. > +static int __mmu_interval_notifier_insert( > + struct mmu_interval_notifier *mni, struct mm_struct *mm, > + struct mmu_notifier_mm *mmn_mm, unsigned long start, > + unsigned long length, const struct mmu_interval_notifier_ops *ops) Odd indentation - we usuall do two tabs (my preference) or align after the opening brace. > + * This function must be paired with mmu_interval_notifier_insert(). It cannot be line > 80 chars. Otherwise this looks good and very similar to what I reviewed earlier: Reviewed-by: Christoph Hellwig