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.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 0C1F2C0044D for ; Mon, 16 Mar 2020 18:07:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B0AB820674 for ; Mon, 16 Mar 2020 18:07:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="DKEnDkSO" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B0AB820674 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 489216B0003; Mon, 16 Mar 2020 14:07:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 43B2D6B0005; Mon, 16 Mar 2020 14:07:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 34F4F6B0007; Mon, 16 Mar 2020 14:07:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0035.hostedemail.com [216.40.44.35]) by kanga.kvack.org (Postfix) with ESMTP id 1A3506B0003 for ; Mon, 16 Mar 2020 14:07:16 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id AB0F352AC for ; Mon, 16 Mar 2020 18:07:15 +0000 (UTC) X-FDA: 76602007230.07.nest54_520727672a5c X-HE-Tag: nest54_520727672a5c X-Filterd-Recvd-Size: 6121 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by imf47.hostedemail.com (Postfix) with ESMTP for ; Mon, 16 Mar 2020 18:07:15 +0000 (UTC) Received: by mail-qk1-f193.google.com with SMTP id e11so27800978qkg.9 for ; Mon, 16 Mar 2020 11:07:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=BL4dn+rFA5hNgcpGIcpSUF3dGv9P3+nzjxB+rkZXdM4=; b=DKEnDkSOlfzjpgSnrXBEjUC1923pN4NQuhu8yekYIb9Cl/gcjD29oe7Zwom9ayo1l6 ii/cMSB9TjmDtSmwKOfTrPDuuuDeMFvopq04LstsyTdRjPRJ3uMeZZgL6GohqV80EcBA ckySb0cXjAfeZzaSxXhVL0vckuNrSqYx+CTuNhNhQ8glRsJSUgFoipt9jfreFxXj+wzr +6+mSPQyLs/j+LIHlg/ZEFHwtEkLK/uwwz06tBZw7JOq2pDMUDodMqQ78UusyshS4ecy 0YdLlpeOTbhQpt6Btu4zmOeR/h1+J13FplibFTzuwj/MzVZ3OTbxBvz9zD/JX6ulW387 iQvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=BL4dn+rFA5hNgcpGIcpSUF3dGv9P3+nzjxB+rkZXdM4=; b=AqtZthgsQqNycBhJfo9ZJNww0mZRANr953ky6JgGXw34Nr5p6o9zxfWth+zA0PiXNX +gKoGbkCN6Pl+wzi27+3qEfOEvRfLMOqH4M+MZ3+d3z3l/PQbqeF2iUJh4O6MWUmd0vn rSQblAk3ACqD50MD7thB3POW+Ce2vkvR3JruPWOnrnWFHLJZ0LyrdD73in53RmXFSdLi mOMxOETsampV/97Tqwunw7X8Q0EyzQ8zALqH/5r56lkIK06dtDrUGpVy40HnoOYfTY4R k9qbKk7VrtIDOXo++4cJaBeS5Kn8a47zcITdgu4n4yD+Fn+C5FTU0VlWCa35CkcUD/Ue M5cw== X-Gm-Message-State: ANhLgQ1YLBWa2wUYPgY25QJXnQZdndNw0onjd4X3p3UvAvEJ7BstSZ+f YeIRgj8FE9mtsBAPOLY7ioKHRQ== X-Google-Smtp-Source: ADFU+vtIq8ax1/sBMUy0L6xn/rwaL8F9DxFBkYRulC2VO6q8JD8eKOdnZIYjW2re88h1P9q0ynL8UA== X-Received: by 2002:a37:9d8f:: with SMTP id g137mr902005qke.133.1584382034472; Mon, 16 Mar 2020 11:07:14 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-142-68-57-212.dhcp-dynamic.fibreop.ns.bellaliant.net. [142.68.57.212]) by smtp.gmail.com with ESMTPSA id 68sm254723qkh.75.2020.03.16.11.07.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Mar 2020 11:07:13 -0700 (PDT) Received: from jgg by mlx.ziepe.ca with local (Exim 4.90_1) (envelope-from ) id 1jDu8v-0005Nz-7D; Mon, 16 Mar 2020 15:07:13 -0300 Date: Mon, 16 Mar 2020 15:07:13 -0300 From: Jason Gunthorpe To: Christoph Hellwig Cc: Jerome Glisse , Ralph Campbell , Felix.Kuehling@amd.com, linux-mm@kvack.org, John Hubbard , dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, Philip Yang Subject: Re: [PATCH hmm 2/8] mm/hmm: don't free the cached pgmap while scanning Message-ID: <20200316180713.GI20941@ziepe.ca> References: <20200311183506.3997-1-jgg@ziepe.ca> <20200311183506.3997-3-jgg@ziepe.ca> <20200316090250.GB12439@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200316090250.GB12439@lst.de> User-Agent: Mutt/1.9.4 (2018-02-28) 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: On Mon, Mar 16, 2020 at 10:02:50AM +0100, Christoph Hellwig wrote: > On Wed, Mar 11, 2020 at 03:35:00PM -0300, Jason Gunthorpe wrote: > > @@ -694,6 +672,15 @@ long hmm_range_fault(struct hmm_range *range, unsigned int flags) > > return -EBUSY; > > ret = walk_page_range(mm, hmm_vma_walk.last, range->end, > > &hmm_walk_ops, &hmm_vma_walk); > > + /* > > + * A pgmap is kept cached in the hmm_vma_walk to avoid expensive > > + * searching in the probably common case that the pgmap is the > > + * same for the entire requested range. > > + */ > > + if (hmm_vma_walk.pgmap) { > > + put_dev_pagemap(hmm_vma_walk.pgmap); > > + hmm_vma_walk.pgmap = NULL; > > + } > > } while (ret == -EBUSY); > > In which case it should only be put on return, and not for every loop. I chose this to be simple without having to goto unwind it. So, instead like this: @@ -683,21 +661,33 @@ long hmm_range_fault(struct hmm_range *range, unsigned int flags) .flags = flags, }; struct mm_struct *mm = range->notifier->mm; - int ret; + long ret; lockdep_assert_held(&mm->mmap_sem); do { /* If range is no longer valid force retry. */ if (mmu_interval_check_retry(range->notifier, - range->notifier_seq)) - return -EBUSY; + range->notifier_seq)) { + ret = -EBUSY; + goto out; + } ret = walk_page_range(mm, hmm_vma_walk.last, range->end, &hmm_walk_ops, &hmm_vma_walk); } while (ret == -EBUSY); if (ret) - return ret; - return (hmm_vma_walk.last - range->start) >> PAGE_SHIFT; + goto out; + ret = (hmm_vma_walk.last - range->start) >> PAGE_SHIFT; + +out: + /* + * A pgmap is kept cached in the hmm_vma_walk to avoid expensive + * searching in the probably common case that the pgmap is the + * same for the entire requested range. + */ + if (hmm_vma_walk.pgmap) + put_dev_pagemap(hmm_vma_walk.pgmap); + return ret; } EXPORT_SYMBOL(hmm_range_fault); ? > I still think the right fix is to just delete all the unused and broken > pgmap handling code. If we ever need to add it back it can be added > in a proper understood and tested way. What I want to add is something like if (pgmap != walk->required_pgmap) cpu_flags = 0 hmm_range_need_fault(..., cpu_flags, ...) Which will fix a bug in nouveau where it blindly assumes any device pages are its own, IIRC. I think Ralph observed it needs to be here, because if the pgmap doesn't match then it should trigger migration, in a single call, rather than iterating. I'm mostly expecting to replace all the other pgmap code, but keep the pgmap caching. The existing pgmap stuff seems approx right, but useless.. Jason