On Tue, Jul 28, 2026 at 10:29:14PM +0100, Pavel Begunkov wrote:
> Introduce a new iterator type for dmabuf maps. The map in an opaque
> object with internals and format specific to the subsystem / driver, and
> only it can use that subsystem / driver for issuing IO. The task of the
> middle layers is to pass the map / iterator further down, maybe doing
> basic splitting and length checking. The iterator can only be used by
> operations of the file the associated map was created for.
>
> Suggested-by: Keith Busch <kbusch(a)kernel.org>
> Signed-off-by: Pavel Begunkov <asml.silence(a)gmail.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
I'm also pretty sure I reviewed this and vairous other patches
before..
The method name feels a bit convoluted, but given all the
previous discussions I don't care too strongly. I'll leave
the dma-buf side review to those who understand it.
Hi Robert,
> Subject: [PATCH v2] dma-buf/udmabuf: Disable the size limit by default
>
> As udmabuf increasingly enjoys popularity - being used in projects like
> libcamera, Gstreamer, Mesa, KWin and Weston - users more frequently
> encounter cases where the current default size limit of 64MB is too low.
> Examples include allocating video buffers at a 8K resolution - and even
> 4K
> is affected when using non-subsampled video formats and high bit
> depths.
>
> In its current form the size limit for individual buffers does not seem to
> provide any additional level of protection - such as limiting the amount
> of
> memory a process can pin - as the later can just allocate multiple
> buffers.
> If additional guardrails are desired, they would likely require some kind
> accounting not limited to individual buffers.
>
> Therefor let's disable the size limit by default by setting it to the
> maximal possible value, INT_MAX.
>
> Signed-off-by: Robert Mader <robert.mader(a)collabora.com>
>
> ---
>
> Changes in V2:
> - Use INT_MAX instead of 0 in order to not change behavior otherwise.
>
> See
> https://lore.kernel.org/dri-devel/20260711144814.8205-1-
> robert.mader(a)collabora.com/
> for a previous attempt to make the value configurable via kconfig - and
> in particular
> https://lore.kernel.org/dri-devel/6764ca6f-b4d8-4baa-9d27-
> 2ca867ac2d41(a)amd.com/
> for the suggestion and discussion to remove the default limit.
> ---
> drivers/dma-buf/udmabuf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index bced421c0d65..639e93704924 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -20,9 +20,9 @@ static int list_limit = 1024;
> module_param(list_limit, int, 0644);
> MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit.
> Default is 1024.");
>
> -static int size_limit_mb = 64;
> +static int size_limit_mb = INT_MAX;
> module_param(size_limit_mb, int, 0644);
> -MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in
> megabytes. Default is 64.");
> +MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in
> megabytes. Default is INT_MAX.");
Acked-by: Vivek Kasireddy <vivek.kasireddy(a)intel.com>
If there are no further concerns/questions from anyone, I'll push it to
drm-misc-next soon.
Thanks,
Vivek
>
> struct udmabuf {
> pgoff_t pagecount;
> --
> 2.55.0
Hello Baineng,
On Mon, 27 Jul 2026 at 08:56, 寿柏能 <shoubaineng(a)gmail.com> wrote:
>
> Ping. Any chance this can be picked up?
Thank you for the patch.
While discussing with T J, we realised that adding a selftest for this
would also be helpful. Could you please re-submit with a selftest
added?
>
> Baineng
Thanks and best regards,
Sumit.
>
> 寿柏能 <shoubaineng(a)gmail.com> 于2026年7月15日周三 10:04写道:
>>
>> Hi David
>>
>> The rollback here is the simplest possible case (put_unused_fd +
>> dma_buf_put), not the complex sockopt scenario you describe.
>>
>> Baineng
>>
>> David Laight <david.laight.linux(a)gmail.com> 于2026年7月14日周二 22:33写道:
>>>
>>> On Tue, 14 Jul 2026 21:38:07 +0800
>>> 寿柏能 <shoubaineng(a)gmail.com> wrote:
>>>
>>> > Hi David,
>>> >
>>> > Thanks for the feedback.
>>> >
>>> > The concern is not just about the EFAULT return — it's about the race
>>> > window between fd_install() and copy_to_user(). Once fd_install()
>>> > returns, the fd is immediately observable by other threads in the same
>>> > process (via /proc/self/fd, SCM_RIGHTS, etc.), even before
>>> > copy_to_user() has a chance to fail. The triggering condition is a
>>> > deliberate mprotect() flip, not a corrupted heap.
>>>
>>> That is what makes doing the close wrong.
>>> But that is a program aggressively trying to hit the timing window,
>>> not a normal program that has managed to pass an invalid pointer.
>>> The most likely reason for a real program passing an invalid pointer
>>> is a corrupted heap (assuming the stupid coding errors are fixed).
>>>
>>> It is really no different from the sockopt code that receives
>>> SCM_RIGHTS messages.
>>> In that case once you've removed the FILE from the socket (or similar)
>>> you really don't want to have to put it back because the write to the
>>> sockopt buffer or length field fails.
>>> The chance of correctly reverting the kernel state is small - and won't
>>> be tested.
>>>
>>> David
>>>
>>> >
>>> > The fix itself is small and follows the standard kernel idiom:
>>> > get_unused_fd_flags() reserves the fd without publishing it, so the
>>> > window between reservation and install is entirely under kernel control.
>>> >
>>> > Baineng
>>> >
>>> > David Laight <david.laight.linux(a)gmail.com> 于2026年7月14日周二 21:14写道:
>>> >
>>> > > On Tue, 14 Jul 2026 19:46:53 +0800
>>> > > Baineng Shou <shoubaineng(a)gmail.com> wrote:
>>> > >
>>> > > > DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
>>> > > > caller's fd table via dma_buf_fd() -> fd_install() before
>>> > > > dma_heap_ioctl() copies the result back to userspace. If the trailing
>>> > > > copy_to_user() fails, userspace never learns the fd number, but the
>>> > > > fd (and the underlying dma-buf reference) are already visible to
>>> > > > other threads in the same process and are leaked for the lifetime of
>>> > > > the process.
>>> > > >
>>> > > > The obvious "close it on the failure path" fix is unsafe: once
>>> > > > fd_install() has run, another thread can already dup() the fd, send
>>> > > > it via SCM_RIGHTS, or close() it and let its number be reused, so a
>>> > > > subsequent close_fd() from the ioctl path can operate on an unrelated
>>> > > > file. This was pointed out by Christian König on v1 [1].
>>> > > ...
>>> > >
>>> > > My 2c:
>>> > >
>>> > > The other option is just to leave it as a 'problem for user space'.
>>> > > No reasonable program is going to handle the EFAULT return by doing
>>> > > anything other than exiting.
>>> > > Even getting an EFAULT is really an indication that the application
>>> > > is already in a real mess - most likely with a badly corrupted heap.
>>> > >
>>> > > Anything else leaves error recovery code in the kernel that is pretty
>>> > > much never executed and open to a variety of bugs.
>>> > > While the recovery here is probably ok, there are some sockopt calls
>>> > > where it is all more complicated.
>>> > >
>>> > > David
>>> > >
>>>
On Fri, 24 Jul 2026 14:41:47 -0700 Stanislav Fomichev wrote:
> > > Talked to Stan and he mentioned the original idea was not necessarily
> > > just for link down, but any test that could be considered disruptive to
> > > other tests, with the idea to eventually introduce some scheduling
> > > (postpone disruptive until the end).
> >
> > Perhaps have such conversations on the list? IDK what Stan has in mind.
>
> I don't have much to add so I didn't post on the list. In my mind, disruptive
> started as "anything that can lead to a soft reset". But since the
> sockets tend to survive these well, I do agree that your definition
> of "link goes down" is a better fit.
My recollection was basically that the test is not safe to run over
SSH (without tmux). Hence the link down concern. If we say soft reset
that'd cover 95% of the tests so it's not very useful.
The existing doc and README already document the link down AFAICT.
On Fri, 24 Jul 2026 14:04:24 -0700 Bobby Eshleman wrote:
> On Tue, Jul 21, 2026 at 11:09:03AM -0700, Jakub Kicinski wrote:
> > On Wed, 08 Jul 2026 15:55:06 -0700 Bobby Eshleman wrote:
> > > +@ksft_disruptive
> > > +def check_rx_large_niov(cfg) -> None:
> > > + """Run the devmem RX test with rx-buf-size = 16 KiB."""
> > > + run_rx_large_niov(cfg)
> >
> > Any idea why the devmem tests sprinkle disruptive everywhere?
> > Disruptive means we take the link down, I don't think this test
> > does that.
>
> Talked to Stan and he mentioned the original idea was not necessarily
> just for link down, but any test that could be considered disruptive to
> other tests, with the idea to eventually introduce some scheduling
> (postpone disruptive until the end).
Perhaps have such conversations on the list? IDK what Stan has in mind.
> If we can get common agreement on its meaning, maybe I could submit
> something into the documentation and update where its misused?
>
> I'll drop it from this patch since it doesn't bring down the link (and
> seemingly no worries about check_rx_hds() tests not having it).
On Fri, 24 Jul 2026 09:39:54 -0700 Bobby Eshleman wrote:
> > BTW did you add both min and max checks? Cause the only risk with using
> > a dummy value would be that the policy will be rendered inline, and
> > inline policy is u16 so 64k wouldn't fit. But your sample above has a
> > max of u32_max which forces the out-of-line policy, which is what we
> > want.
>
> Yep, u32_max:
>
> + name: rx-page-size
> ...
> + checks:
> + min: page-size
> + max: u32-max
>
> Sorry, probably should have just sent the whole patch instead of
> replying hunk-by-hunk.
Ack, LG, just double checking.
> BTW, how expressive do we want these policies? For example, would
> absorbing the power_of_2 check into a policy be valid in the future? or
> is that too bespoke?
Power-of-2 could be useful (it's implicitly one bit set, which is also
potentially useful for validating one-hot flags). The trickiness is
combining power-of-2 and the min check :S We have one validation per
field. I was wondering if we would be better off defining the field
as a shift instead, then we only have to check min. But I thought
that it'd be a little unusual for uAPI and possibly maybe one day
we will want the non-power of 2? So I figured checking min using
the existing facilities and open coding power of two check is good
enough for now.