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=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,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 7E765C433DF for ; Sat, 10 Oct 2020 21:32:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EF9F7207CD for ; Sat, 10 Oct 2020 21:32:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="bkgteCVx" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EF9F7207CD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 429FC6B005C; Sat, 10 Oct 2020 17:32:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3DC2C900002; Sat, 10 Oct 2020 17:32:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2C96E6B0068; Sat, 10 Oct 2020 17:32:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0247.hostedemail.com [216.40.44.247]) by kanga.kvack.org (Postfix) with ESMTP id 00C016B005C for ; Sat, 10 Oct 2020 17:32:16 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 7BF4A362A for ; Sat, 10 Oct 2020 21:32:16 +0000 (UTC) X-FDA: 77357314272.06.wool52_5f12212271ec Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id 47534101580D4 for ; Sat, 10 Oct 2020 21:32:16 +0000 (UTC) X-HE-Tag: wool52_5f12212271ec X-Filterd-Recvd-Size: 11365 Received: from mail-oi1-f193.google.com (mail-oi1-f193.google.com [209.85.167.193]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Sat, 10 Oct 2020 21:32:15 +0000 (UTC) Received: by mail-oi1-f193.google.com with SMTP id c13so14276198oiy.6 for ; Sat, 10 Oct 2020 14:32:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=eOErpfOgEHvqoynno+X7K2KGQQjqKpNf/4DLHM6VCxI=; b=bkgteCVxwM6kN0aEzNJYsf26mAIxhPv2McdrAdqiyH+C70JfkVfDt7/M5rR+rAdLhG cuhWtQlVsFvREMMLlMZEjS9TQsBb7WZQ8UggkyE4P+OvWApOlJjAa2toOrzS1mKuBYLH D1CUxJYWqX5UONNm+6mBvlB1DGdxP0Yv9uoBY= 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=eOErpfOgEHvqoynno+X7K2KGQQjqKpNf/4DLHM6VCxI=; b=jWWSWfP9eGCQk2533na07dwAVdGPxDNf62mB/Gt5rH2NwOExPTBn0TnFIBhuyA8iy6 2BmsFElQxwHMMPEJhSke0i8JxeJkHiysIj7W84L1QdDYGLdCRvRw6UIY1KGTX/guny/C EtF5pJnh+U0Vq76hOnOREM/1yZCm823ImuXVHqjkXPYn6qGmqFFWa1dC4P2xaNqzp3o9 MfauBF+wUQBJFGcM1yf0heGDqz9CMvtwezYHsIdwevjBOfeovn009yUTOyHtVUMZ6Z/U zOvPpQMyP41WezqWuHdNGKpMkt7DhD36+UfVilEiGtXipAAWK3H+0Ywh5jRoBi/lZmhc hdqw== X-Gm-Message-State: AOAM530WR8jFAnBDpe+JN08Qr2SJFkuhedyOTTpL1pQvgxalUPiMOGMC v7TdtaH03TjKnxD8ZpIoOnx1O0DznREjCdd/kV1Gdw== X-Google-Smtp-Source: ABdhPJwlD9CwC0x8+4UqvunFmEbVtlxEXubTPmLCcFCqoFw+1d9HsFP/HM5IpzpxeQb5drq3ZcEO6Uh7XOBwl3Gjidk= X-Received: by 2002:aca:52c4:: with SMTP id g187mr6142563oib.101.1602365534555; Sat, 10 Oct 2020 14:32:14 -0700 (PDT) MIME-Version: 1.0 References: <20201009075934.3509076-1-daniel.vetter@ffwll.ch> <20201009075934.3509076-4-daniel.vetter@ffwll.ch> In-Reply-To: From: Daniel Vetter Date: Sat, 10 Oct 2020 23:32:03 +0200 Message-ID: Subject: Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers To: Oded Gabbay Cc: DRI Development , LKML , KVM list , linux-mm , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , linux-samsung-soc , Linux Media Mailing List , linux-s390 , Daniel Vetter , Jason Gunthorpe , Andrew Morton , John Hubbard , =?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 Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay wrote: > > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter wr= ote: > > > > 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. > > > Thanks for the patch Daniel. > > > Signed-off-by: Daniel Vetter > > 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 > > -- > > v2: Use unpin_user_pages_dirty_lock (John) > > --- > > drivers/misc/habanalabs/Kconfig | 1 - > > drivers/misc/habanalabs/common/habanalabs.h | 3 +- > > drivers/misc/habanalabs/common/memory.c | 49 ++++++++------------- > > 3 files changed, 20 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/= Kconfig > > index 8eb5d38c618e..2f04187f7167 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 DMA_SHARED_BUFFER > > select GENERIC_ALLOCATOR > > select HWMON > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h b/drivers/misc= /habanalabs/common/habanalabs.h > > index edbd627b29d2..c1b3ad613b15 100644 > > --- a/drivers/misc/habanalabs/common/habanalabs.h > > +++ b/drivers/misc/habanalabs/common/habanalabs.h > > @@ -881,7 +881,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; > Can you please update the kerneldoc comment section of this structure > according to your changes ? Apologies I missed the nice kerneldoc. I'll fix that in the next round. > > 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/hab= analabs/common/memory.c > > index 5ff4688683fd..327b64479f97 100644 > > --- a/drivers/misc/habanalabs/common/memory.c > > +++ b/drivers/misc/habanalabs/common/memory.c > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device *hd= ev, u64 addr, u64 size, > > return -EFAULT; > > } > > > > - userptr->vec =3D frame_vector_create(npages); > > - if (!userptr->vec) { > > + userptr->pages =3D kvmalloc_array(npages, sizeof(*userptr->page= s), > > + GFP_KERNEL); > > + if (!userptr->pages) { > > dev_err(hdev->dev, "Failed to create frame vector\n"); > > return -ENOMEM; > > } > > > > - rc =3D get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > > - userptr->vec); > > + rc =3D pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRI= TE, > > + userptr->pages); > > > > if (rc !=3D npages) { > > dev_err(hdev->dev, > > "Failed to map host memory, user ptr probably w= rong\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_ATOMI= C); > > + userptr->pages, > > + npages, offset, size, GFP_ATOMIC= ); > I think that because the call to kvmalloc_array() is done with > GFP_KERNEL, there is no point in using GFP_ATOMIC here. > And actually, this path only needs to avoid yielding when using a > special debug mode. > So I suggest putting here GFP_KERNEL. Huh, I didn't even notice the GFP_ATOMIC here. This looks indeed strange and GFP_KERNEL should be perfectly fine in a function that also calls pin_user_pages (since that one can allocate and do worse stuff like userspace pagefaults). But since that GFP_ATOMIC is there already I'll do that in a separate patch= . > In the meanwhile, I'll run this patch (coupled with the next patch) in > our C/I to make sure there are no regressions. Excellent. I'll wait with v3 until that's done, just in case you hit a snag I need to fix. Cheers, Daniel > Thanks, > Oded > > > if (rc < 0) { > > dev_err(hdev->dev, "failed to create SG table from page= s\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; > > } > > > > @@ -1405,8 +1401,6 @@ int hl_pin_host_memory(struct hl_device *hdev, u6= 4 addr, u64 size, > > */ > > void hl_unpin_host_memory(struct hl_device *hdev, struct hl_userptr *u= serptr) > > { > > - struct page **pages; > > - > > hl_debugfs_remove_userptr(hdev, userptr); > > > > if (userptr->dma_mapped) > > @@ -1414,15 +1408,8 @@ void hl_unpin_host_memory(struct hl_device *hdev= , struct hl_userptr *userptr) > > userptr->sgt->n= ents, > > 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, tr= ue); > > + kvfree(userptr->pages); > > > > list_del(&userptr->job_node); > > > > -- > > 2.28.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch