* Re: [PATCH net-next v4] net: mana: Add MAC address to vPort logs and clarify error messages
From: Jakub Kicinski @ 2026-02-28 17:55 UTC (permalink / raw)
To: Erni Sri Satya Vennela
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, pabeni, dipayanroy, shirazsaleem, ssengar, shradhagupta,
gargaditya, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260225192252.943534-1-ernis@linux.microsoft.com>
On Wed, 25 Feb 2026 11:22:41 -0800 Erni Sri Satya Vennela wrote:
> - dev_err(hwc->dev, "HWC: Request timed out: %u ms\n",
> - hwc->hwc_timeout);
> + dev_err(hwc->dev, "%s:%d: Command 0x%x timed out: %u ms\n",
> + __func__, __LINE__, command, hwc->hwc_timeout);
Please don't include __LINE__, they are meaningless given the amount of
backporting that usually happens in the kernel. The string should be
unique enough to identify the error, which I think yours is given the
__func__ + text you have.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH] scsi: storvsc: Fix scheduling while atomic on PREEMPT_RT
From: Martin K. Petersen @ 2026-02-28 23:12 UTC (permalink / raw)
To: Jan Kiszka
Cc: Martin K. Petersen, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, James E.J. Bottomley, linux-hyperv,
linux-scsi, Linux Kernel Mailing List, Florian Bezdeka, RT,
Mitchell Levy
In-Reply-To: <898e9467-0c05-46b4-a3ed-518797b829c5@siemens.com>
Jan,
>> Applied to 7.0/scsi-fixes, thanks!
>>
>> [1/1] scsi: storvsc: Fix scheduling while atomic on PREEMPT_RT
>> https://git.kernel.org/mkp/scsi/c/57297736c082
>>
>
> Should it be here then already?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/log/?h=7.0/scsi-fixes
Because it was part of a merge of the outstanding commits from
7.0/scsi-queue, this patch doesn't appear at the top of the history for
7.0/scsi-fixes.
It is there, though:
$ git ol v7.0-rc1..7.0/scsi-fixes | grep storvsc
57297736c082 ("scsi: storvsc: Fix scheduling while atomic on PREEMPT_RT")
--
Martin K. Petersen
^ permalink raw reply
* Re: [PATCH net-next v4] net: mana: Add MAC address to vPort logs and clarify error messages
From: Simon Horman @ 2026-03-01 16:41 UTC (permalink / raw)
To: Erni Sri Satya Vennela
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, dipayanroy, shirazsaleem, ssengar,
shradhagupta, gargaditya, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260225192252.943534-1-ernis@linux.microsoft.com>
On Wed, Feb 25, 2026 at 11:22:41AM -0800, Erni Sri Satya Vennela wrote:
> Add MAC address to vPort configuration success message and update error
> message to be more specific about HWC message errors in
> mana_send_request.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
...
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
...
> @@ -893,8 +895,8 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
> if (!wait_for_completion_timeout(&ctx->comp_event,
> (msecs_to_jiffies(hwc->hwc_timeout)))) {
> if (hwc->hwc_timeout != 0)
> - dev_err(hwc->dev, "HWC: Request timed out: %u ms\n",
> - hwc->hwc_timeout);
> + dev_err(hwc->dev, "%s:%d: Command 0x%x timed out: %u ms\n",
> + __func__, __LINE__, command, hwc->hwc_timeout);
I have reservations about the usefulness of including __func__ and __LINE__
in debug messages. In a nutshell, it requires the logs to be correlated
(exactly?) with the source used to build the driver. And at that point
I think other mechanism - e.g. dynamic trace points - are going to be
useful if the debug message (without function and line information)
is insufficient to pinpoint the problem.
This is a general statement, rather than something specifically
about this code. But nonetheless I'd advise against adding this
information here.
>
> /* Reduce further waiting if HWC no response */
> if (hwc->hwc_timeout > 1)
> @@ -914,9 +916,9 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
> err = -EOPNOTSUPP;
> goto out;
> }
> - if (req_msg->req.msg_type != MANA_QUERY_PHY_STAT)
> - dev_err(hwc->dev, "HWC: Failed hw_channel req: 0x%x\n",
> - ctx->status_code);
> + if (command != MANA_QUERY_PHY_STAT)
> + dev_err(hwc->dev, "%s:%d: Command 0x%x failed with status: 0x%x\n",
> + __func__, __LINE__, command, ctx->status_code);
> err = -EPROTO;
> goto out;
> }
...
^ permalink raw reply
* [PATCH v2] mshv: Introduce tracing support
From: Stanislav Kinsburskii @ 2026-03-01 17:39 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli; +Cc: linux-hyperv, linux-kernel
Introduces various trace events and use them in the corresponding places
in the driver.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
drivers/hv/Makefile | 1
drivers/hv/mshv_eventfd.c | 14 +
drivers/hv/mshv_irq.c | 4
drivers/hv/mshv_root.h | 1
drivers/hv/mshv_root_hv_call.c | 22 +-
drivers/hv/mshv_root_main.c | 78 +++++-
drivers/hv/mshv_trace.c | 9 +
drivers/hv/mshv_trace.h | 515 ++++++++++++++++++++++++++++++++++++++++
8 files changed, 629 insertions(+), 15 deletions(-)
create mode 100644 drivers/hv/mshv_trace.c
create mode 100644 drivers/hv/mshv_trace.h
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 2593711c3628..888a748cc7cb 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -16,6 +16,7 @@ hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
mshv_root-y := mshv_root_main.o mshv_synic.o mshv_eventfd.o mshv_irq.o \
mshv_root_hv_call.o mshv_portid_table.o mshv_regions.o
mshv_root-$(CONFIG_DEBUG_FS) += mshv_debugfs.o
+mshv_root-$(CONFIG_TRACEPOINTS) += mshv_trace.o
mshv_vtl-y := mshv_vtl_main.o
# Code that must be built-in
diff --git a/drivers/hv/mshv_eventfd.c b/drivers/hv/mshv_eventfd.c
index 492c6258045c..d2efe248ca9b 100644
--- a/drivers/hv/mshv_eventfd.c
+++ b/drivers/hv/mshv_eventfd.c
@@ -733,6 +733,14 @@ static int mshv_assign_ioeventfd(struct mshv_partition *pt,
ret = mshv_register_doorbell(pt->pt_id, ioeventfd_mmio_write,
(void *)pt, p->iovntfd_addr,
p->iovntfd_datamatch, doorbell_flags);
+
+ trace_mshv_assign_ioeventfd(pt->pt_id, p->iovntfd_addr,
+ p->iovntfd_length,
+ p->iovntfd_datamatch,
+ p->iovntfd_wildcard,
+ p->iovntfd_eventfd,
+ ret);
+
if (ret < 0)
goto unlock_fail;
@@ -780,6 +788,12 @@ static int mshv_deassign_ioeventfd(struct mshv_partition *pt,
p->iovntfd_datamatch != args->datamatch)
continue;
+ trace_mshv_deassign_ioeventfd(pt->pt_id, p->iovntfd_addr,
+ p->iovntfd_length,
+ p->iovntfd_datamatch,
+ p->iovntfd_wildcard,
+ p->iovntfd_eventfd);
+
hlist_del_rcu(&p->iovntfd_hnode);
synchronize_rcu();
ioeventfd_release(p, pt->pt_id);
diff --git a/drivers/hv/mshv_irq.c b/drivers/hv/mshv_irq.c
index 798e7e1ab06e..aba7d3c431b8 100644
--- a/drivers/hv/mshv_irq.c
+++ b/drivers/hv/mshv_irq.c
@@ -71,6 +71,10 @@ int mshv_update_routing_table(struct mshv_partition *partition,
mutex_unlock(&partition->pt_irq_lock);
synchronize_srcu_expedited(&partition->pt_irq_srcu);
+
+ trace_mshv_update_routing_table(partition->pt_id,
+ old, new, numents);
+
new = old;
out:
diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index 04c2a1910a8a..947dfb76bb19 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -17,6 +17,7 @@
#include <linux/build_bug.h>
#include <linux/mmu_notifier.h>
#include <uapi/linux/mshv.h>
+#include "mshv_trace.h"
/*
* Hypervisor must be between these version numbers (inclusive)
diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
index 317191462b63..bdcb8de7fb47 100644
--- a/drivers/hv/mshv_root_hv_call.c
+++ b/drivers/hv/mshv_root_hv_call.c
@@ -44,8 +44,7 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
struct hv_output_withdraw_memory *output_page;
struct page *page;
u16 completed;
- unsigned long remaining = count;
- u64 status;
+ u64 status, withdrawn = 0;
int i;
unsigned long flags;
@@ -54,7 +53,7 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
return -ENOMEM;
output_page = page_address(page);
- while (remaining) {
+ while (withdrawn < count) {
local_irq_save(flags);
input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
@@ -62,7 +61,7 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
memset(input_page, 0, sizeof(*input_page));
input_page->partition_id = partition_id;
status = hv_do_rep_hypercall(HVCALL_WITHDRAW_MEMORY,
- min(remaining, HV_WITHDRAW_BATCH_SIZE),
+ min(count - withdrawn, HV_WITHDRAW_BATCH_SIZE),
0, input_page, output_page);
local_irq_restore(flags);
@@ -78,10 +77,12 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
break;
}
- remaining -= completed;
+ withdrawn += completed;
}
free_page((unsigned long)output_page);
+ trace_mshv_hvcall_withdraw_memory(partition_id, withdrawn, status);
+
return hv_result_to_errno(status);
}
@@ -125,6 +126,8 @@ int hv_call_create_partition(u64 flags,
ret = hv_deposit_memory(hv_current_partition_id, status);
} while (!ret);
+ trace_mshv_hvcall_create_partition(flags, ret ? ret : *partition_id);
+
return ret;
}
@@ -152,6 +155,8 @@ int hv_call_initialize_partition(u64 partition_id)
ret = hv_deposit_memory(partition_id, status);
} while (!ret);
+ trace_mshv_hvcall_initialize_partition(partition_id, status);
+
return ret;
}
@@ -164,6 +169,8 @@ int hv_call_finalize_partition(u64 partition_id)
status = hv_do_fast_hypercall8(HVCALL_FINALIZE_PARTITION,
*(u64 *)&input);
+ trace_mshv_hvcall_finalize_partition(partition_id, status);
+
return hv_result_to_errno(status);
}
@@ -175,6 +182,8 @@ int hv_call_delete_partition(u64 partition_id)
input.partition_id = partition_id;
status = hv_do_fast_hypercall8(HVCALL_DELETE_PARTITION, *(u64 *)&input);
+ trace_mshv_hvcall_delete_partition(partition_id, status);
+
return hv_result_to_errno(status);
}
@@ -571,6 +580,9 @@ static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
ret = hv_deposit_memory(partition_id, status);
} while (!ret);
+ trace_mshv_hvcall_map_vp_state_page(partition_id, vp_index,
+ type, status);
+
return ret;
}
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index e6509c980763..d753f41d3b57 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -430,6 +430,17 @@ mshv_vp_dispatch(struct mshv_vp *vp, u32 flags,
status = hv_do_hypercall(HVCALL_DISPATCH_VP, input, output);
vp->run.flags.root_sched_dispatched = 0;
+ trace_mshv_hvcall_dispatch_vp(vp->vp_partition->pt_id,
+ vp->vp_index, flags,
+ output->dispatch_state,
+ output->dispatch_event,
+#if defined(CONFIG_X86_64)
+ vp->vp_register_page->interrupt_vectors.as_uint64,
+#else
+ 0,
+#endif
+ status);
+
*res = *output;
preempt_enable();
@@ -452,6 +463,9 @@ mshv_vp_clear_explicit_suspend(struct mshv_vp *vp)
ret = mshv_set_vp_registers(vp->vp_index, vp->vp_partition->pt_id,
1, &explicit_suspend);
+ trace_mshv_vp_clear_explicit_suspend(vp->vp_partition->pt_id,
+ vp->vp_index, ret);
+
if (ret)
vp_err(vp, "Failed to unsuspend\n");
@@ -494,6 +508,12 @@ mshv_vp_wait_for_hv_kick(struct mshv_vp *vp)
if (ret)
return -EINTR;
+ trace_mshv_vp_wait_for_hv_kick(vp->vp_partition->pt_id,
+ vp->vp_index,
+ vp->run.kicked_by_hv,
+ mshv_vp_dispatch_thread_blocked(vp),
+ mshv_vp_interrupt_pending(vp));
+
vp->run.flags.root_sched_blocked = 0;
vp->run.kicked_by_hv = 0;
@@ -522,6 +542,12 @@ static long mshv_run_vp_with_root_scheduler(struct mshv_vp *vp)
if (__xfer_to_guest_mode_work_pending()) {
ret = xfer_to_guest_mode_handle_work();
+
+ trace_mshv_xfer_to_guest_mode_work(vp->vp_partition->pt_id,
+ vp->vp_index,
+ read_thread_flags(),
+ ret);
+
if (ret)
break;
}
@@ -673,6 +699,8 @@ static long mshv_vp_ioctl_run_vp(struct mshv_vp *vp, void __user *ret_msg)
{
long rc;
+ trace_mshv_run_vp_entry(vp->vp_partition->pt_id, vp->vp_index);
+
do {
if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
rc = mshv_run_vp_with_root_scheduler(vp);
@@ -680,6 +708,10 @@ static long mshv_vp_ioctl_run_vp(struct mshv_vp *vp, void __user *ret_msg)
rc = mshv_run_vp_with_hyp_scheduler(vp);
} while (rc == 0 && mshv_vp_handle_intercept(vp));
+ trace_mshv_run_vp_exit(vp->vp_partition->pt_id, vp->vp_index,
+ vp->vp_intercept_msg_page->header.message_type,
+ rc);
+
if (rc)
return rc;
@@ -941,6 +973,8 @@ mshv_vp_release(struct inode *inode, struct file *filp)
{
struct mshv_vp *vp = filp->private_data;
+ trace_mshv_vp_release(vp->vp_partition->pt_id, vp->vp_index);
+
/* Rest of VP cleanup happens in destroy_partition() */
mshv_partition_put(vp->vp_partition);
return 0;
@@ -1113,7 +1147,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
partition->pt_vp_count++;
partition->pt_vp_array[args.vp_index] = vp;
- return ret;
+ goto out;
remove_debugfs_vp:
mshv_debugfs_vp_remove(vp);
@@ -1139,6 +1173,8 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
intercept_msg_page, input_vtl_zero);
destroy_vp:
hv_call_delete_vp(partition->pt_id, args.vp_index);
+out:
+ trace_mshv_create_vp(partition->pt_id, args.vp_index, ret);
return ret;
}
@@ -1338,6 +1374,10 @@ mshv_map_user_memory(struct mshv_partition *partition,
break;
}
+ trace_mshv_map_user_memory(partition->pt_id, region->start_uaddr,
+ region->start_gfn, region->nr_pages,
+ region->hv_map_flags, ret);
+
if (ret)
goto errout;
@@ -1633,6 +1673,9 @@ disable_vp_dispatch(struct mshv_vp *vp)
if (ret)
vp_err(vp, "failed to suspend\n");
+ trace_mshv_disable_vp_dispatch(vp->vp_partition->pt_id,
+ vp->vp_index, ret);
+
return ret;
}
@@ -1681,6 +1724,8 @@ drain_vp_signals(struct mshv_vp *vp)
vp->run.kicked_by_hv = 0;
vp_signal_count = atomic64_read(&vp->run.vp_signaled_count);
}
+
+ trace_mshv_drain_vp_signals(vp->vp_partition->pt_id, vp->vp_index);
}
static void drain_all_vps(const struct mshv_partition *partition)
@@ -1734,6 +1779,8 @@ static void destroy_partition(struct mshv_partition *partition)
return;
}
+ trace_mshv_destroy_partition(partition->pt_id);
+
if (partition->pt_initialized) {
/*
* We only need to drain signals for root scheduler. This should be
@@ -1840,6 +1887,8 @@ mshv_partition_release(struct inode *inode, struct file *filp)
{
struct mshv_partition *partition = filp->private_data;
+ trace_mshv_partition_release(partition->pt_id);
+
mshv_eventfd_release(partition);
cleanup_srcu_struct(&partition->pt_irq_srcu);
@@ -1969,6 +2018,7 @@ mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
struct hv_partition_creation_properties creation_properties;
union hv_partition_isolation_properties isolation_properties;
struct mshv_partition *partition;
+ u64 pt_id = -1;
long ret;
ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
@@ -2008,22 +2058,29 @@ mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
ret = hv_call_create_partition(creation_flags,
creation_properties,
isolation_properties,
- &partition->pt_id);
+ &pt_id);
if (ret)
goto cleanup_irq_srcu;
+ partition->pt_id = pt_id;
+
ret = add_partition(partition);
if (ret)
goto delete_partition;
ret = mshv_init_async_handler(partition);
- if (!ret) {
- ret = FD_ADD(O_CLOEXEC, anon_inode_getfile("mshv_partition",
- &mshv_partition_fops,
- partition, O_RDWR));
- if (ret >= 0)
- return ret;
- }
+ if (ret)
+ goto remove_partition;
+
+ ret = FD_ADD(O_CLOEXEC, anon_inode_getfile("mshv_partition",
+ &mshv_partition_fops,
+ partition, O_RDWR));
+ if (ret < 0)
+ goto remove_partition;
+
+ goto out;
+
+remove_partition:
remove_partition(partition);
delete_partition:
hv_call_delete_partition(partition->pt_id);
@@ -2031,7 +2088,8 @@ mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
cleanup_srcu_struct(&partition->pt_irq_srcu);
free_partition:
kfree(partition);
-
+out:
+ trace_mshv_create_partition(pt_id, ret);
return ret;
}
diff --git a/drivers/hv/mshv_trace.c b/drivers/hv/mshv_trace.c
new file mode 100644
index 000000000000..0936b2f95edd
--- /dev/null
+++ b/drivers/hv/mshv_trace.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026, Microsoft Corporation.
+ *
+ * Tracepoint definitions for mshv driver.
+ */
+
+#define CREATE_TRACE_POINTS
+#include "mshv_trace.h"
diff --git a/drivers/hv/mshv_trace.h b/drivers/hv/mshv_trace.h
new file mode 100644
index 000000000000..ba3b3f575983
--- /dev/null
+++ b/drivers/hv/mshv_trace.h
@@ -0,0 +1,515 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2026, Microsoft Corporation.
+ *
+ * Tracepoint declarations for mshv driver.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM mshv
+
+#if !defined(__MSHV_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _MSHV_TRACE_H_
+
+#include <linux/tracepoint.h>
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH ../../drivers/hv
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE mshv_trace
+
+TRACE_EVENT(mshv_create_partition,
+ TP_PROTO(u64 partition_id, int vm_fd),
+ TP_ARGS(partition_id, vm_fd),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(int, vm_fd)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vm_fd = vm_fd;
+ ),
+ TP_printk("partition_id=%llu vm_fd=%d",
+ __entry->partition_id,
+ __entry->vm_fd
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_create_partition,
+ TP_PROTO(u64 flags, s64 partition_id),
+ TP_ARGS(flags, partition_id),
+ TP_STRUCT__entry(
+ __field(u64, flags)
+ __field(s64, partition_id)
+ ),
+ TP_fast_assign(
+ __entry->flags = flags;
+ __entry->partition_id = partition_id;
+ ),
+ TP_printk("flags=%#llx partition_id=%lld",
+ __entry->flags,
+ __entry->partition_id
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_initialize_partition,
+ TP_PROTO(u64 partition_id, u64 status),
+ TP_ARGS(partition_id, status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu status=%#llx",
+ __entry->partition_id,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_partition_release,
+ TP_PROTO(u64 partition_id),
+ TP_ARGS(partition_id),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ ),
+ TP_printk("partition_id=%llu",
+ __entry->partition_id
+ )
+);
+
+TRACE_EVENT(mshv_destroy_partition,
+ TP_PROTO(u64 partition_id),
+ TP_ARGS(partition_id),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ ),
+ TP_printk("partition_id=%llu",
+ __entry->partition_id
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_finalize_partition,
+ TP_PROTO(u64 partition_id, u64 status),
+ TP_ARGS(partition_id, status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu status=%#llx ",
+ __entry->partition_id,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_withdraw_memory,
+ TP_PROTO(u64 partition_id, u64 withdrawn, u64 status),
+ TP_ARGS(partition_id, withdrawn, status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, withdrawn)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->withdrawn = withdrawn;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu withdrawn=%llu status=%#llx",
+ __entry->partition_id,
+ __entry->withdrawn,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_delete_partition,
+ TP_PROTO(u64 partition_id, u64 status),
+ TP_ARGS(partition_id, status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu status=%#llx",
+ __entry->partition_id,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_create_vp,
+ TP_PROTO(u64 partition_id, u32 vp_index, long vp_fd),
+ TP_ARGS(partition_id, vp_index, vp_fd),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(long, vp_fd)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->vp_fd = vp_fd;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u vp_fd=%ld",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->vp_fd
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_map_vp_state_page,
+ TP_PROTO(u64 partition_id, u32 vp_index, u32 page_type, u64 status),
+ TP_ARGS(partition_id, vp_index, page_type, status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(u32, page_type)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->page_type = page_type;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u page_type=%u status=%#llx",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->page_type,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_drain_vp_signals,
+ TP_PROTO(u64 partition_id, u32 vp_index),
+ TP_ARGS(partition_id, vp_index),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u",
+ __entry->partition_id,
+ __entry->vp_index
+ )
+);
+
+TRACE_EVENT(mshv_disable_vp_dispatch,
+ TP_PROTO(u64 partition_id, u32 vp_index, int ret),
+ TP_ARGS(partition_id, vp_index, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(int, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u ret=%d",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_vp_release,
+ TP_PROTO(u64 partition_id, u32 vp_index),
+ TP_ARGS(partition_id, vp_index),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u",
+ __entry->partition_id,
+ __entry->vp_index
+ )
+);
+
+TRACE_EVENT(mshv_run_vp_entry,
+ TP_PROTO(u64 partition_id, u32 vp_index),
+ TP_ARGS(partition_id, vp_index),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u",
+ __entry->partition_id,
+ __entry->vp_index
+ )
+);
+
+TRACE_EVENT(mshv_run_vp_exit,
+ TP_PROTO(u64 partition_id, u32 vp_index, u64 hv_message_type, long ret),
+ TP_ARGS(partition_id, vp_index, hv_message_type, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(u64, hv_message_type)
+ __field(long, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->hv_message_type = hv_message_type;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u hv_message_type=%#llx ret=%ld",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->hv_message_type,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_vp_clear_explicit_suspend,
+ TP_PROTO(u64 partition_id, u32 vp_index, int ret),
+ TP_ARGS(partition_id, vp_index, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(int, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u ret=%d",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_xfer_to_guest_mode_work,
+ TP_PROTO(u64 partition_id, u32 vp_index, unsigned long thread_info_flag, long ret),
+ TP_ARGS(partition_id, vp_index, thread_info_flag, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(unsigned long, thread_info_flag)
+ __field(long, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->thread_info_flag = thread_info_flag;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u thread_info_flag=%#lx ret=%ld",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->thread_info_flag,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_hvcall_dispatch_vp,
+ TP_PROTO(u64 partition_id, u32 vp_index, u32 flags,
+ u32 dispatch_state, u32 dispatch_event, u64 irq_vectors, u64 status),
+ TP_ARGS(partition_id, vp_index, flags, dispatch_state, dispatch_event, irq_vectors,
+ status),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(u32, flags)
+ __field(u32, dispatch_state)
+ __field(u32, dispatch_event)
+ __field(u64, irq_vectors)
+ __field(u64, status)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->flags = flags;
+ __entry->dispatch_state = dispatch_state;
+ __entry->dispatch_event = dispatch_event;
+ __entry->irq_vectors = irq_vectors;
+ __entry->status = status;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u flags=%#x dispatch_state=%#x dispatch_event=%#x irq_vectors=%#016llx status=%#llx",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->flags,
+ __entry->dispatch_state,
+ __entry->dispatch_event,
+ __entry->irq_vectors,
+ __entry->status
+ )
+);
+
+TRACE_EVENT(mshv_update_routing_table,
+ TP_PROTO(u64 partition_id, void *old, void *new, u32 numents),
+ TP_ARGS(partition_id, old, new, numents),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(struct mshv_girq_routing_table *, old)
+ __field(struct mshv_girq_routing_table *, new)
+ __field(u32, numents)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->old = old;
+ __entry->new = new;
+ __entry->numents = numents;
+ ),
+ TP_printk("partition_id=%llu old=%p new=%p numents=%u",
+ __entry->partition_id,
+ __entry->old,
+ __entry->new,
+ __entry->numents
+ )
+);
+
+TRACE_EVENT(mshv_map_user_memory,
+ TP_PROTO(u64 partition_id, u64 start_uaddr, u64 start_gfn, u64 nr_pages, u32 map_flags,
+ long ret),
+ TP_ARGS(partition_id, start_uaddr, start_gfn, nr_pages, map_flags, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, start_uaddr)
+ __field(u64, start_gfn)
+ __field(u64, nr_pages)
+ __field(u32, map_flags)
+ __field(long, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->start_uaddr = start_uaddr;
+ __entry->start_gfn = start_gfn;
+ __entry->nr_pages = nr_pages;
+ __entry->map_flags = map_flags;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu start_uaddr=%#llx start_gfn=%#llx nr_pages=%llu map_flags=%#x ret=%ld",
+ __entry->partition_id,
+ __entry->start_uaddr,
+ __entry->start_gfn,
+ __entry->nr_pages,
+ __entry->map_flags,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_assign_ioeventfd,
+ TP_PROTO(u64 partition_id, u64 addr, u64 length, u64 datamatch, bool wildcard,
+ void *eventfd, int ret),
+ TP_ARGS(partition_id, addr, length, datamatch, wildcard, eventfd, ret),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, addr)
+ __field(u64, length)
+ __field(u64, datamatch)
+ __field(bool, wildcard)
+ __field(struct eventfd_ctx *, eventfd)
+ __field(int, ret)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->addr = addr;
+ __entry->length = length;
+ __entry->datamatch = datamatch;
+ __entry->wildcard = wildcard;
+ __entry->eventfd = eventfd;
+ __entry->ret = ret;
+ ),
+ TP_printk("partition_id=%llu addr=%#016llx length=%#llx datamatch=%#llx wildcard=%d eventfd=%p ret=%d",
+ __entry->partition_id,
+ __entry->addr,
+ __entry->length,
+ __entry->datamatch,
+ __entry->wildcard,
+ __entry->eventfd,
+ __entry->ret
+ )
+);
+
+TRACE_EVENT(mshv_deassign_ioeventfd,
+ TP_PROTO(u64 partition_id, u64 addr, u64 length, u64 datamatch, bool wildcard,
+ void *eventfd),
+ TP_ARGS(partition_id, addr, length, datamatch, wildcard, eventfd),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u64, addr)
+ __field(u64, length)
+ __field(u64, datamatch)
+ __field(bool, wildcard)
+ __field(struct eventfd_ctx *, eventfd)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->addr = addr;
+ __entry->length = length;
+ __entry->datamatch = datamatch;
+ __entry->wildcard = wildcard;
+ __entry->eventfd = eventfd;
+ ),
+ TP_printk("partition_id=%llu addr=%#016llx length=%#llx datamatch=%#llx wildcard=%d eventfd=%p",
+ __entry->partition_id,
+ __entry->addr,
+ __entry->length,
+ __entry->datamatch,
+ __entry->wildcard,
+ __entry->eventfd
+ )
+);
+
+TRACE_EVENT(mshv_vp_wait_for_hv_kick,
+ TP_PROTO(u64 partition_id, u32 vp_index, bool kicked_by_hv, bool blocked,
+ bool irq_pending),
+ TP_ARGS(partition_id, vp_index, kicked_by_hv, blocked, irq_pending),
+ TP_STRUCT__entry(
+ __field(u64, partition_id)
+ __field(u32, vp_index)
+ __field(bool, kicked_by_hv)
+ __field(bool, blocked)
+ __field(bool, irq_pending)
+ ),
+ TP_fast_assign(
+ __entry->partition_id = partition_id;
+ __entry->vp_index = vp_index;
+ __entry->kicked_by_hv = kicked_by_hv;
+ __entry->blocked = blocked;
+ __entry->irq_pending = irq_pending;
+ ),
+ TP_printk("partition_id=%llu vp_index=%u kicked_by_hv=%d blocked=%d irq_pending=%d",
+ __entry->partition_id,
+ __entry->vp_index,
+ __entry->kicked_by_hv,
+ __entry->blocked,
+ __entry->irq_pending
+ )
+);
+
+#endif /* _MSHV_TRACE_H_ */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
^ permalink raw reply related
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: Yuvraj Sakshith @ 2026-03-02 3:33 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: akpm, mst, kys, haiyangz, wei.liu, decui, longli, jasowang,
xuanzhuo, eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt,
surenb, mhocko, jackmanb, hannes, ziy, linux-hyperv,
virtualization, linux-mm, linux-kernel
In-Reply-To: <c618e7a4-42c1-4438-9bc2-9c41450a81a2@kernel.org>
On Fri, Feb 27, 2026 at 09:50:15PM +0100, David Hildenbrand (Arm) wrote:
> On 2/27/26 15:06, Yuvraj Sakshith wrote:
> > PAGE_REPORTING_DEFAULT_ORDER is now set to zero. This means,
> > pages of order zero cannot be reported to a client/driver -- as zero
> > is used to signal a fallback to MAX_PAGE_ORDER.
> >
> > Change PAGE_REPORTING_DEFAULT_ORDER to (-1),
> > so that zero can be used as a valid order with which pages can
> > be reported.
> >
> > Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
> > ---
> > include/linux/page_reporting.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> > index a7e3e30f2..3eb3e26d8 100644
> > --- a/include/linux/page_reporting.h
> > +++ b/include/linux/page_reporting.h
> > @@ -7,7 +7,7 @@
> >
> > /* This value should always be a power of 2, see page_reporting_cycle() */
> > #define PAGE_REPORTING_CAPACITY 32
> > -#define PAGE_REPORTING_DEFAULT_ORDER 0
> > +#define PAGE_REPORTING_DEFAULT_ORDER (-1)
>
> No need for the ().
>
> Wondering whether we now also want to do in this patch:
>
>
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index f0042d5743af..d432aadf9d07 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -11,8 +11,7 @@
> #include "page_reporting.h"
> #include "internal.h"
>
> -/* Initialize to an unsupported value */
> -unsigned int page_reporting_order = -1;
> +unsigned int page_reporting_order = PAGE_REPORTING_DEFAULT_ORDER;
>
> static int page_order_update_notify(const char *val, const struct
> kernel_param *kp)
> {
> @@ -369,7 +368,7 @@ int page_reporting_register(struct
> page_reporting_dev_info *prdev)
> * pageblock_order.
> */
>
> - if (page_reporting_order == -1) {
> + if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
>
>
Sure. Now that I think of it, don’t you think the first nested if() will
always be false? and can be compressed down to just one if()?
- if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
- if (prdev->order != PAGE_REPORTING_DEFAULT_ORDER &&
- prdev->order <= MAX_PAGE_ORDER)
- page_reporting_order = prdev->order;
- else
- page_reporting_order = pageblock_order;
- }
+ page_reporting_order = pageblock_order;
+
+ if (prdev->order != PAGE_REPORTING_DEFAULT_ORDER &&
+ prdev->order <= MAX_PAGE_ORDER)
+ page_reporting_order = prdev->order;
Thanks,
Yuvraj
>
> (and wondering whether we should have called it
> PAGE_REPORTING_USE_DEFAULT_ORDER to make it clearer that it is not an
> actual order. Leaving that up to you :) )
>
> --
> Cheers,
>
> David
^ permalink raw reply
* RE: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: Michael Kelley @ 2026-03-02 5:25 UTC (permalink / raw)
To: Yuvraj Sakshith, David Hildenbrand (Arm)
Cc: akpm@linux-foundation.org, mst@redhat.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <aaUE7M9QkfnYh12e@hu-ysakshit-lv.qualcomm.com>
From: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com> Sent: Sunday, March 1, 2026 7:33 PM
>
> On Fri, Feb 27, 2026 at 09:50:15PM +0100, David Hildenbrand (Arm) wrote:
> > On 2/27/26 15:06, Yuvraj Sakshith wrote:
> > > PAGE_REPORTING_DEFAULT_ORDER is now set to zero. This means,
> > > pages of order zero cannot be reported to a client/driver -- as zero
> > > is used to signal a fallback to MAX_PAGE_ORDER.
> > >
> > > Change PAGE_REPORTING_DEFAULT_ORDER to (-1),
> > > so that zero can be used as a valid order with which pages can
> > > be reported.
> > >
> > > Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
> > > ---
> > > include/linux/page_reporting.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> > > index a7e3e30f2..3eb3e26d8 100644
> > > --- a/include/linux/page_reporting.h
> > > +++ b/include/linux/page_reporting.h
> > > @@ -7,7 +7,7 @@
> > >
> > > /* This value should always be a power of 2, see page_reporting_cycle() */
> > > #define PAGE_REPORTING_CAPACITY 32
> > > -#define PAGE_REPORTING_DEFAULT_ORDER 0
> > > +#define PAGE_REPORTING_DEFAULT_ORDER (-1)
> >
> > No need for the ().
> >
> > Wondering whether we now also want to do in this patch:
> >
> >
> > diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> > index f0042d5743af..d432aadf9d07 100644
> > --- a/mm/page_reporting.c
> > +++ b/mm/page_reporting.c
> > @@ -11,8 +11,7 @@
> > #include "page_reporting.h"
> > #include "internal.h"
> >
> > -/* Initialize to an unsupported value */
> > -unsigned int page_reporting_order = -1;
> > +unsigned int page_reporting_order = PAGE_REPORTING_DEFAULT_ORDER;
> >
> > static int page_order_update_notify(const char *val, const struct
> > kernel_param *kp)
> > {
> > @@ -369,7 +368,7 @@ int page_reporting_register(struct
> > page_reporting_dev_info *prdev)
> > * pageblock_order.
> > */
> >
> > - if (page_reporting_order == -1) {
> > + if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
> >
> >
>
> Sure. Now that I think of it, don’t you think the first nested if() will
> always be false? and can be compressed down to just one if()?
I don't think what you propose is correct. The purpose of testing
page_reporting_order for -1 is to see if a page reporting order has
been specified on the kernel boot line. If it has been specified, then
the page reporting order specified in the call to page_reporting_register()
[either a specific value or the default] is ignored and the kernel boot
line value prevails. But if page_reporting_order is -1 here, then
no kernel boot line value was specified, and the value passed to
page_reporting_register() should prevail.
With this in mind, substituting PAGE_REPORTING_DEFAULT_ORDER
for the -1 in the test doesn’t exactly make sense to me. The -1 in the
test doesn't have quite the same meaning as the -1 for
PAGE_REPORTING_DEFAULT_ORDER. You could even use -2 for
the initial value of page_reporting_order, and here in the test, in
order to make that distinction obvious. Or use a separate symbolic
name like PAGE_REPORTING_ORDER_NOT_SET.
Michael Kelley
>
> - if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
> - if (prdev->order != PAGE_REPORTING_DEFAULT_ORDER &&
> - prdev->order <= MAX_PAGE_ORDER)
> - page_reporting_order = prdev->order;
> - else
> - page_reporting_order = pageblock_order;
> - }
> + page_reporting_order = pageblock_order;
> +
> + if (prdev->order != PAGE_REPORTING_DEFAULT_ORDER &&
> + prdev->order <= MAX_PAGE_ORDER)
> + page_reporting_order = prdev->order;
>
> Thanks,
> Yuvraj
>
> >
> > (and wondering whether we should have called it
> > PAGE_REPORTING_USE_DEFAULT_ORDER to make it clearer that it is not an
> > actual order. Leaving that up to you :) )
> >
> > --
> > Cheers,
> >
> > David
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: David Hildenbrand (Arm) @ 2026-03-02 7:42 UTC (permalink / raw)
To: Michael Kelley, Yuvraj Sakshith
Cc: akpm@linux-foundation.org, mst@redhat.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB4157D37B3D254251F0135B04D47EA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 3/2/26 06:25, Michael Kelley wrote:
> From: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com> Sent: Sunday, March 1, 2026 7:33 PM
>>
>> On Fri, Feb 27, 2026 at 09:50:15PM +0100, David Hildenbrand (Arm) wrote:
>>>
>>> No need for the ().
>>>
>>> Wondering whether we now also want to do in this patch:
>>>
>>>
>>> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
>>> index f0042d5743af..d432aadf9d07 100644
>>> --- a/mm/page_reporting.c
>>> +++ b/mm/page_reporting.c
>>> @@ -11,8 +11,7 @@
>>> #include "page_reporting.h"
>>> #include "internal.h"
>>>
>>> -/* Initialize to an unsupported value */
>>> -unsigned int page_reporting_order = -1;
>>> +unsigned int page_reporting_order = PAGE_REPORTING_DEFAULT_ORDER;
>>>
>>> static int page_order_update_notify(const char *val, const struct
>>> kernel_param *kp)
>>> {
>>> @@ -369,7 +368,7 @@ int page_reporting_register(struct
>>> page_reporting_dev_info *prdev)
>>> * pageblock_order.
>>> */
>>>
>>> - if (page_reporting_order == -1) {
>>> + if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
>>>
>>>
>>
>> Sure. Now that I think of it, don’t you think the first nested if() will
>> always be false? and can be compressed down to just one if()?
>
> I don't think what you propose is correct. The purpose of testing
> page_reporting_order for -1 is to see if a page reporting order has
> been specified on the kernel boot line. If it has been specified, then
> the page reporting order specified in the call to page_reporting_register()
> [either a specific value or the default] is ignored and the kernel boot
> line value prevails. But if page_reporting_order is -1 here, then
> no kernel boot line value was specified, and the value passed to
> page_reporting_register() should prevail.
>
> With this in mind, substituting PAGE_REPORTING_DEFAULT_ORDER
> for the -1 in the test doesn’t exactly make sense to me. The -1 in the
> test doesn't have quite the same meaning as the -1 for
> PAGE_REPORTING_DEFAULT_ORDER. You could even use -2 for
> the initial value of page_reporting_order, and here in the test, in
> order to make that distinction obvious. Or use a separate symbolic
> name like PAGE_REPORTING_ORDER_NOT_SET.
I don't really see a difference between "PAGE_REPORTING_DEFAULT_ORDER"
and "PAGE_REPORTING_ORDER_NOT_SET" that would warrant a split and adding
confusion for the page-reporting drivers.
In both cases, we want "no special requirement, just use the default".
Maybe we can use a better name to express that.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: Yuvraj Sakshith @ 2026-03-02 8:00 UTC (permalink / raw)
To: David Hildenbrand (Arm), Michael Kelley
Cc: Michael Kelley, akpm@linux-foundation.org, mst@redhat.com,
kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <b1390b24-eaef-40e0-a16b-77c27decb77e@kernel.org>
On Mon, Mar 02, 2026 at 08:42:57AM +0100, David Hildenbrand (Arm) wrote:
> On 3/2/26 06:25, Michael Kelley wrote:
> > From: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com> Sent: Sunday, March 1, 2026 7:33 PM
> >>
> >> On Fri, Feb 27, 2026 at 09:50:15PM +0100, David Hildenbrand (Arm) wrote:
> >>>
> >>> No need for the ().
> >>>
> >>> Wondering whether we now also want to do in this patch:
> >>>
> >>>
> >>> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> >>> index f0042d5743af..d432aadf9d07 100644
> >>> --- a/mm/page_reporting.c
> >>> +++ b/mm/page_reporting.c
> >>> @@ -11,8 +11,7 @@
> >>> #include "page_reporting.h"
> >>> #include "internal.h"
> >>>
> >>> -/* Initialize to an unsupported value */
> >>> -unsigned int page_reporting_order = -1;
> >>> +unsigned int page_reporting_order = PAGE_REPORTING_DEFAULT_ORDER;
> >>>
> >>> static int page_order_update_notify(const char *val, const struct
> >>> kernel_param *kp)
> >>> {
> >>> @@ -369,7 +368,7 @@ int page_reporting_register(struct
> >>> page_reporting_dev_info *prdev)
> >>> * pageblock_order.
> >>> */
> >>>
> >>> - if (page_reporting_order == -1) {
> >>> + if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
> >>>
> >>>
> >>
> >> Sure. Now that I think of it, don’t you think the first nested if() will
> >> always be false? and can be compressed down to just one if()?
> >
> > I don't think what you propose is correct. The purpose of testing
> > page_reporting_order for -1 is to see if a page reporting order has
> > been specified on the kernel boot line. If it has been specified, then
> > the page reporting order specified in the call to page_reporting_register()
> > [either a specific value or the default] is ignored and the kernel boot
> > line value prevails. But if page_reporting_order is -1 here, then
> > no kernel boot line value was specified, and the value passed to
> > page_reporting_register() should prevail.
> >
> > With this in mind, substituting PAGE_REPORTING_DEFAULT_ORDER
> > for the -1 in the test doesn’t exactly make sense to me. The -1 in the
> > test doesn't have quite the same meaning as the -1 for
> > PAGE_REPORTING_DEFAULT_ORDER. You could even use -2 for
> > the initial value of page_reporting_order, and here in the test, in
> > order to make that distinction obvious. Or use a separate symbolic
> > name like PAGE_REPORTING_ORDER_NOT_SET.
>
Option 1:
if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
&& prdev->order <= MAX_PAGE_ORDER) {
page_reporting_order = prdev->order;
} else {
page_reporting_order = pageblock_order;
}
}
Option 2:
if (page_reporting_order == PAGE_REPORTING_ORDER_NOT_SET) {
if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
&& prdev->order <= MAX_PAGE_ORDER) {
page_reporting_order = prdev->order;
} else {
page_reporting_order = pageblock_order;
}
}
> I don't really see a difference between "PAGE_REPORTING_DEFAULT_ORDER"
> and "PAGE_REPORTING_ORDER_NOT_SET" that would warrant a split and adding
> confusion for the page-reporting drivers.
>
> In both cases, we want "no special requirement, just use the default".
> Maybe we can use a better name to express that.
Agreed.
If we were to read this code without context, wouldn't it be confusing as to
why PAGE_REPORTING_DEFAULT_ORDER is being checked in the first place?
Option 1 checks if page_reporting_order is equal to PAGE_REPORTING_DEFAULT_ORDER
and then immediately checks if its not equal to it. Which is a bit confusing..
And moreover, page_reporting_order can be set by two people. The commandline and
the driver itself. So PAGE_REPORTING_ORDER_NOT_SET can indicate if its set by cmdline
and PAGE_REPORTING_DEFAULT_ORDER can be used by drivers exclusively to "tell" page-reporting
to select the default value for us.
I think what Michael is pointing out is the prevalence of cmdline option over the driver's
request.
This is not obvious to the reader if we choose to only have one flag IMO :)
Thanks,
Yuvraj
> --
> Cheers,
>
> David
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: David Hildenbrand (Arm) @ 2026-03-02 8:09 UTC (permalink / raw)
To: Yuvraj Sakshith, Michael Kelley
Cc: akpm@linux-foundation.org, mst@redhat.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <aaVDiwEPl5t2UPX4@hu-ysakshit-lv.qualcomm.com>
On 3/2/26 09:00, Yuvraj Sakshith wrote:
> On Mon, Mar 02, 2026 at 08:42:57AM +0100, David Hildenbrand (Arm) wrote:
>> On 3/2/26 06:25, Michael Kelley wrote:
>>> From: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com> Sent: Sunday, March 1, 2026 7:33 PM
>>>
>>> I don't think what you propose is correct. The purpose of testing
>>> page_reporting_order for -1 is to see if a page reporting order has
>>> been specified on the kernel boot line. If it has been specified, then
>>> the page reporting order specified in the call to page_reporting_register()
>>> [either a specific value or the default] is ignored and the kernel boot
>>> line value prevails. But if page_reporting_order is -1 here, then
>>> no kernel boot line value was specified, and the value passed to
>>> page_reporting_register() should prevail.
>>>
>>> With this in mind, substituting PAGE_REPORTING_DEFAULT_ORDER
>>> for the -1 in the test doesn’t exactly make sense to me. The -1 in the
>>> test doesn't have quite the same meaning as the -1 for
>>> PAGE_REPORTING_DEFAULT_ORDER. You could even use -2 for
>>> the initial value of page_reporting_order, and here in the test, in
>>> order to make that distinction obvious. Or use a separate symbolic
>>> name like PAGE_REPORTING_ORDER_NOT_SET.
>>
> Option 1:
>
> if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
> if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
> && prdev->order <= MAX_PAGE_ORDER) {
> page_reporting_order = prdev->order;
> } else {
> page_reporting_order = pageblock_order;
> }
> }
>
> Option 2:
>
> if (page_reporting_order == PAGE_REPORTING_ORDER_NOT_SET) {
> if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
> && prdev->order <= MAX_PAGE_ORDER) {
> page_reporting_order = prdev->order;
> } else {
> page_reporting_order = pageblock_order;
> }
> }
>
>
>> I don't really see a difference between "PAGE_REPORTING_DEFAULT_ORDER"
>> and "PAGE_REPORTING_ORDER_NOT_SET" that would warrant a split and adding
>> confusion for the page-reporting drivers.
>>
>> In both cases, we want "no special requirement, just use the default".
>> Maybe we can use a better name to express that.
>
> Agreed.
>
> If we were to read this code without context, wouldn't it be confusing as to
> why PAGE_REPORTING_DEFAULT_ORDER is being checked in the first place?
I proposed in one of the last mail that
"PAGE_REPORTING_USE_DEFAULT_ORDER" could be clearer, stating that it's
not really an order just yet. Maybe just using
PAGE_REPORTING_ORDER_UNSET might be clearer.
>
> Option 1 checks if page_reporting_order is equal to PAGE_REPORTING_DEFAULT_ORDER
> and then immediately checks if its not equal to it. Which is a bit confusing..
Because it's wrong? :) We're not supposed to check page_reporting_order
a second time. Assume we
s/PAGE_REPORTING_ORDER/PAGE_REPORTING_ORDER_UNSET/ and actually check
prdev->order:
if (page_reporting_order == PAGE_REPORTING_ORDER_UNSET) {
if (prdev->order != PAGE_REPORTING_ORDER_UNSET &&
prdev->order <= MAX_PAGE_ORDER) {
page_reporting_order = prdev->order;
} else {
page_reporting_order = pageblock_order;
}
}
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: Yuvraj Sakshith @ 2026-03-02 8:54 UTC (permalink / raw)
To: David Hildenbrand (Arm), Michael Kelley
Cc: Michael Kelley, akpm@linux-foundation.org, mst@redhat.com,
kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <a0133403-8ce3-45a4-987f-96fb7421f920@kernel.org>
On Mon, Mar 02, 2026 at 09:09:13AM +0100, David Hildenbrand (Arm) wrote:
> On 3/2/26 09:00, Yuvraj Sakshith wrote:
> > On Mon, Mar 02, 2026 at 08:42:57AM +0100, David Hildenbrand (Arm) wrote:
> >> On 3/2/26 06:25, Michael Kelley wrote:
> >>> From: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com> Sent: Sunday, March 1, 2026 7:33 PM
> >>>
> >>> I don't think what you propose is correct. The purpose of testing
> >>> page_reporting_order for -1 is to see if a page reporting order has
> >>> been specified on the kernel boot line. If it has been specified, then
> >>> the page reporting order specified in the call to page_reporting_register()
> >>> [either a specific value or the default] is ignored and the kernel boot
> >>> line value prevails. But if page_reporting_order is -1 here, then
> >>> no kernel boot line value was specified, and the value passed to
> >>> page_reporting_register() should prevail.
> >>>
> >>> With this in mind, substituting PAGE_REPORTING_DEFAULT_ORDER
> >>> for the -1 in the test doesn’t exactly make sense to me. The -1 in the
> >>> test doesn't have quite the same meaning as the -1 for
> >>> PAGE_REPORTING_DEFAULT_ORDER. You could even use -2 for
> >>> the initial value of page_reporting_order, and here in the test, in
> >>> order to make that distinction obvious. Or use a separate symbolic
> >>> name like PAGE_REPORTING_ORDER_NOT_SET.
> >>
> > Option 1:
> >
> > if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
> > if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
> > && prdev->order <= MAX_PAGE_ORDER) {
> > page_reporting_order = prdev->order;
> > } else {
> > page_reporting_order = pageblock_order;
> > }
> > }
> >
> > Option 2:
> >
> > if (page_reporting_order == PAGE_REPORTING_ORDER_NOT_SET) {
> > if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
> > && prdev->order <= MAX_PAGE_ORDER) {
> > page_reporting_order = prdev->order;
> > } else {
> > page_reporting_order = pageblock_order;
> > }
> > }
> >
> >
> >> I don't really see a difference between "PAGE_REPORTING_DEFAULT_ORDER"
> >> and "PAGE_REPORTING_ORDER_NOT_SET" that would warrant a split and adding
> >> confusion for the page-reporting drivers.
> >>
> >> In both cases, we want "no special requirement, just use the default".
> >> Maybe we can use a better name to express that.
> >
> > Agreed.
> >
> > If we were to read this code without context, wouldn't it be confusing as to
> > why PAGE_REPORTING_DEFAULT_ORDER is being checked in the first place?
>
> I proposed in one of the last mail that
> "PAGE_REPORTING_USE_DEFAULT_ORDER" could be clearer, stating that it's
> not really an order just yet. Maybe just using
> PAGE_REPORTING_ORDER_UNSET might be clearer.
>
Ok
> >
> > Option 1 checks if page_reporting_order is equal to PAGE_REPORTING_DEFAULT_ORDER
> > and then immediately checks if its not equal to it. Which is a bit confusing..
>
>
> Because it's wrong? :) We're not supposed to check page_reporting_order
> a second time. Assume we
> s/PAGE_REPORTING_ORDER/PAGE_REPORTING_ORDER_UNSET/ and actually check
> prdev->order:
Oops, typo :) I meant prdev->order.
>
> if (page_reporting_order == PAGE_REPORTING_ORDER_UNSET) {
> if (prdev->order != PAGE_REPORTING_ORDER_UNSET &&
> prdev->order <= MAX_PAGE_ORDER) {
> page_reporting_order = prdev->order;
> } else {
> page_reporting_order = pageblock_order;
> }
> }
>
Great. Much more clearer on page_reporting.c 's end.
Don't you think on the driver's end:
prdev->order = PAGE_REPORTING_USE_DEFAULT; looks clearer? As compared to:
prdev->order = PAGE_REPORTING_ORDER_UNSET; ?
I'm thinking, why would a driver worry about page_reporting_order being set/unset?
But yes, too many flags...
Thanks,
Yuvraj
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: David Hildenbrand (Arm) @ 2026-03-02 9:18 UTC (permalink / raw)
To: Yuvraj Sakshith, Michael Kelley
Cc: akpm@linux-foundation.org, mst@redhat.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <aaVQXbllLVBLZCwQ@hu-ysakshit-lv.qualcomm.com>
On 3/2/26 09:54, Yuvraj Sakshith wrote:
> On Mon, Mar 02, 2026 at 09:09:13AM +0100, David Hildenbrand (Arm) wrote:
>> On 3/2/26 09:00, Yuvraj Sakshith wrote:
>>> Option 1:
>>>
>>> if (page_reporting_order == PAGE_REPORTING_DEFAULT_ORDER) {
>>> if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
>>> && prdev->order <= MAX_PAGE_ORDER) {
>>> page_reporting_order = prdev->order;
>>> } else {
>>> page_reporting_order = pageblock_order;
>>> }
>>> }
>>>
>>> Option 2:
>>>
>>> if (page_reporting_order == PAGE_REPORTING_ORDER_NOT_SET) {
>>> if (page_reporting_order != PAGE_REPORTING_DEFAULT_ORDER
>>> && prdev->order <= MAX_PAGE_ORDER) {
>>> page_reporting_order = prdev->order;
>>> } else {
>>> page_reporting_order = pageblock_order;
>>> }
>>> }
>>>
>>>
>>>
>>> Agreed.
>>>
>>> If we were to read this code without context, wouldn't it be confusing as to
>>> why PAGE_REPORTING_DEFAULT_ORDER is being checked in the first place?
>>
>> I proposed in one of the last mail that
>> "PAGE_REPORTING_USE_DEFAULT_ORDER" could be clearer, stating that it's
>> not really an order just yet. Maybe just using
>> PAGE_REPORTING_ORDER_UNSET might be clearer.
>>
> Ok
>>>
>>> Option 1 checks if page_reporting_order is equal to PAGE_REPORTING_DEFAULT_ORDER
>>> and then immediately checks if its not equal to it. Which is a bit confusing..
>>
>>
>> Because it's wrong? :) We're not supposed to check page_reporting_order
>> a second time. Assume we
>> s/PAGE_REPORTING_ORDER/PAGE_REPORTING_ORDER_UNSET/ and actually check
>> prdev->order:
> Oops, typo :) I meant prdev->order.
>>
>> if (page_reporting_order == PAGE_REPORTING_ORDER_UNSET) {
>> if (prdev->order != PAGE_REPORTING_ORDER_UNSET &&
>> prdev->order <= MAX_PAGE_ORDER) {
>> page_reporting_order = prdev->order;
>> } else {
>> page_reporting_order = pageblock_order;
>> }
>> }
>>
> Great. Much more clearer on page_reporting.c 's end.
>
> Don't you think on the driver's end:
>
> prdev->order = PAGE_REPORTING_USE_DEFAULT; looks clearer? As compared to:
> prdev->order = PAGE_REPORTING_ORDER_UNSET; ?
>
> I'm thinking, why would a driver worry about page_reporting_order being set/unset?
Maybe PAGE_REPORTING_ORDER_UNSPECIFIED ?
In any case, we should use a single flag for this. Everything else will
be confusing once drivers could use only one of them.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v1 4/4] page_reporting: change PAGE_REPORTING_DEFAULT_ORDER to -1
From: Yuvraj Sakshith @ 2026-03-02 9:50 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Michael Kelley, akpm@linux-foundation.org, mst@redhat.com,
kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, longli@microsoft.com, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
lorenzo.stoakes@oracle.com, Liam.Howlett@oracle.com,
vbabka@suse.cz, rppt@kernel.org, surenb@google.com,
mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org,
ziy@nvidia.com, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
In-Reply-To: <571547b0-007a-4cf9-be1d-95a0ef871cf8@kernel.org>
On Mon, Mar 02, 2026 at 10:18:23AM +0100, David Hildenbrand (Arm) wrote:
> > Great. Much more clearer on page_reporting.c 's end.
> >
> > Don't you think on the driver's end:
> >
> > prdev->order = PAGE_REPORTING_USE_DEFAULT; looks clearer? As compared to:
> > prdev->order = PAGE_REPORTING_ORDER_UNSET; ?
> >
> > I'm thinking, why would a driver worry about page_reporting_order being set/unset?
>
> Maybe PAGE_REPORTING_ORDER_UNSPECIFIED ?
>
> In any case, we should use a single flag for this. Everything else will
> be confusing once drivers could use only one of them.
>
> --
> Cheers,
>
> David
Sounds good. Thanks for the suggestion.
Thanks,
Yuvraj
^ permalink raw reply
* [PATCH v2 0/4] Allow order zero pages in page reporting
From: Yuvraj Sakshith @ 2026-03-02 11:17 UTC (permalink / raw)
To: akpm, mst, david, jasowang, kys, haiyangz, wei.liu, decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
Today, page reporting sets page_reporting_order in two ways:
(1) page_reporting.page_reporting_order cmdline parameter
(2) Driver can pass order while registering itself.
In both cases, order zero is ignored by free page reporting
because it is used to set page_reporting_order to a default
value, like MAX_PAGE_ORDER.
In some cases we might want page_reporting_order to be zero.
For instance, when virtio-balloon runs inside a guest with
tiny memory (say, 16MB), it might not be able to find a order 1 page
(or in the worst case order MAX_PAGE_ORDER page) after some uptime.
Page reporting should be able to return order zero pages back for
optimal memory relinquishment.
This patch changes the default fallback value from '0' to '-1' in
all possible clients of free page reporting (hv_balloon and
virtio-balloon) together with allowing '0' as a valid order in
page_reporting_register().
Changes in v1:
- Introduce PAGE_REPORTING_DEFAULT_ORDER macro (initially set to 0).
- Make use of new macro in drivers (hv_balloon and virtio-balloon)
working with page reporting.
- Change PAGE_REPORTING_DEFAULT_ORDER to -1 as zero is a valid
page order that can be requested.
Changes in v2:
- Better naming. Replace PAGE_REPORTING_DEFAULT_ORDER with
PAGE_REPORTING_ORDER_UNSPECIFIED. This takes care of
the situation where page reporting order is not specified
in the commandline.
- Minor commit message changes.
Yuvraj Sakshith (4):
mm/page_reporting: add PAGE_REPORTING_ORDER_UNSPECIFIED
virtio_balloon: set unspecified page reporting order
hv_balloon: set unspecified page reporting order
mm/page_reporting: change PAGE_REPORTING_ORDER_UNSPECIFIED to -1
drivers/hv/hv_balloon.c | 2 +-
drivers/virtio/virtio_balloon.c | 2 ++
include/linux/page_reporting.h | 1 +
mm/page_reporting.c | 7 ++++---
4 files changed, 8 insertions(+), 4 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v2 1/4] mm/page_reporting: add PAGE_REPORTING_ORDER_UNSPECIFIED
From: Yuvraj Sakshith @ 2026-03-02 11:17 UTC (permalink / raw)
To: akpm, mst, david, jasowang, kys, haiyangz, wei.liu, decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-1-yuvraj.sakshith@oss.qualcomm.com>
Drivers can pass order of pages to be reported while
registering itself. Today, this is a magic number, 0.
Label this with PAGE_REPORTING_ORDER_UNSPECIFIED and
check for it when the driver is being registered.
This macro will be used in relevant drivers next.
Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
---
include/linux/page_reporting.h | 1 +
mm/page_reporting.c | 14 +++++---------
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
index fe648dfa3..d1886c657 100644
--- a/include/linux/page_reporting.h
+++ b/include/linux/page_reporting.h
@@ -7,6 +7,7 @@
/* This value should always be a power of 2, see page_reporting_cycle() */
#define PAGE_REPORTING_CAPACITY 32
+#define PAGE_REPORTING_ORDER_UNSPECIFIED 0
struct page_reporting_dev_info {
/* function that alters pages to make them "reported" */
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index e4c428e61..51cd88faf 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -12,7 +12,7 @@
#include "internal.h"
/* Initialize to an unsupported value */
-unsigned int page_reporting_order = -1;
+unsigned int page_reporting_order = PAGE_REPORTING_ORDER_UNSPECIFIED;
static int page_order_update_notify(const char *val, const struct kernel_param *kp)
{
@@ -25,12 +25,7 @@ static int page_order_update_notify(const char *val, const struct kernel_param *
static const struct kernel_param_ops page_reporting_param_ops = {
.set = &page_order_update_notify,
- /*
- * For the get op, use param_get_int instead of param_get_uint.
- * This is to make sure that when unset the initialized value of
- * -1 is shown correctly
- */
- .get = ¶m_get_int,
+ .get = ¶m_get_uint,
};
module_param_cb(page_reporting_order, &page_reporting_param_ops,
@@ -369,8 +364,9 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
* pageblock_order.
*/
- if (page_reporting_order == -1) {
- if (prdev->order > 0 && prdev->order <= MAX_PAGE_ORDER)
+ if (page_reporting_order == PAGE_REPORTING_ORDER_UNSPECIFIED) {
+ if (prdev->order != PAGE_REPORTING_ORDER_UNSPECIFIED &&
+ prdev->order <= MAX_PAGE_ORDER)
page_reporting_order = prdev->order;
else
page_reporting_order = pageblock_order;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/4] virtio_balloon: set unspecified page reporting order
From: Yuvraj Sakshith @ 2026-03-02 11:17 UTC (permalink / raw)
To: akpm, mst, david, jasowang, kys, haiyangz, wei.liu, decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-1-yuvraj.sakshith@oss.qualcomm.com>
virtio_balloon page reporting order is set to MAX_PAGE_ORDER implicitly
as vb->prdev.order is never initialised and is auto-set to zero.
Explicitly mention usage of default page order by making use of
PAGE_REPORTING_ORDER_UNSPECIFIED fallback value.
Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
---
drivers/virtio/virtio_balloon.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 74fe59f5a..2dfe2bcd8 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1044,6 +1044,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
goto out_unregister_oom;
}
+ vb->pr_dev_info.order = PAGE_REPORTING_ORDER_UNSPECIFIED;
+
/*
* The default page reporting order is @pageblock_order, which
* corresponds to 512MB in size on ARM64 when 64KB base page
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/4] hv_balloon: set unspecified page reporting order
From: Yuvraj Sakshith @ 2026-03-02 11:17 UTC (permalink / raw)
To: akpm, mst, david, jasowang, kys, haiyangz, wei.liu, decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-1-yuvraj.sakshith@oss.qualcomm.com>
Explicitly mention page reporting order to be set to
default value using PAGE_REPORTING_ORDER_UNSPECIFIED fallback
value.
Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
---
drivers/hv/hv_balloon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 2b4080e51..09da68101 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1663,7 +1663,7 @@ static void enable_page_reporting(void)
* We let the page_reporting_order parameter decide the order
* in the page_reporting code
*/
- dm_device.pr_dev_info.order = 0;
+ dm_device.pr_dev_info.order = PAGE_REPORTING_ORDER_UNSPECIFIED;
ret = page_reporting_register(&dm_device.pr_dev_info);
if (ret < 0) {
dm_device.pr_dev_info.report = NULL;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 4/4] mm/page_reporting: change PAGE_REPORTING_ORDER_UNSPECIFIED to -1
From: Yuvraj Sakshith @ 2026-03-02 11:17 UTC (permalink / raw)
To: akpm, mst, david, jasowang, kys, haiyangz, wei.liu, decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-1-yuvraj.sakshith@oss.qualcomm.com>
PAGE_REPORTING_ORDER_UNSPECIFIED is now set to zero. This means,
pages of order zero cannot be reported to a client/driver -- as zero
is used to signal a fallback to MAX_PAGE_ORDER.
Change PAGE_REPORTING_ORDER_UNSPECIFIED to (-1),
so that zero can be used as a valid order with which pages can
be reported.
Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
---
include/linux/page_reporting.h | 2 +-
mm/page_reporting.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
index d1886c657..9d4ca5c21 100644
--- a/include/linux/page_reporting.h
+++ b/include/linux/page_reporting.h
@@ -7,7 +7,7 @@
/* This value should always be a power of 2, see page_reporting_cycle() */
#define PAGE_REPORTING_CAPACITY 32
-#define PAGE_REPORTING_ORDER_UNSPECIFIED 0
+#define PAGE_REPORTING_ORDER_UNSPECIFIED -1
struct page_reporting_dev_info {
/* function that alters pages to make them "reported" */
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index 51cd88faf..21c11b75e 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -25,7 +25,12 @@ static int page_order_update_notify(const char *val, const struct kernel_param *
static const struct kernel_param_ops page_reporting_param_ops = {
.set = &page_order_update_notify,
- .get = ¶m_get_uint,
+ /*
+ * For the get op, use param_get_int instead of param_get_uint.
+ * This is to make sure that when unset the initialized value of
+ * -1 is shown correctly
+ */
+ .get = ¶m_get_int,
};
module_param_cb(page_reporting_order, &page_reporting_param_ops,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net-next, v2] net: mana: Trigger VF reset/recovery on health check failure due to HWC timeout
From: Simon Horman @ 2026-03-02 11:27 UTC (permalink / raw)
To: Dipayaan Roy
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, leon, longli, kotaranov, shradhagupta, ssengar,
ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, dipayanroy
In-Reply-To: <aaFShvKnwR5FY8dH@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Fri, Feb 27, 2026 at 12:15:02AM -0800, Dipayaan Roy wrote:
> The GF stats periodic query is used as mechanism to monitor HWC health
> check. If this HWC command times out, it is a strong indication that
> the device/SoC is in a faulty state and requires recovery.
>
> Today, when a timeout is detected, the driver marks
> hwc_timeout_occurred, clears cached stats, and stops rescheduling the
> periodic work. However, the device itself is left in the same failing
> state.
>
> Extend the timeout handling path to trigger the existing MANA VF
> recovery service by queueing a GDMA_EQE_HWC_RESET_REQUEST work item.
> This is expected to initiate the appropriate recovery flow by suspende
> resume first and if it fails then trigger a bus rescan.
>
> This change is intentionally limited to HWC command timeouts and does
> not trigger recovery for errors reported by the SoC as a normal command
> response.
>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> ---
> Changes in v2:
> - Added common helper, proper clearing of gc flags.
Thanks for the update.
Reviewed-by: Simon Horman <horms@kernel.org>
...
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Force full-page RX buffers for 4K page size on specific systems.
From: Simon Horman @ 2026-03-02 14:02 UTC (permalink / raw)
To: Dipayaan Roy
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, leon, longli, kotaranov, shradhagupta, ssengar,
ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, dipayanroy
In-Reply-To: <aaFusIxdbVkUqIpd@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Fri, Feb 27, 2026 at 02:15:12AM -0800, Dipayaan Roy wrote:
> On certain systems configured with 4K PAGE_SIZE, utilizing page_pool
> fragments for RX buffers results in a significant throughput regression.
> Profiling reveals that this regression correlates with high overhead in the
> fragment allocation and reference counting paths on these specific
> platforms, rendering the multi-buffer-per-page strategy counterproductive.
>
> To mitigate this, bypass the page_pool fragment path and force a single RX
> packet per page allocation when all the following conditions are met:
> 1. The system is configured with a 4K PAGE_SIZE.
> 2. A processor-specific quirk is detected via SMBIOS Type 4 data.
>
> This approach restores expected line-rate performance by ensuring
> predictable RX refill behavior on affected hardware.
>
> There is no behavioral change for systems using larger page sizes
> (16K/64K), or platforms where this processor-specific quirk do not
> apply.
>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/4] mm/page_reporting: add PAGE_REPORTING_ORDER_UNSPECIFIED
From: David Hildenbrand (Arm) @ 2026-03-02 14:57 UTC (permalink / raw)
To: Yuvraj Sakshith, akpm, mst, jasowang, kys, haiyangz, wei.liu,
decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-2-yuvraj.sakshith@oss.qualcomm.com>
On 3/2/26 12:17, Yuvraj Sakshith wrote:
> Drivers can pass order of pages to be reported while
> registering itself. Today, this is a magic number, 0.
>
> Label this with PAGE_REPORTING_ORDER_UNSPECIFIED and
> check for it when the driver is being registered.
>
> This macro will be used in relevant drivers next.
>
> Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
> ---
> include/linux/page_reporting.h | 1 +
> mm/page_reporting.c | 14 +++++---------
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> index fe648dfa3..d1886c657 100644
> --- a/include/linux/page_reporting.h
> +++ b/include/linux/page_reporting.h
> @@ -7,6 +7,7 @@
>
> /* This value should always be a power of 2, see page_reporting_cycle() */
> #define PAGE_REPORTING_CAPACITY 32
> +#define PAGE_REPORTING_ORDER_UNSPECIFIED 0
>
> struct page_reporting_dev_info {
> /* function that alters pages to make them "reported" */
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index e4c428e61..51cd88faf 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -12,7 +12,7 @@
> #include "internal.h"
>
> /* Initialize to an unsupported value */
> -unsigned int page_reporting_order = -1;
> +unsigned int page_reporting_order = PAGE_REPORTING_ORDER_UNSPECIFIED;
>
> static int page_order_update_notify(const char *val, const struct kernel_param *kp)
> {
> @@ -25,12 +25,7 @@ static int page_order_update_notify(const char *val, const struct kernel_param *
>
> static const struct kernel_param_ops page_reporting_param_ops = {
> .set = &page_order_update_notify,
> - /*
> - * For the get op, use param_get_int instead of param_get_uint.
> - * This is to make sure that when unset the initialized value of
> - * -1 is shown correctly
> - */
> - .get = ¶m_get_int,
> + .get = ¶m_get_uint,
> };
>
> module_param_cb(page_reporting_order, &page_reporting_param_ops,
> @@ -369,8 +364,9 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
> * pageblock_order.
> */
>
> - if (page_reporting_order == -1) {
> - if (prdev->order > 0 && prdev->order <= MAX_PAGE_ORDER)
> + if (page_reporting_order == PAGE_REPORTING_ORDER_UNSPECIFIED) {
> + if (prdev->order != PAGE_REPORTING_ORDER_UNSPECIFIED &&
> + prdev->order <= MAX_PAGE_ORDER)
> page_reporting_order = prdev->order;
> else
> page_reporting_order = pageblock_order;
I think the change to page_reporting_order (and param_get_int) should
come after patch #4.
Otherwise, you temporarily change the semantics of
page_reporting_param_ops() etc.
So you should perform the page_reporting_order changes either in patch
#4 or in a new patch #5.
Apart from that LGTM.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 2/4] virtio_balloon: set unspecified page reporting order
From: David Hildenbrand (Arm) @ 2026-03-02 15:00 UTC (permalink / raw)
To: Yuvraj Sakshith, akpm, mst, jasowang, kys, haiyangz, wei.liu,
decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-3-yuvraj.sakshith@oss.qualcomm.com>
On 3/2/26 12:17, Yuvraj Sakshith wrote:
> virtio_balloon page reporting order is set to MAX_PAGE_ORDER implicitly
> as vb->prdev.order is never initialised and is auto-set to zero.
>
> Explicitly mention usage of default page order by making use of
> PAGE_REPORTING_ORDER_UNSPECIFIED fallback value.
>
> Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
> ---
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 3/4] hv_balloon: set unspecified page reporting order
From: David Hildenbrand (Arm) @ 2026-03-02 15:00 UTC (permalink / raw)
To: Yuvraj Sakshith, akpm, mst, jasowang, kys, haiyangz, wei.liu,
decui
Cc: linux-mm, virtualization, linux-hyperv, linux-kernel, xuanzhuo,
eperezma, lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb,
mhocko, jackmanb, hannes, ziy
In-Reply-To: <20260302111757.2191056-4-yuvraj.sakshith@oss.qualcomm.com>
On 3/2/26 12:17, Yuvraj Sakshith wrote:
> Explicitly mention page reporting order to be set to
> default value using PAGE_REPORTING_ORDER_UNSPECIFIED fallback
> value.
>
> Signed-off-by: Yuvraj Sakshith <yuvraj.sakshith@oss.qualcomm.com>
> ---
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* RE: [PATCH net-next] net: mana: Force full-page RX buffers for 4K page size on specific systems.
From: Haiyang Zhang @ 2026-03-02 16:38 UTC (permalink / raw)
To: Dipayaan Roy, KY Srinivasan, wei.liu@kernel.org, Dexuan Cui,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, leon@kernel.org, Long Li,
Konstantin Taranov, horms@kernel.org,
shradhagupta@linux.microsoft.com, ssengar@linux.microsoft.com,
ernis@linux.microsoft.com, Shiraz Saleem,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Dipayaan Roy
In-Reply-To: <aaFusIxdbVkUqIpd@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
> -----Original Message-----
> From: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> Sent: Friday, February 27, 2026 5:15 AM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <DECUI@microsoft.com>; andrew+netdev@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; leon@kernel.org;
> Long Li <longli@microsoft.com>; Konstantin Taranov
> <kotaranov@microsoft.com>; horms@kernel.org;
> shradhagupta@linux.microsoft.com; ssengar@linux.microsoft.com;
> ernis@linux.microsoft.com; Shiraz Saleem <shirazsaleem@microsoft.com>;
> linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Dipayaan Roy
> <dipayanroy@microsoft.com>
> Subject: [PATCH net-next] net: mana: Force full-page RX buffers for 4K
> page size on specific systems.
>
> On certain systems configured with 4K PAGE_SIZE, utilizing page_pool
> fragments for RX buffers results in a significant throughput regression.
> Profiling reveals that this regression correlates with high overhead in
> the
> fragment allocation and reference counting paths on these specific
> platforms, rendering the multi-buffer-per-page strategy counterproductive.
>
> To mitigate this, bypass the page_pool fragment path and force a single RX
> packet per page allocation when all the following conditions are met:
> 1. The system is configured with a 4K PAGE_SIZE.
> 2. A processor-specific quirk is detected via SMBIOS Type 4 data.
>
> This approach restores expected line-rate performance by ensuring
> predictable RX refill behavior on affected hardware.
>
> There is no behavioral change for systems using larger page sizes
> (16K/64K), or platforms where this processor-specific quirk do not
> apply.
>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Thanks.
^ permalink raw reply
* [PATCH v3] x86/hyperv: Use __naked attribute to fix stackless C function
From: Ard Biesheuvel @ 2026-03-02 16:45 UTC (permalink / raw)
To: linux-kernel
Cc: x86, Ard Biesheuvel, Andrew Cooper, Mukesh Rathor, Uros Bizjak,
Wei Liu, linux-hyperv
hv_crash_c_entry() is a C function that is entered without a stack,
and this is only allowed for functions that have the __naked attribute,
which informs the compiler that it must not emit the usual prologue and
epilogue or emit any other kind of instrumentation that relies on a
stack frame.
So split up the function, and set the __naked attribute on the initial
part that sets up the stack, GDT, IDT and other pieces that are needed
for ordinary C execution. Given that function calls are not permitted
either, use the existing long return coded in an asm() block to call the
second part of the function, which is an ordinary function that is
permitted to call other functions as usual.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> # asm parts, not hv parts
Reviewed-by: Mukesh Rathor <mrathor@linux.microsoft.com>
Acked-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: linux-hyperv@vger.kernel.org
Fixes: 94212d34618c ("x86/hyperv: Implement hypervisor RAM collection into vmcore")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
v3: make hv_wrmsr() 'asm volatile'
combine SS segment register update with RSP assignment
fix pre-existing bug cr4 -> cr2
update comment gcc -> objtool
v2: apply some asm tweaks suggested by Uros and Andrew
arch/x86/hyperv/hv_crash.c | 82 ++++++++++----------
1 file changed, 43 insertions(+), 39 deletions(-)
diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c
index 92da1b4f2e73..fdb277bf73d8 100644
--- a/arch/x86/hyperv/hv_crash.c
+++ b/arch/x86/hyperv/hv_crash.c
@@ -107,14 +107,12 @@ static void __noreturn hv_panic_timeout_reboot(void)
cpu_relax();
}
-/* This cannot be inlined as it needs stack */
-static noinline __noclone void hv_crash_restore_tss(void)
+static void hv_crash_restore_tss(void)
{
load_TR_desc();
}
-/* This cannot be inlined as it needs stack */
-static noinline void hv_crash_clear_kernpt(void)
+static void hv_crash_clear_kernpt(void)
{
pgd_t *pgd;
p4d_t *p4d;
@@ -125,6 +123,25 @@ static noinline void hv_crash_clear_kernpt(void)
native_p4d_clear(p4d);
}
+
+static void __noreturn hv_crash_handle(void)
+{
+ hv_crash_restore_tss();
+ hv_crash_clear_kernpt();
+
+ /* we are now fully in devirtualized normal kernel mode */
+ __crash_kexec(NULL);
+
+ hv_panic_timeout_reboot();
+}
+
+/*
+ * __naked functions do not permit function calls, not even to __always_inline
+ * functions that only contain asm() blocks themselves. So use a macro instead.
+ */
+#define hv_wrmsr(msr, val) \
+ asm volatile("wrmsr" :: "c"(msr), "a"((u32)val), "d"((u32)(val >> 32)) : "memory")
+
/*
* This is the C entry point from the asm glue code after the disable hypercall.
* We enter here in IA32-e long mode, ie, full 64bit mode running on kernel
@@ -133,51 +150,38 @@ static noinline void hv_crash_clear_kernpt(void)
* available. We restore kernel GDT, and rest of the context, and continue
* to kexec.
*/
-static asmlinkage void __noreturn hv_crash_c_entry(void)
+static void __naked hv_crash_c_entry(void)
{
- struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
-
/* first thing, restore kernel gdt */
- native_load_gdt(&ctxt->gdtr);
+ asm volatile("lgdt %0" : : "m" (hv_crash_ctxt.gdtr));
- asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
- asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
+ asm volatile("movw %0, %%ss\n\t"
+ "movq %1, %%rsp"
+ :: "m"(hv_crash_ctxt.ss), "m"(hv_crash_ctxt.rsp));
- asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
- asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
- asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
- asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
+ asm volatile("movw %0, %%ds" : : "m"(hv_crash_ctxt.ds));
+ asm volatile("movw %0, %%es" : : "m"(hv_crash_ctxt.es));
+ asm volatile("movw %0, %%fs" : : "m"(hv_crash_ctxt.fs));
+ asm volatile("movw %0, %%gs" : : "m"(hv_crash_ctxt.gs));
- native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
- asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
+ hv_wrmsr(MSR_IA32_CR_PAT, hv_crash_ctxt.pat);
+ asm volatile("movq %0, %%cr0" : : "r"(hv_crash_ctxt.cr0));
- asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
- asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
- asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
+ asm volatile("movq %0, %%cr8" : : "r"(hv_crash_ctxt.cr8));
+ asm volatile("movq %0, %%cr4" : : "r"(hv_crash_ctxt.cr4));
+ asm volatile("movq %0, %%cr2" : : "r"(hv_crash_ctxt.cr2));
- native_load_idt(&ctxt->idtr);
- native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
- native_wrmsrq(MSR_EFER, ctxt->efer);
+ asm volatile("lidt %0" : : "m" (hv_crash_ctxt.idtr));
+ hv_wrmsr(MSR_GS_BASE, hv_crash_ctxt.gsbase);
+ hv_wrmsr(MSR_EFER, hv_crash_ctxt.efer);
/* restore the original kernel CS now via far return */
- asm volatile("movzwq %0, %%rax\n\t"
- "pushq %%rax\n\t"
- "pushq $1f\n\t"
- "lretq\n\t"
- "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
-
- /* We are in asmlinkage without stack frame, hence make C function
- * calls which will buy stack frames.
- */
- hv_crash_restore_tss();
- hv_crash_clear_kernpt();
-
- /* we are now fully in devirtualized normal kernel mode */
- __crash_kexec(NULL);
-
- hv_panic_timeout_reboot();
+ asm volatile("pushq %q0\n\t"
+ "pushq %q1\n\t"
+ "lretq"
+ :: "r"(hv_crash_ctxt.cs), "r"(hv_crash_handle));
}
-/* Tell gcc we are using lretq long jump in the above function intentionally */
+/* Tell objtool we are using lretq long jump in the above function intentionally */
STACK_FRAME_NON_STANDARD(hv_crash_c_entry);
static void hv_mark_tss_not_busy(void)
--
2.51.0
^ permalink raw reply related
* Re: [PATCH net-next v4] net: mana: Add MAC address to vPort logs and clarify error messages
From: Erni Sri Satya Vennela @ 2026-03-02 17:27 UTC (permalink / raw)
To: Simon Horman
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, dipayanroy, shirazsaleem, ssengar,
shradhagupta, gargaditya, linux-hyperv, netdev, linux-kernel
In-Reply-To: <aaRsN8FDf8aH54QB@horms.kernel.org>
> I have reservations about the usefulness of including __func__ and __LINE__
> in debug messages. In a nutshell, it requires the logs to be correlated
> (exactly?) with the source used to build the driver. And at that point
> I think other mechanism - e.g. dynamic trace points - are going to be
> useful if the debug message (without function and line information)
> is insufficient to pinpoint the problem.
>
> This is a general statement, rather than something specifically
> about this code. But nonetheless I'd advise against adding this
> information here.
>
Thankyou Jakub and Simon for the suggestions.
I'll remove both in the next version.
^ 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