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=-14.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 36357C2D0A3 for ; Mon, 2 Nov 2020 18:19:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7B1002222B for ; Mon, 2 Nov 2020 18:19:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="hn3DSEKB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7B1002222B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AC0086B0036; Mon, 2 Nov 2020 13:19:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A95EA6B005C; Mon, 2 Nov 2020 13:19:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 937476B0068; Mon, 2 Nov 2020 13:19:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0173.hostedemail.com [216.40.44.173]) by kanga.kvack.org (Postfix) with ESMTP id 675706B0036 for ; Mon, 2 Nov 2020 13:19:30 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id E7E3C3632 for ; Mon, 2 Nov 2020 18:19:29 +0000 (UTC) X-FDA: 77440290858.23.road76_560e2b3272b2 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id D4BD537619 for ; Mon, 2 Nov 2020 18:19:26 +0000 (UTC) X-HE-Tag: road76_560e2b3272b2 X-Filterd-Recvd-Size: 13578 Received: from mail-ej1-f67.google.com (mail-ej1-f67.google.com [209.85.218.67]) by imf08.hostedemail.com (Postfix) with ESMTP for ; Mon, 2 Nov 2020 18:19:26 +0000 (UTC) Received: by mail-ej1-f67.google.com with SMTP id oq3so18330623ejb.7 for ; Mon, 02 Nov 2020 10:19:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=WiRUc29zxqGsO+d9X8MH7HOz5SXqo6HQakNFN6TG8oI=; b=hn3DSEKB/RMDVIl8+uf/cXIPRq4hEnoahMtBDSqMYwZ0yyCmlA1wr7KtKFj+kR7qmG LPKy+PtIfaItp4eIhNoIYgsEJUlT7mzBAcbZtb+5c4m0QBqJRtz8oSrtjxPOo7XWsTSc 2iuELeBrCeMyN6mj2Jj2ucEo0YcivEK6yk290= 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=WiRUc29zxqGsO+d9X8MH7HOz5SXqo6HQakNFN6TG8oI=; b=GLEcr/htLbLJ7kJLLdWTttjpXldzaJ0yTXI/91NSuWXUQNy+L3b/e0FhOgZVfnKijo AOnp5vAyqnGaWWNtCG/5txaqmKKUQGaMNawIfy+KqLnqhjlHhIHpntjjewN2S2/ob4L0 xz4zOICmFFxu8U8ltSVvWR+vJum6PAMTFWCVOGHlRGUIRVAvi8YD3bxjujazg4RkWgV6 /9hZ8bP4HZW+qwP707JZ7BrTqBORdDyhf3Deu4o5XG0WRAxs02eP4Y2CKJBUQe7AKH2M zIcI3d0QuYJfNdKfLR5+ei3ASQQ+D4/aFVNp06wukRYQZ2XEF5lZ/jvQupIQIxBIPOSD K3sA== X-Gm-Message-State: AOAM532QWx2kNgOHaNQzsmN3Mmq2pAcVCU277sTNalun8xxmLkYvOQxg U6xyCQ++lQcCTd1ZWoNiOm8VKWzV+c6f5A== X-Google-Smtp-Source: ABdhPJz9RKY4GHUC+ZAsOKu2A05KI0oDbIR7CN3VRNB2GFmUfUHe7YAVkGlYQ0z4oPnzvqsfA+FQBg== X-Received: by 2002:a17:906:14d1:: with SMTP id y17mr17015890ejc.15.1604341164555; Mon, 02 Nov 2020 10:19:24 -0800 (PST) Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com. [209.85.221.46]) by smtp.gmail.com with ESMTPSA id g7sm7884699edl.5.2020.11.02.10.19.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Nov 2020 10:19:23 -0800 (PST) Received: by mail-wr1-f46.google.com with SMTP id s9so15716792wro.8 for ; Mon, 02 Nov 2020 10:19:23 -0800 (PST) X-Received: by 2002:adf:f511:: with SMTP id q17mr21106249wro.192.1604341162894; Mon, 02 Nov 2020 10:19:22 -0800 (PST) MIME-Version: 1.0 References: <20201030100815.2269-1-daniel.vetter@ffwll.ch> <20201030100815.2269-6-daniel.vetter@ffwll.ch> In-Reply-To: From: Tomasz Figa Date: Mon, 2 Nov 2020 19:19:10 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM To: Daniel Vetter Cc: =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , linux-samsung-soc , Jan Kara , Pawel Osciak , kvm , Jason Gunthorpe , John Hubbard , Mauro Carvalho Chehab , LKML , DRI Development , Linux MM , Kyungmin Park , Daniel Vetter , Andrew Morton , Marek Szyprowski , Dan Williams , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Linux Media Mailing List 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 Fri, Oct 30, 2020 at 3:38 PM Daniel Vetter wrot= e: > > On Fri, Oct 30, 2020 at 3:11 PM Tomasz Figa wrote: > > > > On Fri, Oct 30, 2020 at 11:08 AM Daniel Vetter = wrote: > > > > > > This is used by media/videbuf2 for persistent dma mappings, not just > > > for a single dma operation and then freed again, so needs > > > FOLL_LONGTERM. > > > > > > Unfortunately current pup_locked doesn't support FOLL_LONGTERM due to > > > locking issues. Rework the code to pull the pup path out from the > > > mmap_sem critical section as suggested by Jason. > > > > > > By relying entirely on the vma checks in pin_user_pages and follow_pf= n > > > (for vm_flags and vma_is_fsdax) we can also streamline the code a lot= . > > > > > > Signed-off-by: Daniel Vetter > > > Cc: Jason Gunthorpe > > > Cc: Pawel Osciak > > > Cc: Marek Szyprowski > > > Cc: Kyungmin Park > > > Cc: Tomasz Figa > > > Cc: Mauro Carvalho Chehab > > > 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 > > > Signed-off-by: Daniel Vetter > > > -- > > > v2: Streamline the code and further simplify the loop checks (Jason) > > > > > > v5: Review from Tomasz: > > > - fix page counting for the follow_pfn case by resetting ret > > > - drop gup_flags paramater, now unused > > > --- > > > .../media/common/videobuf2/videobuf2-memops.c | 3 +- > > > include/linux/mm.h | 2 +- > > > mm/frame_vector.c | 53 ++++++-----------= -- > > > 3 files changed, 19 insertions(+), 39 deletions(-) > > > > > > > Thanks, looks good to me now. > > > > Acked-by: Tomasz Figa > > > > From reading the code, this is quite unlikely to introduce any > > behavior changes, but just to be safe, did you have a chance to test > > this with some V4L2 driver? > > Nah, unfortunately not. I believe we don't have any setup that could exercise the IO/PFNMAP user pointers, but it should be possible to exercise the basic userptr path by enabling the virtual (fake) video driver, vivid or CONFIG_VIDEO_VIVID, in your kernel and then using yavta [1] with --userptr and --capture=3D (and possibly some more options) to grab a couple of frames from the test pattern generator. Does it sound like something that you could give a try? Feel free to ping me on IRC (tfiga on #v4l or #dri-devel) if you need any help. [1] https://git.ideasonboard.org/yavta.git Best regards, Tomasz > -Daniel > > > > > Best regards, > > Tomasz > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-memops.c b/driv= ers/media/common/videobuf2/videobuf2-memops.c > > > index 6e9e05153f4e..9dd6c27162f4 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-memops.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-memops.c > > > @@ -40,7 +40,6 @@ struct frame_vector *vb2_create_framevec(unsigned l= ong start, > > > unsigned long first, last; > > > unsigned long nr; > > > struct frame_vector *vec; > > > - unsigned int flags =3D FOLL_FORCE | FOLL_WRITE; > > > > > > first =3D start >> PAGE_SHIFT; > > > last =3D (start + length - 1) >> PAGE_SHIFT; > > > @@ -48,7 +47,7 @@ struct frame_vector *vb2_create_framevec(unsigned l= ong start, > > > vec =3D frame_vector_create(nr); > > > if (!vec) > > > return ERR_PTR(-ENOMEM); > > > - ret =3D get_vaddr_frames(start & PAGE_MASK, nr, flags, vec); > > > + ret =3D get_vaddr_frames(start & PAGE_MASK, nr, vec); > > > if (ret < 0) > > > goto out_destroy; > > > /* We accept only complete set of PFNs */ > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > > index ef360fe70aaf..d6b8e30dce2e 100644 > > > --- a/include/linux/mm.h > > > +++ b/include/linux/mm.h > > > @@ -1765,7 +1765,7 @@ struct frame_vector { > > > struct frame_vector *frame_vector_create(unsigned int nr_frames); > > > void frame_vector_destroy(struct frame_vector *vec); > > > int get_vaddr_frames(unsigned long start, unsigned int nr_pfns, > > > - unsigned int gup_flags, struct frame_vector *vec= ); > > > + struct frame_vector *vec); > > > void put_vaddr_frames(struct frame_vector *vec); > > > int frame_vector_to_pages(struct frame_vector *vec); > > > void frame_vector_to_pfns(struct frame_vector *vec); > > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > > index 10f82d5643b6..f8c34b895c76 100644 > > > --- a/mm/frame_vector.c > > > +++ b/mm/frame_vector.c > > > @@ -32,13 +32,12 @@ > > > * This function takes care of grabbing mmap_lock as necessary. > > > */ > > > int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > - unsigned int gup_flags, struct frame_vector *vec= ) > > > + struct frame_vector *vec) > > > { > > > struct mm_struct *mm =3D current->mm; > > > struct vm_area_struct *vma; > > > int ret =3D 0; > > > int err; > > > - int locked; > > > > > > if (nr_frames =3D=3D 0) > > > return 0; > > > @@ -48,40 +47,26 @@ int get_vaddr_frames(unsigned long start, unsigne= d int nr_frames, > > > > > > start =3D untagged_addr(start); > > > > > > - mmap_read_lock(mm); > > > - locked =3D 1; > > > - vma =3D find_vma_intersection(mm, start, start + 1); > > > - if (!vma) { > > > - ret =3D -EFAULT; > > > - goto out; > > > - } > > > - > > > - /* > > > - * While get_vaddr_frames() could be used for transient (kern= el > > > - * controlled lifetime) pinning of memory pages all current > > > - * users establish long term (userspace controlled lifetime) > > > - * page pinning. Treat get_vaddr_frames() like > > > - * get_user_pages_longterm() and disallow it for filesystem-d= ax > > > - * mappings. > > > - */ > > > - if (vma_is_fsdax(vma)) { > > > - ret =3D -EOPNOTSUPP; > > > - goto out; > > > - } > > > - > > > - if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > + ret =3D pin_user_pages_fast(start, nr_frames, > > > + FOLL_FORCE | FOLL_WRITE | FOLL_LONG= TERM, > > > + (struct page **)(vec->ptrs)); > > > + if (ret > 0) { > > > vec->got_ref =3D true; > > > vec->is_pfns =3D false; > > > - ret =3D pin_user_pages_locked(start, nr_frames, > > > - gup_flags, (struct page **)(vec->ptrs), &lock= ed); > > > - goto out; > > > + goto out_unlocked; > > > } > > > > > > + mmap_read_lock(mm); > > > vec->got_ref =3D false; > > > vec->is_pfns =3D true; > > > + ret =3D 0; > > > do { > > > unsigned long *nums =3D frame_vector_pfns(vec); > > > > > > + vma =3D find_vma_intersection(mm, start, start + 1); > > > + if (!vma) > > > + break; > > > + > > > while (ret < nr_frames && start + PAGE_SIZE <=3D vma-= >vm_end) { > > > err =3D follow_pfn(vma, start, &nums[ret]); > > > if (err) { > > > @@ -92,17 +77,13 @@ int get_vaddr_frames(unsigned long start, unsigne= d int nr_frames, > > > start +=3D PAGE_SIZE; > > > ret++; > > > } > > > - /* > > > - * We stop if we have enough pages or if VMA doesn't = completely > > > - * cover the tail page. > > > - */ > > > - if (ret >=3D nr_frames || start < vma->vm_end) > > > + /* Bail out if VMA doesn't completely cover the tail = page. */ > > > + if (start < vma->vm_end) > > > break; > > > - vma =3D find_vma_intersection(mm, start, start + 1); > > > - } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > > > + } while (ret < nr_frames); > > > out: > > > - if (locked) > > > - mmap_read_unlock(mm); > > > + mmap_read_unlock(mm); > > > +out_unlocked: > > > if (!ret) > > > ret =3D -EFAULT; > > > if (ret > 0) > > > -- > > > 2.28.0 > > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch