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.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=unavailable 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 87489C282C0 for ; Wed, 23 Jan 2019 22:46:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 542782184C for ; Wed, 23 Jan 2019 22:46:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726821AbfAWWqr (ORCPT ); Wed, 23 Jan 2019 17:46:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40332 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726109AbfAWWqr (ORCPT ); Wed, 23 Jan 2019 17:46:47 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CB823C0C6C15; Wed, 23 Jan 2019 22:46:45 +0000 (UTC) Received: from redhat.com (ovpn-120-127.rdu2.redhat.com [10.10.120.127]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AFD076B8FC; Wed, 23 Jan 2019 22:46:42 +0000 (UTC) Date: Wed, 23 Jan 2019 17:46:40 -0500 From: Jerome Glisse To: Jason Gunthorpe Cc: "linux-mm@kvack.org" , Andrew Morton , "linux-kernel@vger.kernel.org" , Christian =?iso-8859-1?Q?K=F6nig?= , Jan Kara , Felix Kuehling , Matthew Wilcox , Ross Zwisler , Dan Williams , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Michal Hocko , Ralph Campbell , John Hubbard , "kvm@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "linux-rdma@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , Arnd Bergmann Subject: Re: [PATCH v4 9/9] RDMA/umem_odp: optimize out the case when a range is updated to read only Message-ID: <20190123224640.GA1257@redhat.com> References: <20190123222315.1122-1-jglisse@redhat.com> <20190123222315.1122-10-jglisse@redhat.com> <20190123223153.GP8986@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190123223153.GP8986@mellanox.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 23 Jan 2019 22:46:46 +0000 (UTC) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Jan 23, 2019 at 10:32:00PM +0000, Jason Gunthorpe wrote: > On Wed, Jan 23, 2019 at 05:23:15PM -0500, jglisse@redhat.com wrote: > > From: Jérôme Glisse > > > > When range of virtual address is updated read only and corresponding > > user ptr object are already read only it is pointless to do anything. > > Optimize this case out. > > > > Signed-off-by: Jérôme Glisse > > Cc: Christian König > > Cc: Jan Kara > > Cc: Felix Kuehling > > Cc: Jason Gunthorpe > > Cc: Andrew Morton > > Cc: Matthew Wilcox > > Cc: Ross Zwisler > > Cc: Dan Williams > > Cc: Paolo Bonzini > > Cc: Radim Krčmář > > Cc: Michal Hocko > > Cc: Ralph Campbell > > Cc: John Hubbard > > Cc: kvm@vger.kernel.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-rdma@vger.kernel.org > > Cc: linux-fsdevel@vger.kernel.org > > Cc: Arnd Bergmann > > drivers/infiniband/core/umem_odp.c | 22 +++++++++++++++++++--- > > include/rdma/ib_umem_odp.h | 1 + > > 2 files changed, 20 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c > > index a4ec43093cb3..fa4e7fdcabfc 100644 > > +++ b/drivers/infiniband/core/umem_odp.c > > @@ -140,8 +140,15 @@ static void ib_umem_notifier_release(struct mmu_notifier *mn, > > static int invalidate_range_start_trampoline(struct ib_umem_odp *item, > > u64 start, u64 end, void *cookie) > > { > > + bool update_to_read_only = *((bool *)cookie); > > + > > ib_umem_notifier_start_account(item); > > - item->umem.context->invalidate_range(item, start, end); > > + /* > > + * If it is already read only and we are updating to read only then we > > + * do not need to change anything. So save time and skip this one. > > + */ > > + if (!update_to_read_only || !item->read_only) > > + item->umem.context->invalidate_range(item, start, end); > > return 0; > > } > > > > @@ -150,6 +157,7 @@ static int ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn, > > { > > struct ib_ucontext_per_mm *per_mm = > > container_of(mn, struct ib_ucontext_per_mm, mn); > > + bool update_to_read_only; > > > > if (range->blockable) > > down_read(&per_mm->umem_rwsem); > > @@ -166,10 +174,13 @@ static int ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn, > > return 0; > > } > > > > + update_to_read_only = mmu_notifier_range_update_to_read_only(range); > > + > > return rbt_ib_umem_for_each_in_range(&per_mm->umem_tree, range->start, > > range->end, > > invalidate_range_start_trampoline, > > - range->blockable, NULL); > > + range->blockable, > > + &update_to_read_only); > > } > > > > static int invalidate_range_end_trampoline(struct ib_umem_odp *item, u64 start, > > @@ -363,6 +374,9 @@ struct ib_umem_odp *ib_alloc_odp_umem(struct ib_ucontext_per_mm *per_mm, > > goto out_odp_data; > > } > > > > + /* Assume read only at first, each time GUP is call this is updated. */ > > + odp_data->read_only = true; > > + > > odp_data->dma_list = > > vzalloc(array_size(pages, sizeof(*odp_data->dma_list))); > > if (!odp_data->dma_list) { > > @@ -619,8 +633,10 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp *umem_odp, u64 user_virt, > > goto out_put_task; > > } > > > > - if (access_mask & ODP_WRITE_ALLOWED_BIT) > > + if (access_mask & ODP_WRITE_ALLOWED_BIT) { > > + umem_odp->read_only = false; > > No locking? The mmu notitfier exclusion will ensure that it is not missed ie it will be false before any mmu notifier might be call on page GUPed with write flag which is what matter here. So lock are useless here. > > > flags |= FOLL_WRITE; > > + } > > > > start_idx = (user_virt - ib_umem_start(umem)) >> page_shift; > > k = start_idx; > > diff --git a/include/rdma/ib_umem_odp.h b/include/rdma/ib_umem_odp.h > > index 0b1446fe2fab..8256668c6170 100644 > > +++ b/include/rdma/ib_umem_odp.h > > @@ -76,6 +76,7 @@ struct ib_umem_odp { > > struct completion notifier_completion; > > int dying; > > struct work_struct work; > > + bool read_only; > > }; > > The ib_umem already has a writeable flag. This reflects if the user > asked for write permission to be granted.. The tracking here is if any > remote fault thus far has requested write, is this an important > difference to justify the new flag? I did that patch couple week ago and now i do not remember why i did not use that, i remember thinking about it ... damm i need to keep better notes. I will review the code again. Cheers, Jérôme