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=-8.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 68BB9C0044D for ; Mon, 16 Mar 2020 16:41:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2027A20409 for ; Mon, 16 Mar 2020 16:41:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="fy2ABSSo" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2027A20409 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 BF77D6B000D; Mon, 16 Mar 2020 12:41:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BA6426B000E; Mon, 16 Mar 2020 12:41:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABCF96B0010; Mon, 16 Mar 2020 12:41:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0168.hostedemail.com [216.40.44.168]) by kanga.kvack.org (Postfix) with ESMTP id 95BD96B000D for ; Mon, 16 Mar 2020 12:41:13 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 339A8DE04 for ; Mon, 16 Mar 2020 16:41:13 +0000 (UTC) X-FDA: 76601790426.03.bears26_7f01db0c9c234 X-HE-Tag: bears26_7f01db0c9c234 X-Filterd-Recvd-Size: 6054 Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Mon, 16 Mar 2020 16:41:12 +0000 (UTC) Received: by mail-qk1-f194.google.com with SMTP id q17so2648750qki.0 for ; Mon, 16 Mar 2020 09:41:12 -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=teJ4I/KRnGMjHNSlQNLKiBR+4QkIP+oMamPkbV9I/eE=; b=fy2ABSSoZVEKtMkGwP5V4uQTGU1ZZXNsmYuN9E374yTTf8QdZks11+xj+8nwwxLQG9 k9XLAgVfoq4NM/thRDTox+1nQHr80YF81BVsRTmsENzQHmnjqQOscRKd0GFdwzIVZm8X hcaec+Otl0O9RgFJ8qoNSgmupoxMZm5taAlvtBI+SZJXpFq6DHlIViOMt/0JfNMOUFUS c+tSOQ+V27l9ZgmXh9PVCytGvPxRLintmfpa8dXEzHbnGmsrSJZ5GPcX+CCG8s0ELhyy 4tr5gWb2GBEDgGNUAHpqPeRrt7b6G9/RzfM5K1iz+8L+pGVAVZxUIzhMMJpg+URI9Y/H fjig== 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=teJ4I/KRnGMjHNSlQNLKiBR+4QkIP+oMamPkbV9I/eE=; b=RwWeuXWroz5JgTuS1dI5875ItLCv65j7KILgG39ySn6Nw/QOxRxkGlbGvfziIaKG2h bt5/CV5bKvXKAWHgoqO+rnM0EQ69S3yBLIHE2YI/Plv6YMoj5b9tcIYQotxqNgIWyq06 KMKVmfkL3Ap1kXFJDOxvd7C6z13u863ETUREM8X6HN/fi1dtxBcZVhQmoB7t5O+s6Art wOvVMDpftNbMmxjFari8LkRXzHITieb1Nns0jZkqxQMftPX50kBWt7qxl61HfhlbVtHd OskYo/MNAjE6sRRu25hO/780sKMNzFbEkg9F2Bd4DXc3j1fZeqTKBn+jmVHpPMO4aKZP hZ+Q== X-Gm-Message-State: ANhLgQ2IOlTot3hJb0xUHQWj+ebR+wWA45Qmp/L3xxC8ToqZ9zFb/B4v Wg7VBXfwJ4p+PFlQkExzLJvLO7r8E9NIug== X-Google-Smtp-Source: ADFU+vvOmZFL4UEXxL0F+VyJeV3bzCvNLstTymZzGsXKcSDSSsmNvxohrEjO7CxZtFRpcU5VzyKxZA== X-Received: by 2002:a37:b042:: with SMTP id z63mr541181qke.269.1584376871917; Mon, 16 Mar 2020 09:41:11 -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 z11sm159027qti.23.2020.03.16.09.41.11 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Mar 2020 09:41:11 -0700 (PDT) Received: from jgg by mlx.ziepe.ca with local (Exim 4.90_1) (envelope-from ) id 1jDsne-0007EQ-Po; Mon, 16 Mar 2020 13:41:10 -0300 Date: Mon, 16 Mar 2020 13:41:10 -0300 From: Jason Gunthorpe To: Christoph Hellwig Cc: Jerome Glisse , Ralph Campbell , linux-mm@kvack.org Subject: Re: [PATCH 5/5] mm: merge hmm_vma_do_fault into into hmm_vma_walk_hole_ Message-ID: <20200316164110.GA27591@ziepe.ca> References: <20200316135310.899364-1-hch@lst.de> <20200316135310.899364-6-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200316135310.899364-6-hch@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 02:53:10PM +0100, Christoph Hellwig wrote: > There is no good reason for this split, as it just obsfucates the flow. > > Signed-off-by: Christoph Hellwig > mm/hmm.c | 49 ++++++++++++++++--------------------------------- > 1 file changed, 16 insertions(+), 33 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index 707edba850de..180e398170b0 100644 > +++ b/mm/hmm.c > @@ -33,32 +33,6 @@ struct hmm_vma_walk { > unsigned int flags; > }; > > -static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr, > - bool write_fault, uint64_t *pfn) > -{ > - unsigned int flags = FAULT_FLAG_REMOTE; > - struct hmm_vma_walk *hmm_vma_walk = walk->private; > - struct hmm_range *range = hmm_vma_walk->range; > - struct vm_area_struct *vma = walk->vma; > - vm_fault_t ret; > - > - if (!vma) > - goto err; > - > - if (write_fault) > - flags |= FAULT_FLAG_WRITE; > - > - ret = handle_mm_fault(vma, addr, flags); > - if (ret & VM_FAULT_ERROR) > - goto err; > - > - return -EBUSY; > - > -err: > - *pfn = range->values[HMM_PFN_ERROR]; > - return -EFAULT; > -} > - > static int hmm_pfns_fill(unsigned long addr, unsigned long end, > struct hmm_range *range, enum hmm_pfn_value_e value) > { > @@ -90,25 +64,34 @@ static int hmm_vma_fault(unsigned long addr, unsigned long end, > { > struct hmm_vma_walk *hmm_vma_walk = walk->private; > struct hmm_range *range = hmm_vma_walk->range; > + struct vm_area_struct *vma = walk->vma; > uint64_t *pfns = range->pfns; > unsigned long i = (addr - range->start) >> PAGE_SHIFT; > + unsigned int fault_flags = FAULT_FLAG_REMOTE; > > WARN_ON_ONCE(!fault && !write_fault); > hmm_vma_walk->last = addr; > > - if (write_fault && walk->vma && !(walk->vma->vm_flags & VM_WRITE)) > - return -EPERM; > + if (!vma) > + goto out_error; > + > + if (write_fault) { > + if (!(vma->vm_flags & VM_WRITE)) > + return -EPERM; > + fault_flags |= FAULT_FLAG_WRITE; > + } > > for (; addr < end; addr += PAGE_SIZE, i++) { > - int ret; > - > + if (handle_mm_fault(vma, addr, fault_flags) & VM_FAULT_ERROR) > + goto out_error; > pfns[i] = range->values[HMM_PFN_NONE]; > - ret = hmm_vma_do_fault(walk, addr, write_fault, &pfns[i]); > - if (ret != -EBUSY) > - return ret; > } Yes, this is much better > return -EBUSY; > + > +out_error: > + pfns[i] = range->values[HMM_PFN_ERROR]; > + return -EFAULT; I've also got a patch deleting these confusing HMM_PFN_ERRORs. They are not applied consistently and no caller would scan the output for ERROR on some failures. For instance the above doesn't set it on EPERM. So this can just be 'return -EFAULT' instead of 'goto out_error' I can fold that in if you agree: Reviewed-by: Jason Gunthorpe Jason