All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "DRI Development" <dri-devel@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Jérôme Glisse" <jglisse@redhat.com>, "Jan Kara" <jack@suse.cz>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Linux MM" <linux-mm@kvack.org>,
	"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
	"Pawel Osciak" <pawel@osciak.com>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Inki Dae" <inki.dae@samsung.com>,
	"Joonyoung Shim" <jy0922.shim@samsung.com>,
	"Seung-Woo Kim" <sw0312.kim@samsung.com>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	"open list:DMA BUFFER SHARING FRAMEWORK"
	<linux-media@vger.kernel.org>,
	"Oded Gabbay" <oded.gabbay@gmail.com>
Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM
Date: Sun, 4 Oct 2020 09:50:59 -0300	[thread overview]
Message-ID: <20201004125059.GP9916@ziepe.ca> (raw)
In-Reply-To: <CAKMK7uFP-XQHUPYeRhPx7tjvjARQiF-os9z9jx6WANV6sgSf6g@mail.gmail.com>

On Sat, Oct 03, 2020 at 11:40:22AM +0200, Daniel Vetter wrote:

> > That leaves the only interesting places as vb2_dc_get_userptr() and
> > vb2_vmalloc_get_userptr() which both completely fail to follow the
> > REQUIRED behavior in the function's comment about checking PTEs. It
> > just DMA maps them. Badly broken.
> >
> > Guessing this hackery is for some embedded P2P DMA transfer?
> 
> Yeah, see also the follow_pfn trickery in
> videobuf_dma_contig_user_get(), I think this is fully intentional and
> userspace abi we can't break :-/

We don't need to break uABI, it just needs to work properly in the
kernel:

  vma = find_vma_intersection()
  dma_buf = dma_buf_get_from_vma(vma)
  sg = dma_buf_p2p_dma_map(dma_buf)
  [.. do dma ..]
  dma_buf_unmap(sg)
  dma_buf_put(dma_buf)

It is as we discussed before, dma buf needs to be discoverable from a
VMA, at least for users doing this kind of stuff.

> Yup this should be done with dma_buf instead, and v4l has that. But
> old uapi and all that. This is why I said we might need a new
> VM_DYNAMIC_PFNMAP or so, to make follow_pfn not resolve this in the
> case where the driver manages the underlying iomem range (or whatever
> it is) dynamically and moves buffer objects around, like drm drivers
> do. But I looked, and we've run out of vma->vm_flags :-(

A VM flag doesn't help - we need to introduce some kind of lifetime,
and that has to be derived from the VMA. It needs data not just a flag

> The other problem is that I also have no real working clue about all
> the VM_* flags and what they all mean, and whether drm drivers set the
> right ones in all cases (they probably don't, but oh well).
> Documentation for this stuff in headers is a bit thin at times.

Yah, I don't really know either :\

The comment above vm_normal_page() is a bit helpful. Don't know what
VM_IO/VM_PFNMAP mean in their 3 combinations

There are very few places that set VM_PFNMAP without VM_IO..

Jason

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Oded Gabbay" <oded.gabbay@gmail.com>,
	"Inki Dae" <inki.dae@samsung.com>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	"Joonyoung Shim" <jy0922.shim@samsung.com>,
	"Pawel Osciak" <pawel@osciak.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Seung-Woo Kim" <sw0312.kim@samsung.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Linux MM" <linux-mm@kvack.org>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"open list:DMA BUFFER SHARING FRAMEWORK"
	<linux-media@vger.kernel.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM
Date: Sun, 4 Oct 2020 09:50:59 -0300	[thread overview]
Message-ID: <20201004125059.GP9916@ziepe.ca> (raw)
In-Reply-To: <CAKMK7uFP-XQHUPYeRhPx7tjvjARQiF-os9z9jx6WANV6sgSf6g@mail.gmail.com>

On Sat, Oct 03, 2020 at 11:40:22AM +0200, Daniel Vetter wrote:

> > That leaves the only interesting places as vb2_dc_get_userptr() and
> > vb2_vmalloc_get_userptr() which both completely fail to follow the
> > REQUIRED behavior in the function's comment about checking PTEs. It
> > just DMA maps them. Badly broken.
> >
> > Guessing this hackery is for some embedded P2P DMA transfer?
> 
> Yeah, see also the follow_pfn trickery in
> videobuf_dma_contig_user_get(), I think this is fully intentional and
> userspace abi we can't break :-/

We don't need to break uABI, it just needs to work properly in the
kernel:

  vma = find_vma_intersection()
  dma_buf = dma_buf_get_from_vma(vma)
  sg = dma_buf_p2p_dma_map(dma_buf)
  [.. do dma ..]
  dma_buf_unmap(sg)
  dma_buf_put(dma_buf)

It is as we discussed before, dma buf needs to be discoverable from a
VMA, at least for users doing this kind of stuff.

> Yup this should be done with dma_buf instead, and v4l has that. But
> old uapi and all that. This is why I said we might need a new
> VM_DYNAMIC_PFNMAP or so, to make follow_pfn not resolve this in the
> case where the driver manages the underlying iomem range (or whatever
> it is) dynamically and moves buffer objects around, like drm drivers
> do. But I looked, and we've run out of vma->vm_flags :-(

A VM flag doesn't help - we need to introduce some kind of lifetime,
and that has to be derived from the VMA. It needs data not just a flag

> The other problem is that I also have no real working clue about all
> the VM_* flags and what they all mean, and whether drm drivers set the
> right ones in all cases (they probably don't, but oh well).
> Documentation for this stuff in headers is a bit thin at times.

Yah, I don't really know either :\

The comment above vm_normal_page() is a bit helpful. Don't know what
VM_IO/VM_PFNMAP mean in their 3 combinations

There are very few places that set VM_PFNMAP without VM_IO..

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	"Joonyoung Shim" <jy0922.shim@samsung.com>,
	"Pawel Osciak" <pawel@osciak.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Seung-Woo Kim" <sw0312.kim@samsung.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Linux MM" <linux-mm@kvack.org>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"open list:DMA BUFFER SHARING FRAMEWORK"
	<linux-media@vger.kernel.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM
Date: Sun, 4 Oct 2020 09:50:59 -0300	[thread overview]
Message-ID: <20201004125059.GP9916@ziepe.ca> (raw)
In-Reply-To: <CAKMK7uFP-XQHUPYeRhPx7tjvjARQiF-os9z9jx6WANV6sgSf6g@mail.gmail.com>

On Sat, Oct 03, 2020 at 11:40:22AM +0200, Daniel Vetter wrote:

> > That leaves the only interesting places as vb2_dc_get_userptr() and
> > vb2_vmalloc_get_userptr() which both completely fail to follow the
> > REQUIRED behavior in the function's comment about checking PTEs. It
> > just DMA maps them. Badly broken.
> >
> > Guessing this hackery is for some embedded P2P DMA transfer?
> 
> Yeah, see also the follow_pfn trickery in
> videobuf_dma_contig_user_get(), I think this is fully intentional and
> userspace abi we can't break :-/

We don't need to break uABI, it just needs to work properly in the
kernel:

  vma = find_vma_intersection()
  dma_buf = dma_buf_get_from_vma(vma)
  sg = dma_buf_p2p_dma_map(dma_buf)
  [.. do dma ..]
  dma_buf_unmap(sg)
  dma_buf_put(dma_buf)

It is as we discussed before, dma buf needs to be discoverable from a
VMA, at least for users doing this kind of stuff.

> Yup this should be done with dma_buf instead, and v4l has that. But
> old uapi and all that. This is why I said we might need a new
> VM_DYNAMIC_PFNMAP or so, to make follow_pfn not resolve this in the
> case where the driver manages the underlying iomem range (or whatever
> it is) dynamically and moves buffer objects around, like drm drivers
> do. But I looked, and we've run out of vma->vm_flags :-(

A VM flag doesn't help - we need to introduce some kind of lifetime,
and that has to be derived from the VMA. It needs data not just a flag

> The other problem is that I also have no real working clue about all
> the VM_* flags and what they all mean, and whether drm drivers set the
> right ones in all cases (they probably don't, but oh well).
> Documentation for this stuff in headers is a bit thin at times.

Yah, I don't really know either :\

The comment above vm_normal_page() is a bit helpful. Don't know what
VM_IO/VM_PFNMAP mean in their 3 combinations

There are very few places that set VM_PFNMAP without VM_IO..

Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-10-04 12:51 UTC|newest]

Thread overview: 156+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 17:53 [PATCH 1/2] mm/frame-vec: Drop gup_flags from get_vaddr_frames() Daniel Vetter
2020-10-02 17:53 ` Daniel Vetter
2020-10-02 17:53 ` Daniel Vetter
2020-10-02 17:53 ` [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM Daniel Vetter
2020-10-02 17:53   ` Daniel Vetter
2020-10-02 17:53   ` Daniel Vetter
2020-10-02 18:06   ` Jason Gunthorpe
2020-10-02 18:06     ` Jason Gunthorpe
2020-10-02 18:06     ` Jason Gunthorpe
2020-10-02 18:16     ` Daniel Vetter
2020-10-02 18:16       ` Daniel Vetter
2020-10-02 18:16       ` Daniel Vetter
2020-10-02 23:31       ` Jason Gunthorpe
2020-10-02 23:31         ` Jason Gunthorpe
2020-10-02 23:31         ` Jason Gunthorpe
2020-10-03  8:34         ` Oded Gabbay
2020-10-03  8:34           ` Oded Gabbay
2020-10-03  8:34           ` Oded Gabbay
2020-10-03  9:40         ` Daniel Vetter
2020-10-03  9:40           ` Daniel Vetter
2020-10-03  9:40           ` Daniel Vetter
2020-10-04 12:50           ` Jason Gunthorpe [this message]
2020-10-04 12:50             ` Jason Gunthorpe
2020-10-04 12:50             ` Jason Gunthorpe
2020-10-04 16:09             ` Daniel Vetter
2020-10-04 16:09               ` Daniel Vetter
2020-10-04 16:09               ` Daniel Vetter
2020-10-05 17:28               ` Jason Gunthorpe
2020-10-05 17:28                 ` Jason Gunthorpe
2020-10-05 17:28                 ` Jason Gunthorpe
2020-10-05 18:16                 ` Daniel Vetter
2020-10-05 18:16                   ` Daniel Vetter
2020-10-05 18:16                   ` Daniel Vetter
2020-10-05 18:37                   ` Jason Gunthorpe
2020-10-05 18:37                     ` Jason Gunthorpe
2020-10-05 18:37                     ` Jason Gunthorpe
2020-10-05 18:54                     ` Daniel Vetter
2020-10-05 18:54                       ` Daniel Vetter
2020-10-05 18:54                       ` Daniel Vetter
2020-10-05 22:43                       ` Daniel Vetter
2020-10-05 22:43                         ` Daniel Vetter
2020-10-05 22:43                         ` Daniel Vetter
2020-10-05 23:41                         ` Jason Gunthorpe
2020-10-05 23:41                           ` Jason Gunthorpe
2020-10-05 23:41                           ` Jason Gunthorpe
2020-10-06  6:23                           ` Daniel Vetter
2020-10-06  6:23                             ` Daniel Vetter
2020-10-06  6:23                             ` Daniel Vetter
2020-10-06 12:26                             ` Jason Gunthorpe
2020-10-06 12:26                               ` Jason Gunthorpe
2020-10-06 12:26                               ` Jason Gunthorpe
2020-10-06 13:08                               ` Daniel Vetter
2020-10-06 13:08                                 ` Daniel Vetter
2020-10-06 13:08                                 ` Daniel Vetter
2020-10-07 10:47           ` Marek Szyprowski
2020-10-07 10:47             ` Marek Szyprowski
2020-10-07 10:47             ` Marek Szyprowski
2020-10-07 12:01             ` Daniel Vetter
2020-10-07 12:01               ` Daniel Vetter
2020-10-07 12:01               ` Daniel Vetter
2020-10-07 12:33               ` Marek Szyprowski
2020-10-07 12:33                 ` Marek Szyprowski
2020-10-07 12:33                 ` Marek Szyprowski
2020-10-07 12:44                 ` Jason Gunthorpe
2020-10-07 12:44                   ` Jason Gunthorpe
2020-10-07 12:44                   ` Jason Gunthorpe
2020-10-07 12:47                   ` Tomasz Figa
2020-10-07 12:47                     ` Tomasz Figa
2020-10-07 12:47                     ` Tomasz Figa
2020-10-07 12:58                     ` Daniel Vetter
2020-10-07 12:58                       ` Daniel Vetter
2020-10-07 12:58                       ` Daniel Vetter
2020-10-07 13:06                       ` Jason Gunthorpe
2020-10-07 13:06                         ` Jason Gunthorpe
2020-10-07 13:06                         ` Jason Gunthorpe
2020-10-07 13:34                         ` Tomasz Figa
2020-10-07 13:34                           ` Tomasz Figa
2020-10-07 13:34                           ` Tomasz Figa
2020-10-07 13:42                           ` Jason Gunthorpe
2020-10-07 13:42                             ` Jason Gunthorpe
2020-10-07 13:42                             ` Jason Gunthorpe
2020-10-07 14:08                           ` Daniel Vetter
2020-10-07 14:08                             ` Daniel Vetter
2020-10-07 14:08                             ` Daniel Vetter
2020-10-07 14:11                             ` Tomasz Figa
2020-10-07 14:11                               ` Tomasz Figa
2020-10-07 14:11                               ` Tomasz Figa
2020-10-07 14:22                               ` Daniel Vetter
2020-10-07 14:22                                 ` Daniel Vetter
2020-10-07 14:22                                 ` Daniel Vetter
2020-10-07 15:05                                 ` Tomasz Figa
2020-10-07 15:05                                   ` Tomasz Figa
2020-10-07 15:05                                   ` Tomasz Figa
2020-10-07 14:58                               ` Jason Gunthorpe
2020-10-07 14:58                                 ` Jason Gunthorpe
2020-10-07 14:58                                 ` Jason Gunthorpe
2020-10-07 13:06         ` Tomasz Figa
2020-10-07 13:06           ` Tomasz Figa
2020-10-07 13:06           ` Tomasz Figa
2020-10-07 13:14           ` Jason Gunthorpe
2020-10-07 13:14             ` Jason Gunthorpe
2020-10-07 13:14             ` Jason Gunthorpe
2020-10-05 15:03     ` Jan Kara
2020-10-05 15:03       ` Jan Kara
2020-10-05 15:03       ` Jan Kara
2020-10-02 22:39   ` John Hubbard
2020-10-02 22:39     ` John Hubbard
2020-10-02 22:39     ` John Hubbard
2020-10-03  9:45     ` Daniel Vetter
2020-10-03  9:45       ` Daniel Vetter
2020-10-03  9:45       ` Daniel Vetter
2020-10-03 22:52       ` John Hubbard
2020-10-03 22:52         ` John Hubbard
2020-10-03 22:52         ` John Hubbard
2020-10-03 23:24         ` Jason Gunthorpe
2020-10-03 23:24           ` Jason Gunthorpe
2020-10-03 23:24           ` Jason Gunthorpe
2020-10-04 11:20           ` Daniel Vetter
2020-10-04 11:20             ` Daniel Vetter
2020-10-04 11:20             ` Daniel Vetter
2020-10-05 17:35             ` Jason Gunthorpe
2020-10-05 17:35               ` Jason Gunthorpe
2020-10-05 17:35               ` Jason Gunthorpe
2020-10-02 18:22 ` [PATCH 1/2] mm/frame-vec: Drop gup_flags from get_vaddr_frames() Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 19:21   ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-05 17:38 [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM Jason Gunthorpe
2020-10-05 17:38 ` Jason Gunthorpe
2020-10-05 17:47 ` Jason Gunthorpe
2020-10-05 17:47   ` Jason Gunthorpe
2020-10-05 17:47   ` Jason Gunthorpe
2020-10-06  3:36   ` Andrew Morton
2020-10-06  3:36     ` Andrew Morton
2020-10-06  3:36     ` Andrew Morton
2020-10-06 11:57     ` Jason Gunthorpe
2020-10-06 11:57       ` Jason Gunthorpe
2020-10-06 11:57       ` Jason Gunthorpe
2020-10-05 17:53 ` Jan Kara
2020-10-05 17:53   ` Jan Kara
2020-10-05 17:53   ` Jan Kara
2020-10-05 17:57   ` Jason Gunthorpe
2020-10-05 17:57     ` Jason Gunthorpe
2020-10-05 17:57     ` Jason Gunthorpe
2020-10-05 18:16     ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-06 11:56     ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201004125059.GP9916@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=inki.dae@samsung.com \
    --cc=jack@suse.cz \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=jy0922.shim@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=oded.gabbay@gmail.com \
    --cc=pawel@osciak.com \
    --cc=sw0312.kim@samsung.com \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.