Linux virtualization list
 help / color / mirror / Atom feed

* [PATCH 27/60] drm/qxl: Convert to atomic_create_state
From: Maxime Ripard @ 2026-07-09 11:50 UTC (permalink / raw)
  To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, Maxime Ripard, airlied, kraxel, spice-devel,
	virtualization
In-Reply-To: <20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org>

The plane only initializes a pristine state in its reset hook
using drm_atomic_helper_plane_reset(), which is equivalent to what
atomic_create_state expects. Convert to it.

The conversion was done using the following Coccinelle semantic patch:

@@
identifier funcs;
symbol drm_atomic_helper_plane_reset;
symbol drm_atomic_helper_plane_create_state;
@@

struct drm_plane_funcs funcs = {
  ...,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
  ...,
};

@match_struct_reset@
identifier funcs, reset_func;
@@
struct drm_plane_funcs funcs = {
    ...,
    .reset = reset_func,
    ...,
};

@reset_uses_helpers depends on match_struct_reset@
identifier match_struct_reset.reset_func;
@@

 void reset_func(...)
 {
 	<+...
(
 	__drm_atomic_helper_plane_reset(...);
|
	__drm_gem_reset_shadow_plane(...);
)
 	...+>
 }

@match_struct_destroy@
identifier funcs, destroy_func;
@@
struct drm_plane_funcs funcs = {
    ...,
    .atomic_destroy_state = destroy_func,
    ...,
};

@script:python renamed_func@
old_name << match_struct_reset.reset_func;
new_name;
@@
if old_name.endswith("_reset"):
    coccinelle.new_name = old_name.replace("_reset", "_create_state")
else:
    coccinelle.new_name = old_name

@update_struct depends on match_struct_reset && reset_uses_helpers@
identifier match_struct_reset.funcs, match_struct_reset.reset_func;
identifier renamed_func.new_name;
@@
struct drm_plane_funcs funcs = {
    ...,
-   .reset = reset_func,
+   .atomic_create_state = new_name,
    ...,
};

@drop_destroy depends on update_struct && match_struct_destroy@
identifier match_struct_reset.reset_func;
identifier match_struct_destroy.destroy_func;
identifier container_func;
identifier P;
symbol drm_atomic_helper_plane_destroy_state;
symbol __drm_atomic_helper_plane_destroy_state;
@@

 void reset_func(struct drm_plane *P)
 {
 	...
(
-	if (P->state) {
- 		<+...
(
-		drm_atomic_helper_plane_destroy_state(P, P->state);
|
-		__drm_atomic_helper_plane_destroy_state(P->state);
|
-		P->funcs->atomic_destroy_state(P, P->state);
|
-		destroy_func(P, P->state);
)
- 		...+>
- 	}
|
-	drm_WARN_ON_ONCE(P->dev, P->state);
|
-	WARN_ON(P->state);
)
 	...
(
-	kfree(P->state);
|
-	kfree(container_func(P->state));
|
 	// kfree is optional
)
(
-	P->state = NULL;
|
 	// plane->state clearing is optional
)
 	...
 }

@drop_destroy_mtk depends on update_struct@
identifier P;
symbol __drm_atomic_helper_plane_destroy_state;
symbol to_mtk_plane_state;
@@

 void mtk_plane_reset(struct drm_plane *P)
 {
 	...
-	if (P->state) {
-		__drm_atomic_helper_plane_destroy_state(P->state);
-		...
-	} else {
 		...
-	}
 	...
 }

@transform_nv50_wndw depends on update_struct@
identifier S;
@@

 void nv50_wndw_reset(...)
 {
 	...
-	if (WARN_ON(!(S = kzalloc_obj(*S))))
+	S = kzalloc_obj(*S);
+	if (WARN_ON(!S))
 		return;
 	...
 }

@transform_kzalloc depends on update_struct@
identifier match_struct_reset.reset_func;
identifier P, S;
statement ST;
statement list STL;
@@

 void reset_func(struct drm_plane *P)
 {
 	<...
 	S = kzalloc_obj(*S);
(
-	if (S)
-	{
-		STL
-	}
+	if (!S) return;
+
+	STL
|
-	if (S) ST
+	if (!S) return;
+
+	ST
)
	...>
 }

@transform_body depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier S, P;
expression PS;
@@
- void reset_func(struct drm_plane *P)
+ struct drm_plane_state *new_name(struct drm_plane *P)
{
	...
 	S = kzalloc_obj(*S);
	...
(
 	if (!S) {
		...
-		return;
+		return ERR_PTR(-ENOMEM);
 	}
|
 	if (WARN_ON(!S)) {
		...
-		return;
+		return ERR_PTR(-ENOMEM);
 	}
|
 	if (S == NULL) {
 		...
-		return;
+		return ERR_PTR(-ENOMEM);
 	}
)
	...
(
-	__drm_atomic_helper_plane_reset(P, PS);
+	__drm_atomic_helper_plane_state_init(PS, P);
|
-	__drm_gem_reset_shadow_plane(P, PS);
+	__drm_gem_shadow_plane_state_init(P, PS);
)
	...
}

@update_early_return depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
 struct drm_plane_state *new_name(struct drm_plane *P)
{
	<+...
-	return;
+	return ERR_PTR(-EINVAL);
	...+>
}

@update_return_plane depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
 struct drm_plane_state *new_name(struct drm_plane *P)
{
	...
 	__drm_atomic_helper_plane_state_init(PS, P);
	...
+
+	return PS;
}

