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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 A7BCCC63697 for ; Sat, 21 Nov 2020 12:47:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1658222261 for ; Sat, 21 Nov 2020 12:47:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mLymZ4jY" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1658222261 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 42AFB6B005C; Sat, 21 Nov 2020 07:47:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3DBE26B005D; Sat, 21 Nov 2020 07:47:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A3AF6B0068; Sat, 21 Nov 2020 07:47:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0220.hostedemail.com [216.40.44.220]) by kanga.kvack.org (Postfix) with ESMTP id F25EA6B005C for ; Sat, 21 Nov 2020 07:47:33 -0500 (EST) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 9F93E8249980 for ; Sat, 21 Nov 2020 12:47:33 +0000 (UTC) X-FDA: 77508401586.09.ball59_2c0bda927354 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin09.hostedemail.com (Postfix) with ESMTP id 8822A180AD822 for ; Sat, 21 Nov 2020 12:47:33 +0000 (UTC) X-HE-Tag: ball59_2c0bda927354 X-Filterd-Recvd-Size: 11305 Received: from mail-oi1-f195.google.com (mail-oi1-f195.google.com [209.85.167.195]) by imf38.hostedemail.com (Postfix) with ESMTP for ; Sat, 21 Nov 2020 12:47:33 +0000 (UTC) Received: by mail-oi1-f195.google.com with SMTP id d9so13960603oib.3 for ; Sat, 21 Nov 2020 04:47:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=FlSNWJmNWiQiTz/L3So6lTyhHOBK1+EUvQhZPXGeudI=; b=mLymZ4jYEYnHEMUGmJGhbkCkm1/1qhhUhOnp9qFSCFG2Dxje5uBk1Cu74PjQWtH6b5 4meVf+nGBt24oxg5D64fUDpYxNPXcFJ3JWirTSlcfcbkEXHfIfA7ryvdgRH7Pg+TdZSE dXaqVbw2dDru151HevP7AdbOrmVOFzpq6gwTytqG99SrZRo9HT8jxqccu5LbPFDciEjK 73dQt+jHxzIchZmrsV9kc/D+EIubFH/d9GrQZfH7Ef62qaz1/BCyVf0uIUsUtVCSJIBQ KGgGvmw8sADN7mTwRTiaZGW1oqtzyDge51JVHpD0LL9DGy9LuVKt7/YUO+W9kh9V3uLD 9uiQ== 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:content-transfer-encoding; bh=FlSNWJmNWiQiTz/L3So6lTyhHOBK1+EUvQhZPXGeudI=; b=qSj52vCoguRkaAU4PsoHcb/QB3KROdlpskitHVGawgF5cYl4kEcGdgx0nBnDGNRER/ o4Eag+d3HoxVKkQbKf6f+ApXb0t2z9QTwuSY4ABsy/JZr0L8gn4LXmuwban4isziQVgw ceUK/paKdNk2NIT00VxB3pXi190efDr0d8GempmuDirUGxg8eEUe/G0uBhBmabreEAQV Zrsr8YSVZdMCVwdE3E991MOUsdX7DtN6dKsM/+2JT4SBwSFtmZTfEMmi0pxyelMGj66Z 21HrfGNOnW0xZ7pesBuGEFGLRjzk09hV4eFh9faOtk/+FvhVoaZ4o28OWruixN8xRQdb jcAw== X-Gm-Message-State: AOAM532HPGKhi445m1Sb4nZT8r/KKHTu72TciHzt3JoKiIMzvwNSRLQG bn64Ff25XSBFb382Iw+YpSBsVoiPm3jw9+YuPx4= X-Google-Smtp-Source: ABdhPJwm/h9kUMFqlGm0lJmnxV9/hlp30MxHOKOag39ZoEYRMrUPlCrFLaqhGvcbUmpwSXdstBWktehEKaGSR9+lNnw= X-Received: by 2002:aca:a896:: with SMTP id r144mr2111725oie.154.1605962852497; Sat, 21 Nov 2020 04:47:32 -0800 (PST) MIME-Version: 1.0 References: <20201119144146.1045202-1-daniel.vetter@ffwll.ch> <20201119144146.1045202-4-daniel.vetter@ffwll.ch> In-Reply-To: <20201119144146.1045202-4-daniel.vetter@ffwll.ch> From: Oded Gabbay Date: Sat, 21 Nov 2020 14:47:05 +0200 Message-ID: Subject: Re: [PATCH v6 03/17] misc/habana: Stop using frame_vector helpers To: Daniel Vetter Cc: DRI Development , LKML , KVM list , linux-mm , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , linux-samsung-soc , Linux Media Mailing List , John Hubbard , Daniel Vetter , Christoph Hellwig , Jason Gunthorpe , Andrew Morton , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Jan Kara , Dan Williams , Omer Shpigelman , Ofir Bitton , Tomer Tayar , Moti Haimovski , Greg Kroah-Hartman , Pawel Piskorski Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Thu, Nov 19, 2020 at 4:41 PM Daniel Vetter wrot= e: > > All we need are a pages array, pin_user_pages_fast can give us that > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames. > > Note that pin_user_pages_fast is a safe replacement despite the > seeming lack of checking for vma->vm_flasg & (VM_IO | VM_PFNMAP). Such > ptes are marked with pte_mkspecial (which pup_fast rejects in the > fastpath), and only architectures supporting that support the > pin_user_pages_fast fastpath. > > Reviewed-by: John Hubbard > Signed-off-by: Daniel Vetter > Cc: Christoph Hellwig > Cc: Jason Gunthorpe > Cc: Andrew Morton > Cc: John Hubbard > Cc: J=C3=A9r=C3=B4me Glisse > Cc: Jan Kara > Cc: Dan Williams > Cc: linux-mm@kvack.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-samsung-soc@vger.kernel.org > Cc: linux-media@vger.kernel.org > Cc: Oded Gabbay > Cc: Omer Shpigelman > Cc: Ofir Bitton > Cc: Tomer Tayar > Cc: Moti Haimovski > Cc: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Pawel Piskorski > Signed-off-by: Daniel Vetter > -- > v2: Use unpin_user_pages_dirty_lock (John) > v3: Update kerneldoc (Oded) > v6: Explain why pup_fast is safe, after discussions with John and > Christoph. > --- > drivers/misc/habanalabs/Kconfig | 1 - > drivers/misc/habanalabs/common/habanalabs.h | 6 ++- > drivers/misc/habanalabs/common/memory.c | 49 ++++++++------------- > 3 files changed, 22 insertions(+), 34 deletions(-) > > diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kc= onfig > index 1640340d3e62..293d79811372 100644 > --- a/drivers/misc/habanalabs/Kconfig > +++ b/drivers/misc/habanalabs/Kconfig > @@ -6,7 +6,6 @@ > config HABANA_AI > tristate "HabanaAI accelerators (habanalabs)" > depends on PCI && HAS_IOMEM > - select FRAME_VECTOR > select GENERIC_ALLOCATOR > select HWMON > help > diff --git a/drivers/misc/habanalabs/common/habanalabs.h b/drivers/misc/h= abanalabs/common/habanalabs.h > index 80d4d7385ffe..272aa3f29200 100644 > --- a/drivers/misc/habanalabs/common/habanalabs.h > +++ b/drivers/misc/habanalabs/common/habanalabs.h > @@ -921,7 +921,8 @@ struct hl_ctx_mgr { > * struct hl_userptr - memory mapping chunk information > * @vm_type: type of the VM. > * @job_node: linked-list node for hanging the object on the Job's list. > - * @vec: pointer to the frame vector. > + * @pages: pointer to struct page array > + * @npages: size of @pages array > * @sgt: pointer to the scatter-gather table that holds the pages. > * @dir: for DMA unmapping, the direction must be supplied, so save it. > * @debugfs_list: node in debugfs list of command submissions. > @@ -932,7 +933,8 @@ struct hl_ctx_mgr { > struct hl_userptr { > enum vm_type_t vm_type; /* must be first */ > struct list_head job_node; > - struct frame_vector *vec; > + struct page **pages; > + unsigned int npages; > struct sg_table *sgt; > enum dma_data_direction dir; > struct list_head debugfs_list; > diff --git a/drivers/misc/habanalabs/common/memory.c b/drivers/misc/haban= alabs/common/memory.c > index 84227819e4d1..0b220221873d 100644 > --- a/drivers/misc/habanalabs/common/memory.c > +++ b/drivers/misc/habanalabs/common/memory.c > @@ -1291,45 +1291,41 @@ static int get_user_memory(struct hl_device *hdev= , u64 addr, u64 size, > return -EFAULT; > } > > - userptr->vec =3D frame_vector_create(npages); > - if (!userptr->vec) { > + userptr->pages =3D kvmalloc_array(npages, sizeof(*userptr->pages)= , > + GFP_KERNEL); > + if (!userptr->pages) { > dev_err(hdev->dev, "Failed to create frame vector\n"); > return -ENOMEM; > } Hi Daniel, sorry but missed this in my initial review. The error message no longer fits the code, and actually it isn't needed as we don't print error messages on malloc failures. With that fixed, this patch is: Reviewed-by: Oded Gabbay > > - rc =3D get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > - userptr->vec); > + rc =3D pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE= , > + userptr->pages); > > if (rc !=3D npages) { > dev_err(hdev->dev, > "Failed to map host memory, user ptr probably wro= ng\n"); > if (rc < 0) > - goto destroy_framevec; > + goto destroy_pages; > + npages =3D rc; > rc =3D -EFAULT; > - goto put_framevec; > - } > - > - if (frame_vector_to_pages(userptr->vec) < 0) { > - dev_err(hdev->dev, > - "Failed to translate frame vector to pages\n"); > - rc =3D -EFAULT; > - goto put_framevec; > + goto put_pages; > } > + userptr->npages =3D npages; > > rc =3D sg_alloc_table_from_pages(userptr->sgt, > - frame_vector_pages(userptr->vec), > - npages, offset, size, GFP_ATOMIC)= ; > + userptr->pages, > + npages, offset, size, GFP_ATOMIC); > if (rc < 0) { > dev_err(hdev->dev, "failed to create SG table from pages\= n"); > - goto put_framevec; > + goto put_pages; > } > > return 0; > > -put_framevec: > - put_vaddr_frames(userptr->vec); > -destroy_framevec: > - frame_vector_destroy(userptr->vec); > +put_pages: > + unpin_user_pages(userptr->pages, npages); > +destroy_pages: > + kvfree(userptr->pages); > return rc; > } > > @@ -1415,8 +1411,6 @@ int hl_pin_host_memory(struct hl_device *hdev, u64 = addr, u64 size, > */ > void hl_unpin_host_memory(struct hl_device *hdev, struct hl_userptr *use= rptr) > { > - struct page **pages; > - > hl_debugfs_remove_userptr(hdev, userptr); > > if (userptr->dma_mapped) > @@ -1424,15 +1418,8 @@ void hl_unpin_host_memory(struct hl_device *hdev, = struct hl_userptr *userptr) > userptr->sgt->nen= ts, > userptr->dir); > > - pages =3D frame_vector_pages(userptr->vec); > - if (!IS_ERR(pages)) { > - int i; > - > - for (i =3D 0; i < frame_vector_count(userptr->vec); i++) > - set_page_dirty_lock(pages[i]); > - } > - put_vaddr_frames(userptr->vec); > - frame_vector_destroy(userptr->vec); > + unpin_user_pages_dirty_lock(userptr->pages, userptr->npages, true= ); > + kvfree(userptr->pages); > > list_del(&userptr->job_node); > > -- > 2.29.2 >