[PATCH] media: videobuf2: mark pinned pages dirty when unpinning them
From: Tal Zussman
Date: Sat Oct 31 2026 - 12:48:51 EST
The three USERPTR release paths each loop over the pinned pages with
set_page_dirty_lock() before vb2_destroy_framevec() unpins them. Use
unpin_user_pages_dirty_lock() instead by passing a dirty argument to
put_vaddr_frames(). unpin_user_pages_dirty_lock() works a folio at a
time, so a pinned THP is marked dirty once rather than once per page.
This removes the last callers of set_page_dirty_lock() in videobuf2.
Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
---
drivers/media/common/videobuf2/frame_vector.c | 11 ++++++-----
drivers/media/common/videobuf2/videobuf2-dma-contig.c | 15 +++------------
drivers/media/common/videobuf2/videobuf2-dma-sg.c | 10 +++-------
drivers/media/common/videobuf2/videobuf2-memops.c | 10 ++++++----
drivers/media/common/videobuf2/videobuf2-vmalloc.c | 14 +++-----------
include/media/frame_vector.h | 2 +-
include/media/videobuf2-memops.h | 2 +-
7 files changed, 23 insertions(+), 41 deletions(-)
diff --git a/drivers/media/common/videobuf2/frame_vector.c b/drivers/media/common/videobuf2/frame_vector.c
index 41f289c75cbb..d4da45cc5624 100644
--- a/drivers/media/common/videobuf2/frame_vector.c
+++ b/drivers/media/common/videobuf2/frame_vector.c
@@ -72,12 +72,13 @@ EXPORT_SYMBOL(get_vaddr_frames);
* put_vaddr_frames() - drop references to pages if get_vaddr_frames() acquired
* them
* @vec: frame vector to put
+ * @dirty: whether to mark the pages dirty first
*
- * Drop references to pages if get_vaddr_frames() acquired them. We also
- * invalidate the frame vector so that it is prepared for the next call into
- * get_vaddr_frames().
+ * Drop references to pages if get_vaddr_frames() acquired them, marking them
+ * dirty first if @dirty is set. We also invalidate the frame vector so that it
+ * is prepared for the next call into get_vaddr_frames().
*/
-void put_vaddr_frames(struct frame_vector *vec)
+void put_vaddr_frames(struct frame_vector *vec, bool dirty)
{
struct page **pages;
@@ -92,7 +93,7 @@ void put_vaddr_frames(struct frame_vector *vec)
if (WARN_ON(IS_ERR(pages)))
goto out;
- unpin_user_pages(pages, vec->nr_frames);
+ unpin_user_pages_dirty_lock(pages, vec->nr_frames, dirty);
vec->got_ref = false;
out:
vec->nr_frames = 0;
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 9ce6284cd5f2..94e00e6bc0fb 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -533,8 +533,6 @@ static void vb2_dc_put_userptr(void *buf_priv)
{
struct vb2_dc_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
- int i;
- struct page **pages;
if (sgt) {
/*
@@ -543,21 +541,14 @@ static void vb2_dc_put_userptr(void *buf_priv)
*/
dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
DMA_ATTR_SKIP_CPU_SYNC);
- if (buf->dma_dir == DMA_FROM_DEVICE ||
- buf->dma_dir == DMA_BIDIRECTIONAL) {
- pages = frame_vector_pages(buf->vec);
- /* sgt should exist only if vector contains pages... */
- if (!WARN_ON_ONCE(IS_ERR(pages)))
- for (i = 0; i < frame_vector_count(buf->vec); i++)
- set_page_dirty_lock(pages[i]);
- }
sg_free_table(sgt);
kfree(sgt);
} else {
dma_unmap_resource(buf->dev, buf->dma_addr, buf->size,
buf->dma_dir, 0);
}
- vb2_destroy_framevec(buf->vec);
+ vb2_destroy_framevec(buf->vec, buf->dma_dir == DMA_FROM_DEVICE ||
+ buf->dma_dir == DMA_BIDIRECTIONAL);
kfree(buf);
}
@@ -676,7 +667,7 @@ static void *vb2_dc_get_userptr(struct vb2_buffer *vb, struct device *dev,
kfree(sgt);
fail_pfnvec:
- vb2_destroy_framevec(vec);
+ vb2_destroy_framevec(vec, false);
fail_buf:
kfree(buf);
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index b1d0695cda26..c80d3232436d 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -271,7 +271,7 @@ static void *vb2_dma_sg_get_userptr(struct vb2_buffer *vb, struct device *dev,
userptr_fail_map:
sg_free_table(&buf->sg_table);
userptr_fail_sgtable:
- vb2_destroy_framevec(vec);
+ vb2_destroy_framevec(vec, false);
userptr_fail_pfnvec:
kfree(buf);
return ERR_PTR(-ENOMEM);
@@ -285,7 +285,6 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
{
struct vb2_dma_sg_buf *buf = buf_priv;
struct sg_table *sgt = &buf->sg_table;
- int i = buf->num_pages;
dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
__func__, buf->num_pages);
@@ -293,11 +292,8 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->num_pages);
sg_free_table(buf->dma_sgt);
- if (buf->dma_dir == DMA_FROM_DEVICE ||
- buf->dma_dir == DMA_BIDIRECTIONAL)
- while (--i >= 0)
- set_page_dirty_lock(buf->pages[i]);
- vb2_destroy_framevec(buf->vec);
+ vb2_destroy_framevec(buf->vec, buf->dma_dir == DMA_FROM_DEVICE ||
+ buf->dma_dir == DMA_BIDIRECTIONAL);
kfree(buf);
}
diff --git a/drivers/media/common/videobuf2/videobuf2-memops.c b/drivers/media/common/videobuf2/videobuf2-memops.c
index f9a4ec44422e..605e24aa710b 100644
--- a/drivers/media/common/videobuf2/videobuf2-memops.c
+++ b/drivers/media/common/videobuf2/videobuf2-memops.c
@@ -59,7 +59,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
}
return vec;
out_release:
- put_vaddr_frames(vec);
+ put_vaddr_frames(vec, false);
out_destroy:
frame_vector_destroy(vec);
return ERR_PTR(ret);
@@ -69,13 +69,15 @@ EXPORT_SYMBOL(vb2_create_framevec);
/**
* vb2_destroy_framevec() - release vector of mapped pfns
* @vec: vector of pfns / pages to release
+ * @dirty: whether to mark the pages dirty first
*
* This releases references to all pages in the vector @vec (if corresponding
- * pfns are backed by pages) and frees the passed vector.
+ * pfns are backed by pages), marking them dirty first if @dirty is set, and
+ * frees the passed vector.
*/
-void vb2_destroy_framevec(struct frame_vector *vec)
+void vb2_destroy_framevec(struct frame_vector *vec, bool dirty)
{
- put_vaddr_frames(vec);
+ put_vaddr_frames(vec, dirty);
frame_vector_destroy(vec);
}
EXPORT_SYMBOL(vb2_destroy_framevec);
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index 63f023696f3e..bf6273f3bc86 100644
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -116,7 +116,7 @@ static void *vb2_vmalloc_get_userptr(struct vb2_buffer *vb, struct device *dev,
return buf;
fail_map:
- vb2_destroy_framevec(vec);
+ vb2_destroy_framevec(vec, false);
fail_pfnvec_create:
kfree(buf);
@@ -127,25 +127,17 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
{
struct vb2_vmalloc_buf *buf = buf_priv;
unsigned long vaddr = (unsigned long)buf->vaddr & PAGE_MASK;
- unsigned int i;
- struct page **pages;
unsigned int n_pages;
if (!buf->vec->is_pfns) {
n_pages = frame_vector_count(buf->vec);
if (vaddr)
vm_unmap_ram((void *)vaddr, n_pages);
- if (buf->dma_dir == DMA_FROM_DEVICE ||
- buf->dma_dir == DMA_BIDIRECTIONAL) {
- pages = frame_vector_pages(buf->vec);
- if (!WARN_ON_ONCE(IS_ERR(pages)))
- for (i = 0; i < n_pages; i++)
- set_page_dirty_lock(pages[i]);
- }
} else {
iounmap((__force void __iomem *)buf->vaddr);
}
- vb2_destroy_framevec(buf->vec);
+ vb2_destroy_framevec(buf->vec, buf->dma_dir == DMA_FROM_DEVICE ||
+ buf->dma_dir == DMA_BIDIRECTIONAL);
kfree(buf);
}
diff --git a/include/media/frame_vector.h b/include/media/frame_vector.h
index 541c71a2c7be..daf40b9b3d3d 100644
--- a/include/media/frame_vector.h
+++ b/include/media/frame_vector.h
@@ -17,7 +17,7 @@ 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,
bool write, struct frame_vector *vec);
-void put_vaddr_frames(struct frame_vector *vec);
+void put_vaddr_frames(struct frame_vector *vec, bool dirty);
int frame_vector_to_pages(struct frame_vector *vec);
void frame_vector_to_pfns(struct frame_vector *vec);
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index 4b5b84f93538..8a66ec46bc71 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -36,6 +36,6 @@ extern const struct vm_operations_struct vb2_common_vm_ops;
struct frame_vector *vb2_create_framevec(unsigned long start,
unsigned long length,
bool write);
-void vb2_destroy_framevec(struct frame_vector *vec);
+void vb2_destroy_framevec(struct frame_vector *vec, bool dirty);
#endif
---
base-commit: 27285b099fbc5cb2ae3aef20f3bbd7b3c88b3005
change-id: 20261015-vb2-unpin-dirty-912660272e32
Best regards,
--
Tal Zussman <tz2294@xxxxxxxxxxxx>