@update_return_shadow depends on update_struct@
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
 struct drm_plane_state *new_name(struct drm_plane *P)
{
	...
 	__drm_gem_shadow_plane_state_init(P, PS);
	...
+
+	return &PS->base;
}

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: airlied@redhat.com
Cc: kraxel@redhat.com
Cc: spice-devel@lists.freedesktop.org
Cc: virtualization@lists.linux.dev
---
 drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a026bd35ef48..d0e47be7febd 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -927,11 +927,11 @@ static const struct drm_plane_helper_funcs qxl_cursor_helper_funcs = {
 
 static const struct drm_plane_funcs qxl_cursor_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
 	.destroy	= drm_plane_helper_destroy,
-	.reset		= drm_atomic_helper_plane_reset,
+	.atomic_create_state = drm_atomic_helper_plane_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
 static const uint32_t qxl_primary_plane_formats[] = {
@@ -949,11 +949,11 @@ static const struct drm_plane_helper_funcs primary_helper_funcs = {
 
 static const struct drm_plane_funcs qxl_primary_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
 	.destroy	= drm_plane_helper_destroy,
-	.reset		= drm_atomic_helper_plane_reset,
+	.atomic_create_state = drm_atomic_helper_plane_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
 static struct drm_plane *qxl_create_plane(struct qxl_device *qdev,

-- 
2.54.0


^ permalink raw reply related

* [PATCH 00/60] drm/plane: Convert all drivers to atomic_create_state and remove reset
From: Maxime Ripard @ 2026-07-09 11:50 UTC (permalink / raw)
  To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, Maxime Ripard, javierm, ikerpedrosam, admin,
	gargaditya08, hamohammed.sa, louis.chauvet, melissa.srw,
	alexander.deucher, amd-gfx, christian.koenig, liviu.dudau,
	alison.wang, stefan, jstultz, sumit.semwal, xinliang.liu,
	yongqin.liu, Frank.Li, Sascha Hauer, festevam, imx, kernel,
	victor.liu, l.stach, laurentiu.palcu, linux-mips, paul,
	anitha.chrisanthus, paulk, chenhuacai, jeffbai, lvjianmin,
	wuqianhai, xry111, zhengxingda, jbrunet, khilman, linux-amlogic,
	martin.blumenstingl, abhinav.kumar, freedreno, jesszhan0024,
	linux-arm-msm, lumag, marijn.suijten, robin.clark, sean, marex,
	airlied, kraxel, spice-devel, virtualization, andy.yan, heiko,
	hjc, linux-rockchip, Baolin Wang, orsonzhai, zhang.lyra,
	alain.volmat, rgallaispou, alexandre.torgue, mcoquelin.stm32,
	philippe.cornu, raphael.gallais-pou, yannick.fertre, linux-sunxi,
	samuel, wens, jyri.sarha, tomi.valkeinen, hansg, dmitry.osipenko,
	gurchetansingh, olvaffe, michal.simek, harry.wentland, siqueira,
	sunpeng.li, linux, alexandre.belloni, claudiu.beznea, dharma.b,
	manikandan.m, nicolas.ferre, alim.akhtar, inki.dae, krzk,
	kyungmin.park, linux-samsung-soc, sw0312.kim, p.zabel,
	angelogioacchino.delregno, chunkuang.hu, matthias.bgg, dakr,
	lyude, nouveau, geert+renesas, kieran.bingham+renesas,
	laurent.pinchart+renesas, linux-renesas-soc, magnus.damm,
	tomi.valkeinen+renesas, biju.das.jz, dave.stevenson, kernel-list,
	mcanal, bcm-kernel-feedback-list, zack.rusin, jonathanh,
	linux-tegra, mperttunen, thierry.reding

This is a follow-up to the bridge reset removal series, and part of a
larger effort to remove the reset hook from all KMS objects.

The plane reset hook is overloaded: it is called both at probe time
to create the initial software state and during suspend/resume to
reset hardware and software state. These two roles have different
requirements, and the reset hook is not fallible, making error
handling difficult for the initial state allocation path.

While reset has the semantics to reset both the software and hardware
state, the vast majority of implementations and all the helpers only
reset the software state, making them equivalent to
atomic_create_state in practice. The atomic_create_state hook makes
this explicit: it only allocates and initializes a pristine state
without any side effect, and returns the state pointer or an ERR_PTR
on failure.

This series first adds the necessary infrastructure in the simple-kms
and GEM atomic helpers, then converts all 51 plane drivers tree-wide
from the reset hook to atomic_create_state. The conversions were done
using a combination of Coccinelle semantic patches and manual
adjustments. Once all drivers are converted, the old helpers and the
reset hook itself are removed from struct drm_plane_funcs.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Maxime Ripard (60):
      drm/simple-kms: Add create_plane_state hook
      drm/gem-atomic-helper: Create drm_gem_create_shadow_plane_state()
      drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state
      drm/gem-atomic-helper: Switch DRM_GEM_SHADOW_PLANE_FUNCS to atomic_create_state
      drm/gem-atomic-helper: Remove drm_gem_reset_shadow_plane()
      drm/sysfb: Convert to atomic_create_state
      drm/simple-kms: Switch to atomic_create_state
      drm/ssd130x: Convert to atomic_create_state
      drm/st7920: Convert to atomic_create_state
      drm/appletbdrm: Convert to atomic_create_state
      drm/vkms: Convert to atomic_create_state
      drm/gem-atomic-helper: Remove __drm_gem_reset_shadow_plane()
      drm/amdgpu: Convert to atomic_create_state
      drm/hdlcd: Convert to atomic_create_state
      drm/fsl-dcu: Convert to atomic_create_state
      drm/hisilicon/kirin: Convert to atomic_create_state
      drm/imx/dc: Convert to atomic_create_state
      drm/imx/dcss: Convert to atomic_create_state
      drm/ingenic: Convert to atomic_create_state
      drm/kmb: Convert to atomic_create_state
      drm/logicvc: Convert to atomic_create_state
      drm/loongson: Convert to atomic_create_state
      drm/meson: Convert to atomic_create_state
      drm/msm/mdp4: Convert to atomic_create_state
      drm/lcdif: Convert to atomic_create_state
      drm/mxsfb: Convert to atomic_create_state
      drm/qxl: Convert to atomic_create_state
      drm/rockchip: Convert to atomic_create_state
      drm/sprd: Convert to atomic_create_state
      drm/sti: Convert to atomic_create_state
      drm/stm: Convert to atomic_create_state
      drm/sun4i: sun8i: Convert to atomic_create_state
      drm/tests: kunit: Convert to atomic_create_state
      drm/tilcdc: Convert to atomic_create_state
      drm/vboxvideo: Convert to atomic_create_state
      drm/virtio: Convert to atomic_create_state
      drm/xlnx: Convert to atomic_create_state
      drm/atomic-state-helper: Remove drm_atomic_helper_plane_reset()
      drm/amdgpu_dm: Convert to atomic_create_state
      drm/komeda: Convert to atomic_create_state
      drm/malidp: Convert to atomic_create_state
      drm/armada: Convert to atomic_create_state
      drm/atmel-hlcdc: Convert to atomic_create_state
      drm/exynos: Convert to atomic_create_state
      drm/imx/ipuv3: Convert to atomic_create_state
      drm/mediatek: Convert to atomic_create_state
      drm/msm/dpu1: Convert to atomic_create_state
      drm/msm/mdp5: Convert to atomic_create_state
      drm/nouveau: Convert to atomic_create_state
      drm/omap: Convert to atomic_create_state
      drm/rcar-du: Convert to atomic_create_state
      drm/rz-du: Convert to atomic_create_state
      drm/shmobile: Convert to atomic_create_state
      drm/sun4i: layer: Convert to atomic_create_state
      drm/vc4: Convert to atomic_create_state
      drm/verisilicon: Convert to atomic_create_state
      drm/vmwgfx: Convert to atomic_create_state
      drm/atomic-state-helper: Remove __drm_atomic_helper_plane_reset()
      drm/tegra: Convert to atomic_create_state
      drm/plane: Remove reset

 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c           |   2 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c    |  17 ++--
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c  |  18 ++--
 drivers/gpu/drm/arm/hdlcd_crtc.c                   |   2 +-
 drivers/gpu/drm/arm/malidp_planes.c                |  12 ++-
 drivers/gpu/drm/armada/armada_overlay.c            |  39 ++++----
 drivers/gpu/drm/armada/armada_plane.c              |  15 ++--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c    |  27 +++---
 drivers/gpu/drm/drm_atomic_state_helper.c          |  41 ---------
 drivers/gpu/drm/drm_gem_atomic_helper.c            | 100 +++++++++++----------
 drivers/gpu/drm/drm_mode_config.c                  |   4 +-
 drivers/gpu/drm/drm_simple_kms_helper.c            |  11 +--
 drivers/gpu/drm/exynos/exynos_drm_plane.c          |  22 ++---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c        |   2 +-
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c    |   2 +-
 drivers/gpu/drm/imx/dc/dc-plane.c                  |   2 +-
 drivers/gpu/drm/imx/dcss/dcss-plane.c              |   2 +-
 drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c            |  19 ++--
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c          |   2 +-
 drivers/gpu/drm/ingenic/ingenic-ipu.c              |   2 +-
 drivers/gpu/drm/kmb/kmb_plane.c                    |   2 +-
 drivers/gpu/drm/logicvc/logicvc_layer.c            |   2 +-
 drivers/gpu/drm/loongson/lsdc_plane.c              |   2 +-
 drivers/gpu/drm/mediatek/mtk_plane.c               |  22 ++---
 drivers/gpu/drm/meson/meson_overlay.c              |   2 +-
 drivers/gpu/drm/meson/meson_plane.c                |   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  18 ++--
 drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c         |   2 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c         |  16 ++--
 drivers/gpu/drm/mxsfb/lcdif_kms.c                  |   2 +-
 drivers/gpu/drm/mxsfb/mxsfb_kms.c                  |   2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndw.c            |  15 ++--
 drivers/gpu/drm/omapdrm/omap_plane.c               |  13 ++-
 drivers/gpu/drm/qxl/qxl_display.c                  |   4 +-
 drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c    |  15 ++--
 drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c      |  15 ++--
 drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c       |  15 ++--
 drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c |  15 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |   2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c       |   2 +-
 drivers/gpu/drm/sitronix/st7920.c                  |  12 +--
 drivers/gpu/drm/solomon/ssd130x.c                  |  12 +--
 drivers/gpu/drm/sprd/sprd_dpu.c                    |   2 +-
 drivers/gpu/drm/sti/sti_cursor.c                   |   2 +-
 drivers/gpu/drm/sti/sti_gdp.c                      |   2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c                    |   2 +-
 drivers/gpu/drm/stm/ltdc.c                         |   2 +-
 drivers/gpu/drm/sun4i/sun4i_layer.c                |  12 ++-
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c             |   2 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c             |   2 +-
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h           |   4 +-
 drivers/gpu/drm/sysfb/drm_sysfb_modeset.c          |  30 +++++--
 drivers/gpu/drm/tegra/plane.c                      |  28 +++---
 drivers/gpu/drm/tests/drm_kunit_helpers.c          |   2 +-
 drivers/gpu/drm/tilcdc/tilcdc_plane.c              |   2 +-
 drivers/gpu/drm/tiny/appletbdrm.c                  |  12 +--
 drivers/gpu/drm/vboxvideo/vbox_mode.c              |   2 +-
 drivers/gpu/drm/vc4/vc4_plane.c                    |  15 ++--
 drivers/gpu/drm/verisilicon/vs_cursor_plane.c      |   2 +-
 drivers/gpu/drm/verisilicon/vs_plane.c             |  14 ++-
 drivers/gpu/drm/verisilicon/vs_plane.h             |   2 +-
 drivers/gpu/drm/verisilicon/vs_primary_plane.c     |   2 +-
 drivers/gpu/drm/virtio/virtgpu_plane.c             |   2 +-
 drivers/gpu/drm/vkms/vkms_plane.c                  |  15 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c                |  17 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h                |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c                |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c               |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c               |   4 +-
 drivers/gpu/drm/xlnx/zynqmp_kms.c                  |   2 +-
 include/drm/drm_atomic_state_helper.h              |   3 -
 include/drm/drm_gem_atomic_helper.h                |  13 +--
 include/drm/drm_plane.h                            |  12 ---
 include/drm/drm_simple_kms_helper.h                |   1 +
 74 files changed, 331 insertions(+), 422 deletions(-)
---
base-commit: 671b7825dbfe9ea6e3ad3001003aeee0df48d1b5
change-id: 20260629-drm-no-more-plane-reset-04950f42e07f

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>


^ permalink raw reply

* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Joseph Qi @ 2026-07-09 11:01 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: Christoph Hellwig, virtualization, linux-kernel, Baokun Li
In-Reply-To: <CAM9Jb+hPmV4KF5xAy0=CPy6yh4=9=aACmLbZ3G5daz7QuAH3hQ@mail.gmail.com>



On 7/9/26 6:58 PM, Pankaj Gupta wrote:
>> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
>>> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>>>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>>>> index 4176046627beb..081370aac6317 100644
>>>> --- a/drivers/nvdimm/nd_virtio.c
>>>> +++ b/drivers/nvdimm/nd_virtio.c
>>>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>>>>      if (bio && bio->bi_iter.bi_sector != -1) {
>>>>              struct bio *child = bio_alloc(bio->bi_bdev, 0,
>>>>                                            REQ_OP_WRITE | REQ_PREFLUSH,
>>>> -                                          GFP_ATOMIC);
>>>> +                                          GFP_NOIO);
>>>>
>>>>              if (!child)
>>>>                      return -ENOMEM;
>>>
>>> This NULL  check can go away now, and probaby should to avoid confusion.
>>>
>>> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
>>> is from that, we can still deadlock.  We'll need a separate bio_set
>>> for this to be deadlock free.  disk->bio_split isn't otherwise
>>> used for the drivers/nvdimm/ driverss, so you might be able to
>>> repurpose that if you want to be creative.
>>
>> Thanks for pointing this out.
>>
>> Seems it is more proper to use a driver-private bio_set instead of
>> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>>
>> Pankaj, what's your opinion?
> 
> Hi Joseph,
> 
> Thank you for the patch!
> 
> I am not very familiar with the internals of fs_bio_set, so I do not
> have a strong opinion on the implementation details. However, a
> driver-private bio_set sounds reasonable to me, and I would go with
> Christoph's suggestion.
> 
Fine, I'll send out v2 later to address this.

Thanks,
Joseph


^ permalink raw reply

* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Pankaj Gupta @ 2026-07-09 10:58 UTC (permalink / raw)
  To: Joseph Qi; +Cc: Christoph Hellwig, virtualization, linux-kernel, Baokun Li
In-Reply-To: <55f7befb-5ef3-4ede-a3ed-4968864c45b7@linux.alibaba.com>

> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> > On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
> >> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> >> index 4176046627beb..081370aac6317 100644
> >> --- a/drivers/nvdimm/nd_virtio.c
> >> +++ b/drivers/nvdimm/nd_virtio.c
> >> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
> >>      if (bio && bio->bi_iter.bi_sector != -1) {
> >>              struct bio *child = bio_alloc(bio->bi_bdev, 0,
> >>                                            REQ_OP_WRITE | REQ_PREFLUSH,
> >> -                                          GFP_ATOMIC);
> >> +                                          GFP_NOIO);
> >>
> >>              if (!child)
> >>                      return -ENOMEM;
> >
> > This NULL  check can go away now, and probaby should to avoid confusion.
> >
> > Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> > is from that, we can still deadlock.  We'll need a separate bio_set
> > for this to be deadlock free.  disk->bio_split isn't otherwise
> > used for the drivers/nvdimm/ driverss, so you might be able to
> > repurpose that if you want to be creative.
>
> Thanks for pointing this out.
>
> Seems it is more proper to use a driver-private bio_set instead of
> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>
> Pankaj, what's your opinion?

Hi Joseph,

Thank you for the patch!

I am not very familiar with the internals of fs_bio_set, so I do not
have a strong opinion on the implementation details. However, a
driver-private bio_set sounds reasonable to me, and I would go with
Christoph's suggestion.

Best regards,
Pankaj
>
> Thanks,
> Joseph

^ permalink raw reply

* Re: [PATCH] iommu/virtio: reject short event buffers
From: Jean-Philippe Brucker @ 2026-07-09  9:55 UTC (permalink / raw)
  To: raoxu; +Cc: joro, will, robin.murphy, virtualization, iommu, linux-kernel
In-Reply-To: <6800BE0812AF0897+20260706083148.788991-1-raoxu@uniontech.com>

On Mon, Jul 06, 2026 at 04:31:48PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> The event queue uses fixed-size buffers for struct viommu_event. The
> device-reported used length is currently only checked for oversized
> buffers, so a short used buffer can still be passed to the fault
> handler.

Thank you for the patch. The buffer is allowed to be smaller than struct
virtio_iommu_fault, because valid fields depend on flags:

	struct virtio_iommu_fault {
		u8 reason;
		u8 reserved[3];
		le32 flags;
		le32 endpoint;
		le32 reserved1;
		le64 address;
	};
	
	#define VIRTIO_IOMMU_FAULT_F_READ	(1 << 0)
	#define VIRTIO_IOMMU_FAULT_F_WRITE      (1 << 1)
	#define VIRTIO_IOMMU_FAULT_F_ADDRESS    (1 << 8)

If F_ADDRESS isn't set, the address field is invalid. The spec (Virtio v1.3)
says in 5.13.6.9.2 "Device Requirements: Fault reporting"

  "The device MAY omit setting VIRTIO_IOMMU_FAULT_F_ADDRESS and writing
   address in any fault report, regardless of the reason."

I think the current check aims to catch newer implementations that would
use an extended virtio_iommu_fault to report recoverable faults, but I'm
not sure anymore.

Overall the driver assumes that the IOMMU device is well behaved. If we
want to add sanity checks, maybe viommu_fault_handler() should check len
depending on flags. But I suspect more work is needed if we change the
security model to not trust the device.

Thanks,
Jean

> 
> In that case the handler parses fields that were not written by the
> device for this event, potentially using stale data from a previous
> event and reporting a bogus fault.
> 
> Reject any event buffer whose used length is not exactly the expected
> event size. Still recycle the buffer back to the event queue so a bad
> event does not permanently shrink the queue.
> 
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
>  drivers/iommu/virtio-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 587fc13197f1..3ace4a6dd02a 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -635,7 +635,7 @@ static void viommu_event_handler(struct virtqueue *vq)
>  	struct viommu_dev *viommu = vq->vdev->priv;
> 
>  	while ((evt = virtqueue_get_buf(vq, &len)) != NULL) {
> -		if (len > sizeof(*evt)) {
> +		if (len != sizeof(*evt)) {
>  			dev_err(viommu->dev,
>  				"invalid event buffer (len %u != %zu)\n",
>  				len, sizeof(*evt));
> --
> 2.50.1
> 

^ permalink raw reply

* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Stefano Garzarella @ 2026-07-09  9:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
	Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang
In-Reply-To: <20260708065853-mutt-send-email-mst@kernel.org>

On Wed, Jul 08, 2026 at 06:59:41AM -0400, Michael S. Tsirkin wrote:
>On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> Add a test that sends 2 MB of data using randomly sized small packets
>> (129-512 bytes) over a SOCK_STREAM connection. Packets above
>> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
>> forcing each one into its own skb.
>>
>> Without receive queue collapsing, the per-skb overhead eventually
>> exceeds buf_alloc and the connection is reset. The test verifies
>> that all data arrives and that content integrity is preserved.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to
>trigger?

Currently, with the default value, the trigger is practically immediate 
for packets between 129 and 512 bytes, but yes, a smaller buffer size 
certainly makes this effect even more pronounced.

>
>anyway
>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks!

If I need to post a v3, I'll add it; otherwise, I guess we can leave it 
as is or send a follow-up for net-next.

Thanks,
Stefano


^ permalink raw reply

* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Stefano Garzarella @ 2026-07-09  8:54 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
	Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable,
	Brien Oberstein
In-Reply-To: <20260708065947-mutt-send-email-mst@kernel.org>

On Wed, Jul 08, 2026 at 07:00:00AM -0400, Michael S. Tsirkin wrote:
>On Wed, Jul 08, 2026 at 12:29:03PM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> When many small packets accumulate in the receive queue, the skb overhead
>> can exceed buf_alloc even while the payload is within bounds. This causes
>> virtio_transport_inc_rx_pkt() to reject packets, leading to connection
>> resets during large transfers under backpressure.
>>
>> The issue was reported by Brien, who has a reproducer, but it is also
>> easily reproducible with iperf-vsock [1] using a small packet size:
>>
>>   iperf3 --vsock -c $CID -l 129
>>
>> which fails immediately without this patch but with commit 059b7dbd20a6
>> ("vsock/virtio: fix potential unbounded skb queue").
>>
>> Inspired by TCP's tcp_collapse() which solves a similar problem, add
>> virtio_transport_collapse_rx_queue() that walks the receive queue and
>> re-copies data into compact linear skbs to reduce the overhead.
>>
>> The collapse is triggered proactively from when the number of skb queued
>> is close to exceeding the overhead budget.
>>
>> A pre-scan counts the eligible bytes to size each allocation precisely,
>> avoiding waste for isolated small packets. Partially consumed skbs are
>> kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to
>> maintain SEQPACKET message boundaries, and skbs already larger than the
>> collapse target because they already have a good data-to-overhead ratio.
>>
>> Walking a large queue may take a significant amount of time and cache
>> misses, causing traffic burstiness. To limit this, the collapse stops
>> once enough room is freed for this packet and the next one, but may
>> opportunistically free more to fill each collapsed skb to capacity.
>>
>> [1] https://github.com/stefano-garzarella/iperf-vsock
>>
>> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>> Cc: stable@vger.kernel.org
>> Reported-by: Brien Oberstein <brienpub@gmail.com>
>> Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/
>> Tested-by: Brien Oberstein <brienpub@gmail.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>
>this is the right approach

Yeah, I have a follow up to start to use skb->truesize, etc. but I guess 
more net-next material.

>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks,
Stefano


^ permalink raw reply

* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Joseph Qi @ 2026-07-09  3:13 UTC (permalink / raw)
  To: Christoph Hellwig, Pankaj Gupta; +Cc: virtualization, linux-kernel, Baokun Li
In-Reply-To: <20260708162435.GB2502@lst.de>



On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>> index 4176046627beb..081370aac6317 100644
>> --- a/drivers/nvdimm/nd_virtio.c
>> +++ b/drivers/nvdimm/nd_virtio.c
>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>>  	if (bio && bio->bi_iter.bi_sector != -1) {
>>  		struct bio *child = bio_alloc(bio->bi_bdev, 0,
>>  					      REQ_OP_WRITE | REQ_PREFLUSH,
>> -					      GFP_ATOMIC);
>> +					      GFP_NOIO);
>>  
>>  		if (!child)
>>  			return -ENOMEM;
> 
> This NULL  check can go away now, and probaby should to avoid confusion.
> 
> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> is from that, we can still deadlock.  We'll need a separate bio_set
> for this to be deadlock free.  disk->bio_split isn't otherwise
> used for the drivers/nvdimm/ driverss, so you might be able to
> repurpose that if you want to be creative.

Thanks for pointing this out.

Seems it is more proper to use a driver-private bio_set instead of
repurposing bd_disk->bio_split, like raid5-ppl flush_bs.

Pankaj, what's your opinion?

Thanks,
Joseph

^ permalink raw reply

* Re: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
From: Zi Yan @ 2026-07-09  2:28 UTC (permalink / raw)
  To: Lorenzo Stoakes, Lance Yang
  Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
	tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
	kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
	rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
	tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
	jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
	thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
	brauner, muchun.song, osalvador, david, baolin.wang, liam, npache,
	ryan.roberts, dev.jain, baohua, hughd, vbabka, rppt, surenb,
	mhocko, jannh, pfalcato, kees, perex, tiwai, linux-mips,
	linux-kernel, linuxppc-dev, dri-devel, etnaviv, linux-arm-kernel,
	linux-samsung-soc, intel-gfx, linux-arm-msm, freedreno, nouveau,
	linux-rockchip, linux-tegra, virtualization, intel-xe, xen-devel,
	linux-fbdev, linux-aio, linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <akaJx8Zt8kazlrjq@lucifer>

On Thu Jul 2, 2026 at 12:07 PM EDT, Lorenzo Stoakes wrote:
> On Thu, Jul 02, 2026 at 09:49:47PM +0800, Lance Yang wrote:
>>
>> On Mon, Jun 29, 2026 at 08:25:36PM +0100, Lorenzo Stoakes wrote:
>> >Replace use of the legacy vm_flags_t flags with vma_flags_t values
>> >throughout the mremap logic.
>> >
>> >Additionally update comments to reflect the changes to be consistent.
>> >
>> >No functional change intended.
>> >
>> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> >---
>>
>> The vm_flags_set() cases below spell out vma_start_write(), but the
>> vm_flags_clear() cases don't?
>
> Yep as I said elsewhere, implicitly taking the lock is terrible and me doing
> this is completely on purpose to get rid of that :)
>
> But I haven't been clear enough clearly, so I should put the argument as to why
> that's ok in the commit message.
>
> Will do so on respin.

How about also add a comment to vma_clear*() telling us a lock is not
needed and why like you explained a lock is needed for vma_set*()?
This asymmetry could confuse people. 

This patch looks good to me.

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi


^ permalink raw reply

* Re: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
From: Zi Yan @ 2026-07-09  2:16 UTC (permalink / raw)
  To: Lorenzo Stoakes, Lance Yang
  Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
	tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
	kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
	rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
	tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
	jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
	thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
	brauner, muchun.song, osalvador, david, baolin.wang, liam, npache,
	ryan.roberts, dev.jain, baohua, hughd, vbabka, rppt, surenb,
	mhocko, jannh, pfalcato, kees, perex, tiwai, linux-mips,
	linux-kernel, linuxppc-dev, dri-devel, etnaviv, linux-arm-kernel,
	linux-samsung-soc, intel-gfx, linux-arm-msm, freedreno, nouveau,
	linux-rockchip, linux-tegra, virtualization, intel-xe, xen-devel,
	linux-fbdev, linux-aio, linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <akaIfhfbTCAdJm3H@lucifer>

On Thu Jul 2, 2026 at 11:53 AM EDT, Lorenzo Stoakes wrote:
> On Thu, Jul 02, 2026 at 12:09:17AM +0800, Lance Yang wrote:
>>
>> On Mon, Jun 29, 2026 at 08:25:35PM +0100, Lorenzo Stoakes wrote:
>> >Replace use of the legacy vm_flags_t flags with vma_flags_t values
>> >throughout the mprotect logic.
>> >
>> >Note that we retain the legacy vm_flags_t bit shifting code in
>> >do_mprotect_key(), deferring a vma_flags_t approach to this for the time
>> >being.
>> >
>> >Additionally update comments to reflect the changes to be consistent.
>> >
>> >No functional change intended.
>> >
>> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> >---
>> > mm/mprotect.c | 16 ++++++++--------
>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>> >
>> >diff --git a/mm/mprotect.c b/mm/mprotect.c
>> >index 9cbf932b028c..c9504b2a2525 100644
>> >--- a/mm/mprotect.c
>> >+++ b/mm/mprotect.c
>> >@@ -40,7 +40,7 @@
>> >
>> > static bool maybe_change_pte_writable(struct vm_area_struct *vma, pte_t pte)
>> > {
>> >-	if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE)))
>> >+	if (WARN_ON_ONCE(!vma_test(vma, VMA_WRITE_BIT)))
>> > 		return false;
>> >
>> > 	/* Don't touch entries that are not even readable. */
>> >@@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,
>> > bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
>> > 			     pte_t pte)
>> > {
>> >-	if (!(vma->vm_flags & VM_SHARED))
>> >+	if (!vma_test(vma, VMA_SHARED_BIT))
>> > 		return can_change_private_pte_writable(vma, addr, pte);
>> >
>> > 	return can_change_shared_pte_writable(vma, pte);
>> >@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru
>> > {
>> > 	bool set_write;
>> >
>> >-	if (vma->vm_flags & VM_SHARED) {
>> >+	if (vma_test(vma, VMA_SHARED_BIT)) {
>> > 		set_write = can_change_shared_pte_writable(vma, ptent);
>> > 		prot_commit_flush_ptes(vma, addr, ptep, oldpte, ptent, nr_ptes,
>> > 				       /* idx = */ 0, set_write, tlb);
>> >@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
>> > 		vm_unacct_memory(nrpages);
>> >
>> > 	/*
>> >-	 * Private VM_LOCKED VMA becoming writable: trigger COW to avoid major
>> >-	 * fault on access.
>> >+	 * Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid
>> >+	 * major fault on access.
>> > 	 */
>> > 	if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) &&
>> > 	    vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&
>> >@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
>> > 			goto out;
>> > 		start = vma->vm_start;
>> > 		error = -EINVAL;
>> >-		if (!(vma->vm_flags & VM_GROWSDOWN))
>> >+		if (!vma_test(vma, VMA_GROWSDOWN_BIT))
>> > 			goto out;
>> > 	} else {
>> > 		if (vma->vm_start > start)
>> >@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
>> > 		if (unlikely(grows & PROT_GROWSUP)) {
>> > 			end = vma->vm_end;
>> > 			error = -EINVAL;
>> >-			if (!(vma->vm_flags & VM_GROWSUP))
>> >+			if (!vma_test(vma, VMA_GROWSUP_BIT))
>>
>> IIUC, should this be
>>
>> if (!vma_test_single_mask(vma, VMA_GROWSUP))
>>
>> instead?
>>
>> #elif defined(CONFIG_PARISC)
>> #define VM_GROWSUP	INIT_VM_FLAG(GROWSUP)
>> ...
>> #ifndef VM_GROWSUP
>> #define VM_GROWSUP	VM_NONE
>> ...
>>
>> VM_GROWSUP is only defined as GROWSUP on parisc and becomes VM_NONE
>> elsewhere. But VMA_GROWSUP_BIT is the raw ARCH_1 bit, which is also used
>> for other arch-specific VMA flags:
>>
>> 	DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1),		/* Strong Access Ordering (powerpc) */
>> 	DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1),		/* parisc */
>> 	DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1),	/* sparc64 */
>> 	DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1),	/* arm64 */
>> 	DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1),	/* sparc64, arm64 */
>> 	DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1),	/* !CONFIG_MMU */
>>
>> Other vma_test() changes look fine to me: just fixed INIT_VM_FLAG()
>> masks matching their VMA_*_BIT :)
>
> Thanks you're right, will fix!
>
> Again I swear I ran claude on all of this so it's failing me here :)
>

Is it better to add something like below to avoid misuse these mutually
exclusive bit aliases?

An example for VMA_GROWSUP_BIT:

#if defined(CONFIG_PARISC)
DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1),		/* parisc */
#else
/* make VMA_GROWSUP_BIT a build bug on */
#endif

Hmm, these VMA_*_BIT are enum items, so the above might not be possible.
An alternative is to only define them for the corresponding config and
you will get build errors when trying to use them directly and the
config is not enabled. Otherwise, misuses like "vma_test(vma,
VMA_GROWSUP_BIT)" is harder to uncover.


-- 
Best Regards,
Yan, Zi


^ permalink raw reply

* Re: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
From: Zi Yan @ 2026-07-09  2:01 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Thomas Bogendoerfer, Madhavan Srinivasan, Michael Ellerman,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Lucas Stach, Inki Dae, Seung-Woo Kim,
	Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Rob Clark,
	Dmitry Baryshkov, Lyude Paul, Danilo Krummrich, Tomi Valkeinen,
	Sandy Huang, Heiko Stübner, Andy Yan, Thierry Reding,
	Mikko Perttunen, Jonathan Hunter, Gerd Hoffmann, Dmitry Osipenko,
	Zack Rusin, Matthew Brost, Thomas Hellstrom,
	Oleksandr Andrushchenko, Helge Deller, Benjamin LaHaise,
	Alexander Viro, Christian Brauner, Muchun Song, Oscar Salvador,
	David Hildenbrand, Baolin Wang, Liam R . Howlett, Nico Pache,
	Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Hugh Dickins,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Jann Horn, Pedro Falcato, Kees Cook, Jaroslav Kysela,
	Takashi Iwai, linux-mips, linux-kernel, linuxppc-dev, dri-devel,
	etnaviv, linux-arm-kernel, linux-samsung-soc, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-rockchip, linux-tegra,
	virtualization, intel-xe, xen-devel, linux-fbdev, linux-aio,
	linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <2db16db81538355ca65f778c246d2381c673cad4.1782760670.git.ljs@kernel.org>

On Mon Jun 29, 2026 at 3:25 PM EDT, Lorenzo Stoakes wrote:
> Replace use of the legacy vm_flags_t flags with vma_flags_t values
> throughout the mlock logic.
>
> Additionally update comments to reflect the changes to be consistent.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
>  mm/mlock.c | 86 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 46 insertions(+), 40 deletions(-)
>

LGTM. What confused me when I was reading the code is VMA_LOCKED_MASK
and VMA_LOCKED_BIT, since both share the same prefix VMA_LOCKED. Before
looking at the definition of VMA_LOCKED_MASK, I was wondering when to
use _BIT or _MASK. But it is just a naming thing.

Anyway,

Reviewed-by: Zi Yan <ziy@nvidia.com>


-- 
Best Regards,
Yan, Zi


^ permalink raw reply

* Re: [PATCH 10/13] mm/vma: convert miscellaneous uses of VMA flags in core mm
From: Zi Yan @ 2026-07-09  1:52 UTC (permalink / raw)
  To: Lorenzo Stoakes, Lance Yang
  Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
	tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
	kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
	rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
	tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
	jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
	thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
	brauner, muchun.song, osalvador, david, baolin.wang, liam, npache,
	ryan.roberts, dev.jain, baohua, hughd, vbabka, rppt, surenb,
	mhocko, jannh, pfalcato, kees, perex, tiwai, linux-mips,
	linux-kernel, linuxppc-dev, dri-devel, etnaviv, linux-arm-kernel,
	linux-samsung-soc, intel-gfx, linux-arm-msm, freedreno, nouveau,
	linux-rockchip, linux-tegra, virtualization, intel-xe, xen-devel,
	linux-fbdev, linux-aio, linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <akaGxiTaJreEQn8T@lucifer>

On Thu Jul 2, 2026 at 11:46 AM EDT, Lorenzo Stoakes wrote:
> On Thu, Jul 02, 2026 at 09:12:33PM +0800, Lance Yang wrote:
>>
>> On Mon, Jun 29, 2026 at 08:25:33PM +0100, Lorenzo Stoakes wrote:
>> >Update various uses of legacy flags in vma.c and mmap.c to the new
>> >vma_flags_t type, updating comments alongside them to be consistent.
>> >
>> >Also update __install_special_mapping() to rearrange things slightly to
>> >accommodate the changes.
>> >
>> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> >---
>> [...]
>> >diff --git a/mm/vma.c b/mm/vma.c
>> >index b81c05e67a61..ab2ef0f04420 100644
>> >--- a/mm/vma.c
>> >+++ b/mm/vma.c
>> >@@ -3417,23 +3417,27 @@ struct vm_area_struct *__install_special_mapping(
>> > 	vm_flags_t vm_flags, void *priv,
>> > 	const struct vm_operations_struct *ops)
>> > {
>> >-	int ret;
>> >+	vma_flags_t vma_flags = legacy_to_vma_flags(vm_flags);
>> > 	struct vm_area_struct *vma;
>> >+	int ret;
>> >
>> > 	vma = vm_area_alloc(mm);
>> >-	if (unlikely(vma == NULL))
>> >+	if (unlikely(!vma))
>> > 		return ERR_PTR(-ENOMEM);
>> >
>> >-	vma_set_range(vma, addr, addr + len, 0);
>> >-	vm_flags |= vma_flags_to_legacy(mm->def_vma_flags) | VM_DONTEXPAND;
>> >+	vma_flags_set_mask(&vma_flags, mm->def_vma_flags);
>> >+	vma_flags_set(&vma_flags, VMA_DONTEXPAND_BIT);
>> > 	if (pgtable_supports_soft_dirty())
>> >-		vm_flags |= VM_SOFTDIRTY;
>> >-	vm_flags_init(vma, vm_flags & ~VM_LOCKED_MASK);
>> >+		vma_flags_set(&vma_flags, VMA_SOFTDIRTY_BIT);
>> >+	vma_flags_clear_mask(&vma_flags, VMA_LOCKED_MASK);
>> >+	vma->flags = vma_flags;
>>
>> Maybe worth a vma_flags_init() helper here to mirror vm_flags_init()?
>> With this open-coded, we lose the soft-dirty WARN_ON_ONCE sanity check.
>>
>> Might be nicer to keep that check in one place ;)
>
> I really hate all the VMA flag accessors, they conflate things horribly - we
> should be explicitly taking VMA write locks when we need to (and often killable
> ones actually) not assuming that a VMA flags accessor does (they should at most
> assert).
>
> This case is even more terribly egregious - you are setting flags at an
> arbitrary time, why are we asserting something about softdirty?
>
> You may update them as part of initialisation, maybe not. It's far from a
> guarantee and feels like a lazy place to put it.
>
> BUT obviously it's an oversight not to open code that here, so I'll update the
> patch to do that!

What do you want to open code here? softdirty WARN_ON_ONCE()?

vma_flags gets VMA_SOFTDIRTY_BIT just above vma->flags, why do we need a
check after that?

BTW, if you think the check is needed, patch 9 will need to be updated,
since the same pattern appears in create_init_stack_vma().

>
> I want VMA flags to be a clean stateless thing, other than the flags
> themselves. Implicit, unrelated, asserts or lock acquisitions in general should
> be done separately IMO.
>

Anyway,

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi


^ permalink raw reply

* Re: [PATCH 09/13] mm/vma: update create_init_stack_vma() to use vma_flags_t
From: Zi Yan @ 2026-07-09  1:42 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Thomas Bogendoerfer, Madhavan Srinivasan, Michael Ellerman,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Lucas Stach, Inki Dae, Seung-Woo Kim,
	Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Rob Clark,
	Dmitry Baryshkov, Lyude Paul, Danilo Krummrich, Tomi Valkeinen,
	Sandy Huang, Heiko Stübner, Andy Yan, Thierry Reding,
	Mikko Perttunen, Jonathan Hunter, Gerd Hoffmann, Dmitry Osipenko,
	Zack Rusin, Matthew Brost, Thomas Hellstrom,
	Oleksandr Andrushchenko, Helge Deller, Benjamin LaHaise,
	Alexander Viro, Christian Brauner, Muchun Song, Oscar Salvador,
	David Hildenbrand, Baolin Wang, Liam R . Howlett, Nico Pache,
	Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Hugh Dickins,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Jann Horn, Pedro Falcato, Kees Cook, Jaroslav Kysela,
	Takashi Iwai, linux-mips, linux-kernel, linuxppc-dev, dri-devel,
	etnaviv, linux-arm-kernel, linux-samsung-soc, intel-gfx,
	linux-arm-msm, freedreno, nouveau, linux-rockchip, linux-tegra,
	virtualization, intel-xe, xen-devel, linux-fbdev, linux-aio,
	linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <34689784ee6856f100c02ad4eabeaa4db643713a.1782760670.git.ljs@kernel.org>

On Mon Jun 29, 2026 at 3:25 PM EDT, Lorenzo Stoakes wrote:
> Replace use of the legacy vm_flags_t flags with vma_flags_t values in
> create_init_stack_vma().
>
> As part of this change we add VMA_STACK_EARLY and VMA_STACK_INCOMPLETE
> vma_flags_t defines, and slightly rework create_init_stack_vma() for
> clarity.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
>  include/linux/mm.h |  4 ++++
>  mm/vma_exec.c      | 18 +++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
>

LGTM.

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi


^ permalink raw reply

* Re: [PATCH] scsi: virtio_scsi: fixup endian conversions for warning messages
From: Martin K. Petersen @ 2026-07-09  1:39 UTC (permalink / raw)
  To: Ben Dooks
  Cc: Michael S. Tsirkin, Jason Wang, Paolo Bonzini, Stefan Hajnoczi,
	Eugenio Pérez, James E.J. Bottomley, Martin K. Petersen,
	virtualization, linux-scsi, linux-kernel
In-Reply-To: <20260623132427.838900-1-ben.dooks@codethink.co.uk>


Ben,

> There are several places where printing functions are being passed
> parameters that have not been through endian conversion functions. Use
> the virtio32_to_cpu to fix the warnings.

Applied to 7.3/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply

* Re: [PATCH net] vhost-net: fix TX stall when vhost owns virtio-net header
From: Michael S. Tsirkin @ 2026-07-08 16:50 UTC (permalink / raw)
  To: enrico.zanda
  Cc: jasowangio, virtualization, netdev, kuba, kvm, linux-kernel,
	eperezma, nd
In-Reply-To: <20260708152242.2268848-1-enrico.zanda@arm.com>

On Wed, Jul 08, 2026 at 04:22:42PM +0100, enrico.zanda@arm.com wrote:
> From: Enrico Zanda <enrico.zanda@arm.com>
> 
> When vhost owns the virtio-net header, i.e. when
> VHOST_NET_F_VIRTIO_NET_HDR is negotiated, sock_hlen is 0,
> meaning that no header will be forwarded to the TAP device.
> 
> In the current vhost_net_build_xdp() implementation,
> when sock_hlen == 0, the gso pointer can point at the start of the
> Ethernet frame instead of a virtio-net header.
> This results in a wrong interpretation of the destination MAC address
> bytes as struct virtio_net_hdr fields.
> 
> This can, for some MAC addresses, trigger -EINVAL and return early
> before the TX descriptor is completed, which can stall vhost-net TX.
> 
> Before 97b2409f28e0, the gso pointer was set to the zeroed padding area,
> using it as a synthetic virtio-net header. Restore that behavior.
> 
> Fixes: 97b2409f28e0 ("vhost-net: reduce one userspace copy when building XDP buff")
> Signed-off-by: Enrico Zanda <enrico.zanda@arm.com>


The fix looks good:
Acked-by: Michael S. Tsirkin <mst@redhat.com>

Sashiko thinks there's something something security here, but I think
it is misguided. It's just guest hurting itself. driver breaks the
device it gets to keep both pieces.

> ---
>  drivers/vhost/net.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 77b59f49bddb..3e72b9c6af0c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -731,10 +731,12 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue *nvq,
>  		goto err;
>  	}
>  
> -	gso = buf + pad - sock_hlen;
> -
> -	if (!sock_hlen)
> +	if (!sock_hlen) {
>  		memset(buf, 0, pad);
> +		gso = buf;
> +	} else {
> +		gso = buf + pad - sock_hlen;
> +	}
>  
>  	if ((gso->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
>  	    vhost16_to_cpu(vq, gso->csum_start) +
> -- 
> 2.43.0


^ permalink raw reply

* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Christoph Hellwig @ 2026-07-08 16:24 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Pankaj Gupta, virtualization, linux-kernel, Christoph Hellwig,
	Baokun Li
In-Reply-To: <20260708124238.2817165-1-joseph.qi@linux.alibaba.com>

On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> index 4176046627beb..081370aac6317 100644
> --- a/drivers/nvdimm/nd_virtio.c
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>  	if (bio && bio->bi_iter.bi_sector != -1) {
>  		struct bio *child = bio_alloc(bio->bi_bdev, 0,
>  					      REQ_OP_WRITE | REQ_PREFLUSH,
> -					      GFP_ATOMIC);
> +					      GFP_NOIO);
>  
>  		if (!child)
>  			return -ENOMEM;

This NULL  check can go away now, and probaby should to avoid confusion.

Also bio_alloc allocates from fs_bio_set, so if the incoming bio
is from that, we can still deadlock.  We'll need a separate bio_set
for this to be deadlock free.  disk->bio_split isn't otherwise
used for the drivers/nvdimm/ driverss, so you might be able to
repurpose that if you want to be creative.

^ permalink raw reply

* [PATCH net] vhost-net: fix TX stall when vhost owns virtio-net header
From: enrico.zanda @ 2026-07-08 15:22 UTC (permalink / raw)
  To: jasowangio, virtualization, mst, netdev, kuba
  Cc: kvm, linux-kernel, eperezma, nd, Enrico Zanda

From: Enrico Zanda <enrico.zanda@arm.com>

When vhost owns the virtio-net header, i.e. when
VHOST_NET_F_VIRTIO_NET_HDR is negotiated, sock_hlen is 0,
meaning that no header will be forwarded to the TAP device.

In the current vhost_net_build_xdp() implementation,
when sock_hlen == 0, the gso pointer can point at the start of the
Ethernet frame instead of a virtio-net header.
This results in a wrong interpretation of the destination MAC address
bytes as struct virtio_net_hdr fields.

This can, for some MAC addresses, trigger -EINVAL and return early
before the TX descriptor is completed, which can stall vhost-net TX.

Before 97b2409f28e0, the gso pointer was set to the zeroed padding area,
using it as a synthetic virtio-net header. Restore that behavior.

Fixes: 97b2409f28e0 ("vhost-net: reduce one userspace copy when building XDP buff")
Signed-off-by: Enrico Zanda <enrico.zanda@arm.com>
---
 drivers/vhost/net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 77b59f49bddb..3e72b9c6af0c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -731,10 +731,12 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue *nvq,
 		goto err;
 	}
 
-	gso = buf + pad - sock_hlen;
-
-	if (!sock_hlen)
+	if (!sock_hlen) {
 		memset(buf, 0, pad);
+		gso = buf;
+	} else {
+		gso = buf + pad - sock_hlen;
+	}
 
 	if ((gso->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
 	    vhost16_to_cpu(vq, gso->csum_start) +
-- 
2.43.0


^ permalink raw reply related

* [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Joseph Qi @ 2026-07-08 12:42 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: virtualization, linux-kernel, Christoph Hellwig, Baokun Li

async_pmem_flush() allocates a child bio for the flush with GFP_ATOMIC.
This runs from pmem_submit_bio(), a ->submit_bio callback that executes
in a sleepable context, so there is no atomicity requirement here.

bio_alloc() only guarantees success when __GFP_DIRECT_RECLAIM is set,
because that is what lets it fall back to the mempool reserve. With
GFP_ATOMIC the reclaim bit is absent, so the allocation can fail and
return -ENOMEM whenever the fast paths (percpu cache and slab) are
exhausted, which is common right after boot. A flush is issued from
filesystem writeback and must not fail on a transient allocation
shortage, otherwise the device can appear unmountable:

  Buffer I/O error on dev pmem0, logical block 0, lost sync page write

Use GFP_NOIO instead. It keeps __GFP_DIRECT_RECLAIM so the mempool and
rescuer machinery guarantee forward progress, while avoiding recursion
back into the filesystem and block layer during writeback.

Fixes: 6e84200c0a29 ("virtio-pmem: Add virtio pmem driver")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 drivers/nvdimm/nd_virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 4176046627beb..081370aac6317 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
 	if (bio && bio->bi_iter.bi_sector != -1) {
 		struct bio *child = bio_alloc(bio->bi_bdev, 0,
 					      REQ_OP_WRITE | REQ_PREFLUSH,
-					      GFP_ATOMIC);
+					      GFP_NOIO);
 
 		if (!child)
 			return -ENOMEM;
-- 
2.39.3


^ permalink raw reply related

* Re: [PATCH v2] drm/qxl: remove dependency on DRM simple helpers
From: Thomas Zimmermann @ 2026-07-08 12:33 UTC (permalink / raw)
  To: Diogo Silva, Dave Airlie, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, David Airlie, Simona Vetter
  Cc: virtualization, spice-devel, dri-devel, linux-kernel
In-Reply-To: <20260707-qxl-simple-v2-1-08d21bc74a41@gmail.com>



Am 07.07.26 um 18:14 schrieb Diogo Silva:
> Simple KMS helper are deprecated since they only add an intermediate
> layer between drivers and the atomic modesetting.
> This patch removes the drm_simple_encoder_init() helper usage in the
> qxl display driver by open coding it and using the encoder atomic
> helpers directly. This is a step to eventually get rid of this simple
> KMS helper, once all drivers that use it have been converted.
>
> Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> Changes in v2:
> - fix type error
> - Link to v1: https://lore.kernel.org/r/20260707-qxl-simple-v1-1-524f6316b5d5@gmail.com
> ---
>   drivers/gpu/drm/qxl/qxl_display.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index a026bd35ef485..7f4178800afd7 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -31,12 +31,12 @@
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_encoder.h>
>   #include <drm/drm_framebuffer.h>
>   #include <drm/drm_gem_framebuffer_helper.h>
>   #include <drm/drm_plane_helper.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
>   #include <drm/drm_gem_atomic_helper.h>
>   #include <drm/drm_vblank.h>
>   #include <drm/drm_vblank_helper.h>
> @@ -1095,6 +1095,10 @@ static const struct drm_connector_helper_funcs qxl_connector_helper_funcs = {
>   	.best_encoder = qxl_best_encoder,
>   };
>   
> +static const struct drm_encoder_funcs qxl_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
>   static enum drm_connector_status qxl_conn_detect(
>   			struct drm_connector *connector,
>   			bool force)
> @@ -1169,10 +1173,10 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
>   	drm_connector_init(dev, &qxl_output->base,
>   			   &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
>   
> -	ret = drm_simple_encoder_init(dev, &qxl_output->enc,
> -				      DRM_MODE_ENCODER_VIRTUAL);
> +	ret = drm_encoder_init(dev, &qxl_output->enc, &qxl_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>   	if (ret) {
> -		drm_err(dev, "drm_simple_encoder_init() failed, error %d\n",
> +		drm_err(dev, "drm_encoder_init() failed, error %d\n",
>   			ret);
>   		goto err_drm_connector_cleanup;
>   	}
>
> ---
> base-commit: ee2867b79f9bac3a6fc3221139b09598ce79099c
> change-id: 20260707-qxl-simple-c01aa1a5c4eb
>
> Best regards,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)



^ permalink raw reply

* [PATCH] x86/vmware: mark RDI and RSI clobbered in hypercall1
From: Guangshuo Li @ 2026-07-08 11:57 UTC (permalink / raw)
  To: Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, virtualization, linux-kernel
  Cc: Guangshuo Li

vmware_hypercall1() uses inline asm for the VMware backdoor hypercall,
but its clobber list does not include RDI and RSI.

QEMU's vmmouse emulation can restore those registers through 32-bit
storage, clearing the upper halves of RDI and RSI. If the compiler keeps
a live 64-bit pointer in either register across vmware_hypercall1(), the
pointer can be truncated and a later dereference can fault.

Mark RDI and RSI as clobbered for vmware_hypercall1(), matching the
protection already used by the other VMware hypercall wrappers which are
affected by the same register-clobbering behavior.

Fixes: 34bf25e820ae ("x86/vmware: Introduce VMware hypercall API")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 arch/x86/include/asm/vmware.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index 4220dae14a2d..b9d9c4b98238 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -115,7 +115,7 @@ unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
 		  "b" (in1),
 		  "c" (cmd),
 		  "d" (0)
-		: "cc", "memory");
+		: "di", "si", "cc", "memory");
 	return out0;
 }
 
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Michael S. Tsirkin @ 2026-07-08 11:00 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
	Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable,
	Brien Oberstein
In-Reply-To: <20260708102904.50732-2-sgarzare@redhat.com>

On Wed, Jul 08, 2026 at 12:29:03PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> When many small packets accumulate in the receive queue, the skb overhead
> can exceed buf_alloc even while the payload is within bounds. This causes
> virtio_transport_inc_rx_pkt() to reject packets, leading to connection
> resets during large transfers under backpressure.
> 
> The issue was reported by Brien, who has a reproducer, but it is also
> easily reproducible with iperf-vsock [1] using a small packet size:
> 
>   iperf3 --vsock -c $CID -l 129
> 
> which fails immediately without this patch but with commit 059b7dbd20a6
> ("vsock/virtio: fix potential unbounded skb queue").
> 
> Inspired by TCP's tcp_collapse() which solves a similar problem, add
> virtio_transport_collapse_rx_queue() that walks the receive queue and
> re-copies data into compact linear skbs to reduce the overhead.
> 
> The collapse is triggered proactively from when the number of skb queued
> is close to exceeding the overhead budget.
> 
> A pre-scan counts the eligible bytes to size each allocation precisely,
> avoiding waste for isolated small packets. Partially consumed skbs are
> kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to
> maintain SEQPACKET message boundaries, and skbs already larger than the
> collapse target because they already have a good data-to-overhead ratio.
> 
> Walking a large queue may take a significant amount of time and cache
> misses, causing traffic burstiness. To limit this, the collapse stops
> once enough room is freed for this packet and the next one, but may
> opportunistically free more to fill each collapsed skb to capacity.
> 
> [1] https://github.com/stefano-garzarella/iperf-vsock
> 
> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> Cc: stable@vger.kernel.org
> Reported-by: Brien Oberstein <brienpub@gmail.com>
> Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/
> Tested-by: Brien Oberstein <brienpub@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


this is the right approach

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> v2:
> - defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo]
> - added a threshold to avoid walking all the queue while collapsing
>   [Paolo]
> - collapsed the queue before calling virtio_transport_inc_rx_pkt().
>   While working on the threshold, I figured out that the check I was
>   introducing can also be used to proactively trigger the collapse, so I
>   moved the call to virtio_transport_collapse_rx_queue() before acquiring
>   the rx_lock to have also a better diff to simplify backports
> - improved code readability (removed `out` label, `keep` initialization,
>   etc.) [Paolo + other small stuff]
> - Brien kindly retested this version as well (thank you so much)
> ---
>  net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++-
>  1 file changed, 164 insertions(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 09475007165b..8becad81279c 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -26,6 +26,13 @@
>  /* Threshold for detecting small packets to copy */
>  #define GOOD_COPY_LEN  128
>  
> +/* Max payload that can be collapsed into a single linear skb, using the same
> + * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure
> + * on the page allocator.
> + */
> +#define MAX_COLLAPSE_LEN \
> +	SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
> +
>  static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
>  					       bool cancel_timeout);
>  static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
> @@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	return ret;
>  }
>  
> +static bool virtio_transport_can_collapse(struct sk_buff *skb)
> +{
> +	/* skbs that are partially consumed, mark a SEQPACKET message boundary,
> +	 * or are already large enough should not be collapsed: they either
> +	 * need special accounting, carry protocol state, or already have a
> +	 * good data-to-overhead ratio.
> +	 */
> +	if (VIRTIO_VSOCK_SKB_CB(skb)->offset)
> +		return false;
> +	if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM)
> +		return false;
> +	if (skb->len >= MAX_COLLAPSE_LEN)
> +		return false;
> +	return true;
> +}
> +
> +/* Iterate through the packets in the queue starting from the current skb to
> + * count the number of bytes we can collapse.
> + */
> +static unsigned int
> +virtio_transport_collapse_size(struct sk_buff *skb, struct sk_buff_head *queue)
> +{
> +	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +	while ((skb = skb_peek_next(skb, queue)) &&
> +	       virtio_transport_can_collapse(skb)) {
> +		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +		if (len > MAX_COLLAPSE_LEN - target)
> +			return target;
> +
> +		target += len;
> +	}
> +
> +	return target;
> +}
> +
> +/* Called under lock_sock to compact the receive queue by merging small skbs.
> + * @min_to_free: minimum number of skbs to eliminate from the queue. May free
> + *               more to fill each collapsed skb to capacity.
> + */
> +static void
> +virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
> +				   u32 min_to_free)
> +{
> +	struct sk_buff *skb, *next_skb, *new_skb = NULL;
> +	struct sk_buff_head new_queue;
> +	u32 saved = 0;
> +
> +	__skb_queue_head_init(&new_queue);
> +
> +	skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {
> +		struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> +		u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +		u32 src_len = skb->len - src_off;
> +		bool keep;
> +
> +		keep = !virtio_transport_can_collapse(skb);
> +		if (keep) {
> +			/* Finalize pending collapsed skb to preserve packet
> +			 * ordering.
> +			 */
> +			if (new_skb) {
> +				__skb_queue_tail(&new_queue, new_skb);
> +				new_skb = NULL;
> +				saved--;
> +			}
> +			goto next;
> +		}
> +
> +		/* Finalize if this packet won't fit in the remaining tailroom,
> +		 * so we can allocate a right-sized new_skb.
> +		 */
> +		if (new_skb && src_len > skb_tailroom(new_skb)) {
> +			__skb_queue_tail(&new_queue, new_skb);
> +			new_skb = NULL;
> +			saved--;
> +		}
> +
> +		if (!new_skb) {
> +			unsigned int alloc_size;
> +
> +			/* Check after finalizing to opportunistically fill
> +			 * each collapsed skb to capacity, merging more skbs
> +			 * than strictly required.
> +			 */
> +			if (saved >= min_to_free)
> +				break;
> +
> +			alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue);
> +
> +			/* Only this skb's data is eligible, nothing to merge
> +			 * with. Keep as-is.
> +			 */
> +			if (alloc_size <= src_len) {
> +				keep = true;
> +				goto next;
> +			}
> +
> +			new_skb = virtio_vsock_alloc_linear_skb(alloc_size +
> +					VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL);
> +			if (!new_skb)
> +				break;
> +
> +			memcpy(virtio_vsock_hdr(new_skb), hdr,
> +			       sizeof(struct virtio_vsock_hdr));
> +			virtio_vsock_hdr(new_skb)->len = 0;
> +		}
> +
> +		/* Cannot fail since src_off/src_len are within bounds, but if
> +		 * it does, discard new_skb to avoid queuing corrupted data.
> +		 */
> +		if (WARN_ON_ONCE(skb_copy_bits(skb, src_off,
> +					       skb_put(new_skb, src_len),
> +					       src_len))) {
> +			kfree_skb(new_skb);
> +			new_skb = NULL;
> +			break;
> +		}
> +
> +		le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len);
> +		virtio_vsock_hdr(new_skb)->flags |= hdr->flags;
> +
> +next:
> +		__skb_unlink(skb, &vvs->rx_queue);
> +		if (keep) {
> +			__skb_queue_tail(&new_queue, skb);
> +		} else {
> +			consume_skb(skb);
> +			saved++;
> +		}
> +	}
> +
> +	if (new_skb)
> +		__skb_queue_tail(&new_queue, new_skb);
> +
> +	skb_queue_splice(&new_queue, &vvs->rx_queue);
> +}
> +
>  static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>  					u32 len)
>  {
> @@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>  {
>  	struct virtio_vsock_sock *vvs = vsk->trans;
>  	bool can_enqueue, free_pkt = false;
> +	u32 len, queue_max, queue_len;
>  	struct virtio_vsock_hdr *hdr;
> -	u32 len;
>  
>  	hdr = virtio_vsock_hdr(skb);
>  	len = le32_to_cpu(hdr->len);
>  
> +	/* virtio_transport_inc_rx_pkt() rejects packets when the per-skb
> +	 * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc.
> +	 * Proactively collapse the queue before that happens.
> +	 * No rx_lock needed: lock_sock is held by caller, preventing
> +	 * concurrent enqueue or dequeue.
> +	 */
> +	queue_max = vvs->buf_alloc / SKB_TRUESIZE(0);
> +	queue_len = skb_queue_len(&vvs->rx_queue);
> +	if (queue_len >= queue_max) {
> +		/* Walking a large queue may take a significant amount of time
> +		 * and cache misses, causing traffic burstiness. Limit the
> +		 * collapse to freeing room for this packet and the next one.
> +		 * It may free more to fill each collapsed skb to capacity.
> +		 */
> +		virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max);
> +	}
> +
>  	spin_lock_bh(&vvs->rx_lock);
>  
>  	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> -- 
> 2.55.0


^ permalink raw reply

* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Michael S. Tsirkin @ 2026-07-08 10:59 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
	Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang
In-Reply-To: <20260708102904.50732-3-sgarzare@redhat.com>

On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> Add a test that sends 2 MB of data using randomly sized small packets
> (129-512 bytes) over a SOCK_STREAM connection. Packets above
> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
> forcing each one into its own skb.
> 
> Without receive queue collapsing, the per-skb overhead eventually
> exceeds buf_alloc and the connection is reset. The test verifies
> that all data arrives and that content integrity is preserved.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to
trigger?

anyway

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 76be0e4a7f0e..b4ff9f946565 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts)
>  	close(fd);
>  }
>  
> +/* Test that many small packets don't cause a connection reset under pressure
> + * and that data integrity is preserved.  Packet sizes vary randomly between
> + * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing
> + * in recv_enqueue, forcing each one into its own skb.  Without receive queue
> + * collapsing, the per-skb overhead eventually exceeds buf_alloc and the
> + * connection is reset.
> + */
> +#define COLLAPSE_PKT_MIN 129
> +#define COLLAPSE_PKT_MAX 512
> +#define COLLAPSE_TOTAL (2 * 1024 * 1024)
> +
> +static void test_stream_collapse_client(const struct test_opts *opts)
> +{
> +	unsigned char *data;
> +	unsigned long hash;
> +	size_t offset = 0;
> +	int i, fd;
> +
> +	data = malloc(COLLAPSE_TOTAL);
> +	if (!data) {
> +		perror("malloc");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	for (i = 0; i < COLLAPSE_TOTAL; i++)
> +		data[i] = rand() & 0xff;
> +
> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
> +	if (fd < 0) {
> +		perror("connect");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	while (offset < COLLAPSE_TOTAL) {
> +		size_t pkt_size = COLLAPSE_PKT_MIN +
> +			rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1);
> +
> +		pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset);
> +
> +		send_buf(fd, data + offset, pkt_size, 0, pkt_size);
> +		offset += pkt_size;
> +	}
> +
> +	hash = hash_djb2(data, COLLAPSE_TOTAL);
> +	control_writeulong(hash);
> +
> +	free(data);
> +	close(fd);
> +}
> +
> +static void test_stream_collapse_server(const struct test_opts *opts)
> +{
> +	unsigned long hash, remote_hash;
> +	unsigned char *data;
> +	int fd;
> +
> +	data = malloc(COLLAPSE_TOTAL);
> +	if (!data) {
> +		perror("malloc");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
> +	if (fd < 0) {
> +		perror("accept");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL);
> +
> +	hash = hash_djb2(data, COLLAPSE_TOTAL);
> +	remote_hash = control_readulong();
> +	if (hash != remote_hash) {
> +		fprintf(stderr, "hash mismatch: local %lu remote %lu\n",
> +			hash, remote_hash);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	free(data);
> +	close(fd);
> +}
> +
>  static struct test_case test_cases[] = {
>  	{
>  		.name = "SOCK_STREAM connection reset",
> @@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = {
>  		.run_client = test_stream_msg_peek_client,
>  		.run_server = test_stream_peek_after_recv_server,
>  	},
> +	{
> +		.name = "SOCK_STREAM small packets backpressure",
> +		.run_client = test_stream_collapse_client,
> +		.run_server = test_stream_collapse_server,
> +	},
>  	{},
>  };
>  
> -- 
> 2.55.0


^ permalink raw reply

* [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet,
	Eugenio Pérez, Simon Horman, Stefan Hajnoczi,
	David S. Miller, linux-kernel, Michael S. Tsirkin, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang
In-Reply-To: <20260708102904.50732-1-sgarzare@redhat.com>

From: Stefano Garzarella <sgarzare@redhat.com>

Add a test that sends 2 MB of data using randomly sized small packets
(129-512 bytes) over a SOCK_STREAM connection. Packets above
GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
forcing each one into its own skb.

Without receive queue collapsing, the per-skb overhead eventually
exceeds buf_alloc and the connection is reset. The test verifies
that all data arrives and that content integrity is preserved.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 76be0e4a7f0e..b4ff9f946565 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts)
 	close(fd);
 }
 
+/* Test that many small packets don't cause a connection reset under pressure
+ * and that data integrity is preserved.  Packet sizes vary randomly between
+ * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing
+ * in recv_enqueue, forcing each one into its own skb.  Without receive queue
+ * collapsing, the per-skb overhead eventually exceeds buf_alloc and the
+ * connection is reset.
+ */
+#define COLLAPSE_PKT_MIN 129
+#define COLLAPSE_PKT_MAX 512
+#define COLLAPSE_TOTAL (2 * 1024 * 1024)
+
+static void test_stream_collapse_client(const struct test_opts *opts)
+{
+	unsigned char *data;
+	unsigned long hash;
+	size_t offset = 0;
+	int i, fd;
+
+	data = malloc(COLLAPSE_TOTAL);
+	if (!data) {
+		perror("malloc");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = 0; i < COLLAPSE_TOTAL; i++)
+		data[i] = rand() & 0xff;
+
+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	while (offset < COLLAPSE_TOTAL) {
+		size_t pkt_size = COLLAPSE_PKT_MIN +
+			rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1);
+
+		pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset);
+
+		send_buf(fd, data + offset, pkt_size, 0, pkt_size);
+		offset += pkt_size;
+	}
+
+	hash = hash_djb2(data, COLLAPSE_TOTAL);
+	control_writeulong(hash);
+
+	free(data);
+	close(fd);
+}
+
+static void test_stream_collapse_server(const struct test_opts *opts)
+{
+	unsigned long hash, remote_hash;
+	unsigned char *data;
+	int fd;
+
+	data = malloc(COLLAPSE_TOTAL);
+	if (!data) {
+		perror("malloc");
+		exit(EXIT_FAILURE);
+	}
+
+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
+	if (fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL);
+
+	hash = hash_djb2(data, COLLAPSE_TOTAL);
+	remote_hash = control_readulong();
+	if (hash != remote_hash) {
+		fprintf(stderr, "hash mismatch: local %lu remote %lu\n",
+			hash, remote_hash);
+		exit(EXIT_FAILURE);
+	}
+
+	free(data);
+	close(fd);
+}
+
 static struct test_case test_cases[] = {
 	{
 		.name = "SOCK_STREAM connection reset",
@@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = {
 		.run_client = test_stream_msg_peek_client,
 		.run_server = test_stream_peek_after_recv_server,
 	},
+	{
+		.name = "SOCK_STREAM small packets backpressure",
+		.run_client = test_stream_collapse_client,
+		.run_server = test_stream_collapse_server,
+	},
 	{},
 };
 
-- 
2.55.0


^ permalink raw reply related

* [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet,
	Eugenio Pérez, Simon Horman, Stefan Hajnoczi,
	David S. Miller, linux-kernel, Michael S. Tsirkin, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable,
	Brien Oberstein
In-Reply-To: <20260708102904.50732-1-sgarzare@redhat.com>

From: Stefano Garzarella <sgarzare@redhat.com>

When many small packets accumulate in the receive queue, the skb overhead
can exceed buf_alloc even while the payload is within bounds. This causes
virtio_transport_inc_rx_pkt() to reject packets, leading to connection
resets during large transfers under backpressure.

The issue was reported by Brien, who has a reproducer, but it is also
easily reproducible with iperf-vsock [1] using a small packet size:

  iperf3 --vsock -c $CID -l 129

which fails immediately without this patch but with commit 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue").

Inspired by TCP's tcp_collapse() which solves a similar problem, add
virtio_transport_collapse_rx_queue() that walks the receive queue and
re-copies data into compact linear skbs to reduce the overhead.

The collapse is triggered proactively from when the number of skb queued
is close to exceeding the overhead budget.

A pre-scan counts the eligible bytes to size each allocation precisely,
avoiding waste for isolated small packets. Partially consumed skbs are
kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to
maintain SEQPACKET message boundaries, and skbs already larger than the
collapse target because they already have a good data-to-overhead ratio.

Walking a large queue may take a significant amount of time and cache
misses, causing traffic burstiness. To limit this, the collapse stops
once enough room is freed for this packet and the next one, but may
opportunistically free more to fill each collapsed skb to capacity.

[1] https://github.com/stefano-garzarella/iperf-vsock

Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Cc: stable@vger.kernel.org
Reported-by: Brien Oberstein <brienpub@gmail.com>
Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/
Tested-by: Brien Oberstein <brienpub@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo]
- added a threshold to avoid walking all the queue while collapsing
  [Paolo]
- collapsed the queue before calling virtio_transport_inc_rx_pkt().
  While working on the threshold, I figured out that the check I was
  introducing can also be used to proactively trigger the collapse, so I
  moved the call to virtio_transport_collapse_rx_queue() before acquiring
  the rx_lock to have also a better diff to simplify backports
- improved code readability (removed `out` label, `keep` initialization,
  etc.) [Paolo + other small stuff]
- Brien kindly retested this version as well (thank you so much)
---
 net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++-
 1 file changed, 164 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 09475007165b..8becad81279c 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -26,6 +26,13 @@
 /* Threshold for detecting small packets to copy */
 #define GOOD_COPY_LEN  128
 
+/* Max payload that can be collapsed into a single linear skb, using the same
+ * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure
+ * on the page allocator.
+ */
+#define MAX_COLLAPSE_LEN \
+	SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
+
 static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
 					       bool cancel_timeout);
 static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
@@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 	return ret;
 }
 
+static bool virtio_transport_can_collapse(struct sk_buff *skb)
+{
+	/* skbs that are partially consumed, mark a SEQPACKET message boundary,
+	 * or are already large enough should not be collapsed: they either
+	 * need special accounting, carry protocol state, or already have a
+	 * good data-to-overhead ratio.
+	 */
+	if (VIRTIO_VSOCK_SKB_CB(skb)->offset)
+		return false;
+	if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM)
+		return false;
+	if (skb->len >= MAX_COLLAPSE_LEN)
+		return false;
+	return true;
+}
+
+/* Iterate through the packets in the queue starting from the current skb to
+ * count the number of bytes we can collapse.
+ */
+static unsigned int
+virtio_transport_collapse_size(struct sk_buff *skb, struct sk_buff_head *queue)
+{
+	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
+
+	while ((skb = skb_peek_next(skb, queue)) &&
+	       virtio_transport_can_collapse(skb)) {
+		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
+
+		if (len > MAX_COLLAPSE_LEN - target)
+			return target;
+
+		target += len;
+	}
+
+	return target;
+}
+
+/* Called under lock_sock to compact the receive queue by merging small skbs.
+ * @min_to_free: minimum number of skbs to eliminate from the queue. May free
+ *               more to fill each collapsed skb to capacity.
+ */
+static void
+virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
+				   u32 min_to_free)
+{
+	struct sk_buff *skb, *next_skb, *new_skb = NULL;
+	struct sk_buff_head new_queue;
+	u32 saved = 0;
+
+	__skb_queue_head_init(&new_queue);
+
+	skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {
+		struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
+		u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset;
+		u32 src_len = skb->len - src_off;
+		bool keep;
+
+		keep = !virtio_transport_can_collapse(skb);
+		if (keep) {
+			/* Finalize pending collapsed skb to preserve packet
+			 * ordering.
+			 */
+			if (new_skb) {
+				__skb_queue_tail(&new_queue, new_skb);
+				new_skb = NULL;
+				saved--;
+			}
+			goto next;
+		}
+
+		/* Finalize if this packet won't fit in the remaining tailroom,
+		 * so we can allocate a right-sized new_skb.
+		 */
+		if (new_skb && src_len > skb_tailroom(new_skb)) {
+			__skb_queue_tail(&new_queue, new_skb);
+			new_skb = NULL;
+			saved--;
+		}
+
+		if (!new_skb) {
+			unsigned int alloc_size;
+
+			/* Check after finalizing to opportunistically fill
+			 * each collapsed skb to capacity, merging more skbs
+			 * than strictly required.
+			 */
+			if (saved >= min_to_free)
+				break;
+
+			alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue);
+
+			/* Only this skb's data is eligible, nothing to merge
+			 * with. Keep as-is.
+			 */
+			if (alloc_size <= src_len) {
+				keep = true;
+				goto next;
+			}
+
+			new_skb = virtio_vsock_alloc_linear_skb(alloc_size +
+					VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL);
+			if (!new_skb)
+				break;
+
+			memcpy(virtio_vsock_hdr(new_skb), hdr,
+			       sizeof(struct virtio_vsock_hdr));
+			virtio_vsock_hdr(new_skb)->len = 0;
+		}
+
+		/* Cannot fail since src_off/src_len are within bounds, but if
+		 * it does, discard new_skb to avoid queuing corrupted data.
+		 */
+		if (WARN_ON_ONCE(skb_copy_bits(skb, src_off,
+					       skb_put(new_skb, src_len),
+					       src_len))) {
+			kfree_skb(new_skb);
+			new_skb = NULL;
+			break;
+		}
+
+		le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len);
+		virtio_vsock_hdr(new_skb)->flags |= hdr->flags;
+
+next:
+		__skb_unlink(skb, &vvs->rx_queue);
+		if (keep) {
+			__skb_queue_tail(&new_queue, skb);
+		} else {
+			consume_skb(skb);
+			saved++;
+		}
+	}
+
+	if (new_skb)
+		__skb_queue_tail(&new_queue, new_skb);
+
+	skb_queue_splice(&new_queue, &vvs->rx_queue);
+}
+
 static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
 					u32 len)
 {
@@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
 {
 	struct virtio_vsock_sock *vvs = vsk->trans;
 	bool can_enqueue, free_pkt = false;
+	u32 len, queue_max, queue_len;
 	struct virtio_vsock_hdr *hdr;
-	u32 len;
 
 	hdr = virtio_vsock_hdr(skb);
 	len = le32_to_cpu(hdr->len);
 
+	/* virtio_transport_inc_rx_pkt() rejects packets when the per-skb
+	 * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc.
+	 * Proactively collapse the queue before that happens.
+	 * No rx_lock needed: lock_sock is held by caller, preventing
+	 * concurrent enqueue or dequeue.
+	 */
+	queue_max = vvs->buf_alloc / SKB_TRUESIZE(0);
+	queue_len = skb_queue_len(&vvs->rx_queue);
+	if (queue_len >= queue_max) {
+		/* Walking a large queue may take a significant amount of time
+		 * and cache misses, causing traffic burstiness. Limit the
+		 * collapse to freeing room for this packet and the next one.
+		 * It may free more to fill each collapsed skb to capacity.
+		 */
+		virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max);
+	}
+
 	spin_lock_bh(&vvs->rx_lock);
 
 	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
-- 
2.55.0


^ permalink raw reply related

* [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure
From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet,
	Eugenio Pérez, Simon Horman, Stefan Hajnoczi,
	David S. Miller, linux-kernel, Michael S. Tsirkin, kvm,
	Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang

This series contains a patch (the first one) that is part of work I'm
doing to improve the tracking of memory used by AF_VSOCK sockets.
The second patch is a test for our suite that highlights the issue.

Since Brien reported an issue with his environment (based on Linux 6.12.y)
related to the work I’m doing, I extracted this patch and tried to make it
as easy as possible to backport. Brien tested it by backporting it to
6.12.y, which now contains the backport of the 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue").

This patch primarily fixes STREAM sockets, but also partially fixes
SEQPACKET (with the exception of EOMs, which are kept in separate skbs to
avoid overcomplicating the code).

The rest of the work, I feel, is more net-next material and still needs
some work to be completed.

Changelog
---------

v2:
- defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo]
- added a threshold to avoid walking all the queue while collapsing
  [Paolo]
- collapsed the queue before calling virtio_transport_inc_rx_pkt().
  While working on the threshold, I figured out that the check I was
  introducing can also be used to proactively trigger the collapse, so I
  moved the call to virtio_transport_collapse_rx_queue() before acquiring
  the rx_lock to have also a better diff to simplify backports
- improved code readability (removed `out` label, `keep` initialization,
  etc.) [Paolo + other small stuff]
- Brien kindly retested this version as well (thank you so much)

v1: https://lore.kernel.org/netdev/20260626134823.206676-1-sgarzare@redhat.com/

Thanks,
Stefano

Stefano Garzarella (2):
  vsock/virtio: collapse receive queue under memory pressure
  vsock/test: add test for small packets under pressure

 net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++-
 tools/testing/vsock/vsock_test.c        |  87 +++++++++++++
 2 files changed, 251 insertions(+), 1 deletion(-)

-- 
2.55.0


^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox