* [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements
@ 2018-02-07 17:49 Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 1/4] hv_balloon: fix printk loglevel Vitaly Kuznetsov
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-02-07 17:49 UTC (permalink / raw)
To: devel
Cc: Eduardo Otubo, Stephen Hemminger, Haiyang Zhang, Alex Ng, linux-kernel
I just noticed that this series got lost; the initial submission was on
Nov, 8 but nothing happened after. Resending.
Original description:
While doing routing code review I noticed that commit 6df8d9aaf3af
("Drivers: hv: balloon: Correctly update onlined page count") introduced
an issue with num_pages_onlined accounting on memory offlining. Deeper look
showed that the accounting was always buggy. This is fixed in PATCH3.
PATCHes 1 and 2 are preparatory cleanups, PATCH4 adds a tracepoint to
post_status so it's now possible to see what's being sent to the host and
where the data comes from.
Vitaly Kuznetsov (4):
hv_balloon: fix printk loglevel
hv_balloon: simplify hv_online_page()/hv_page_online_one()
hv_balloon: fix bugs in num_pages_onlined accounting
hv_balloon: trace post_status
drivers/hv/Makefile | 1 +
drivers/hv/hv_balloon.c | 121 +++++++++++++++++++++++++++++-------------
drivers/hv/hv_trace_balloon.h | 48 +++++++++++++++++
3 files changed, 132 insertions(+), 38 deletions(-)
create mode 100644 drivers/hv/hv_trace_balloon.h
--
2.14.3
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH RESEND 1/4] hv_balloon: fix printk loglevel
2018-02-07 17:49 [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements Vitaly Kuznetsov
@ 2018-02-07 17:50 ` Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 2/4] hv_balloon: simplify hv_online_page()/hv_page_online_one() Vitaly Kuznetsov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-02-07 17:50 UTC (permalink / raw)
To: devel
Cc: Eduardo Otubo, Stephen Hemminger, Haiyang Zhang, Alex Ng, linux-kernel
We have a mix of different ideas of which loglevel should be used. Unify
on the following:
- pr_info() for normal operation
- pr_warn() for 'strange' host behavior
- pr_err() for all errors.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/hv_balloon.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index db0e6652d7ef..1aece72da9ba 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -691,7 +691,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
(HA_CHUNK << PAGE_SHIFT));
if (ret) {
- pr_warn("hot_add memory failed error is %d\n", ret);
+ pr_err("hot_add memory failed error is %d\n", ret);
if (ret == -EEXIST) {
/*
* This error indicates that the error
@@ -1014,7 +1014,7 @@ static void hot_add_req(struct work_struct *dummy)
resp.result = 0;
if (!do_hot_add || (resp.page_count == 0))
- pr_info("Memory hot add failed\n");
+ pr_err("Memory hot add failed\n");
dm->state = DM_INITIALIZED;
resp.hdr.trans_id = atomic_inc_return(&trans_id);
@@ -1041,7 +1041,7 @@ static void process_info(struct hv_dynmem_device *dm, struct dm_info_msg *msg)
break;
default:
- pr_info("Received Unknown type: %d\n", info_hdr->type);
+ pr_warn("Received Unknown type: %d\n", info_hdr->type);
}
}
@@ -1290,7 +1290,7 @@ static void balloon_up(struct work_struct *dummy)
/*
* Free up the memory we allocatted.
*/
- pr_info("Balloon response failed\n");
+ pr_err("Balloon response failed\n");
for (i = 0; i < bl_resp->range_count; i++)
free_balloon_pages(&dm_device,
@@ -1421,7 +1421,7 @@ static void cap_resp(struct hv_dynmem_device *dm,
struct dm_capabilities_resp_msg *cap_resp)
{
if (!cap_resp->is_accepted) {
- pr_info("Capabilities not accepted by host\n");
+ pr_err("Capabilities not accepted by host\n");
dm->state = DM_INIT_ERROR;
}
complete(&dm->host_event);
@@ -1508,7 +1508,7 @@ static void balloon_onchannelcallback(void *context)
break;
default:
- pr_err("Unhandled message: type: %d\n", dm_hdr->type);
+ pr_warn("Unhandled message: type: %d\n", dm_hdr->type);
}
}
--
2.14.3
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH RESEND 2/4] hv_balloon: simplify hv_online_page()/hv_page_online_one()
2018-02-07 17:49 [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 1/4] hv_balloon: fix printk loglevel Vitaly Kuznetsov
@ 2018-02-07 17:50 ` Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 3/4] hv_balloon: fix bugs in num_pages_onlined accounting Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 4/4] hv_balloon: trace post_status Vitaly Kuznetsov
3 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-02-07 17:50 UTC (permalink / raw)
To: devel
Cc: Eduardo Otubo, Stephen Hemminger, Haiyang Zhang, Alex Ng, linux-kernel
Instead of doing pfn_to_page() and continuosly casting page to unsigned
long just cache the pfn of the page with page_to_pfn().
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/hv_balloon.c | 27 +++++----------------------
1 file changed, 5 insertions(+), 22 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 1aece72da9ba..5b8e1ad1bcfe 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -612,28 +612,17 @@ static struct notifier_block hv_memory_nb = {
/* Check if the particular page is backed and can be onlined and online it. */
static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
{
- unsigned long cur_start_pgp;
- unsigned long cur_end_pgp;
struct hv_hotadd_gap *gap;
-
- cur_start_pgp = (unsigned long)pfn_to_page(has->covered_start_pfn);
- cur_end_pgp = (unsigned long)pfn_to_page(has->covered_end_pfn);
+ unsigned long pfn = page_to_pfn(pg);
/* The page is not backed. */
- if (((unsigned long)pg < cur_start_pgp) ||
- ((unsigned long)pg >= cur_end_pgp))
+ if ((pfn < has->covered_start_pfn) || (pfn >= has->covered_end_pfn))
return;
/* Check for gaps. */
list_for_each_entry(gap, &has->gap_list, list) {
- cur_start_pgp = (unsigned long)
- pfn_to_page(gap->start_pfn);
- cur_end_pgp = (unsigned long)
- pfn_to_page(gap->end_pfn);
- if (((unsigned long)pg >= cur_start_pgp) &&
- ((unsigned long)pg < cur_end_pgp)) {
+ if ((pfn >= gap->start_pfn) && (pfn < gap->end_pfn))
return;
- }
}
/* This frame is currently backed; online the page. */
@@ -726,19 +715,13 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
static void hv_online_page(struct page *pg)
{
struct hv_hotadd_state *has;
- unsigned long cur_start_pgp;
- unsigned long cur_end_pgp;
unsigned long flags;
+ unsigned long pfn = page_to_pfn(pg);
spin_lock_irqsave(&dm_device.ha_lock, flags);
list_for_each_entry(has, &dm_device.ha_region_list, list) {
- cur_start_pgp = (unsigned long)
- pfn_to_page(has->start_pfn);
- cur_end_pgp = (unsigned long)pfn_to_page(has->end_pfn);
-
/* The page belongs to a different HAS. */
- if (((unsigned long)pg < cur_start_pgp) ||
- ((unsigned long)pg >= cur_end_pgp))
+ if ((pfn < has->start_pfn) || (pfn >= has->end_pfn))
continue;
hv_page_online_one(has, pg);
--
2.14.3
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH RESEND 3/4] hv_balloon: fix bugs in num_pages_onlined accounting
2018-02-07 17:49 [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 1/4] hv_balloon: fix printk loglevel Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 2/4] hv_balloon: simplify hv_online_page()/hv_page_online_one() Vitaly Kuznetsov
@ 2018-02-07 17:50 ` Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 4/4] hv_balloon: trace post_status Vitaly Kuznetsov
3 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-02-07 17:50 UTC (permalink / raw)
To: devel
Cc: Eduardo Otubo, Stephen Hemminger, Haiyang Zhang, Alex Ng, linux-kernel
Our num_pages_onlined accounting is buggy:
1) In case we're offlining a memory block which was present at boot (e.g.
when there was no hotplug at all) we subtract 32k from 0 and as
num_pages_onlined is unsigned get a very big positive number.
2) Commit 6df8d9aaf3af ("Drivers: hv: balloon: Correctly update onlined
page count") made num_pages_onlined counter accurate on onlining but
totally incorrect on offlining for partly populated regions: no matter
how many pages were onlined and what was actually added to
num_pages_onlined counter we always subtract the full region (32k) so
again, num_pages_onlined can wrap around zero. By onlining/offlining
the same partly populated region multiple times we can make the
situation worse.
Solve these issues by doing accurate accounting on offlining: walk HAS
list, check for covered range and gaps.
Fixes: 6df8d9aaf3af ("Drivers: hv: balloon: Correctly update onlined page count")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/hv_balloon.c | 82 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 69 insertions(+), 13 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 5b8e1ad1bcfe..7514a32d0de4 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -576,11 +576,65 @@ static struct hv_dynmem_device dm_device;
static void post_status(struct hv_dynmem_device *dm);
#ifdef CONFIG_MEMORY_HOTPLUG
+static inline bool has_pfn_is_backed(struct hv_hotadd_state *has,
+ unsigned long pfn)
+{
+ struct hv_hotadd_gap *gap;
+
+ /* The page is not backed. */
+ if ((pfn < has->covered_start_pfn) || (pfn >= has->covered_end_pfn))
+ return false;
+
+ /* Check for gaps. */
+ list_for_each_entry(gap, &has->gap_list, list) {
+ if ((pfn >= gap->start_pfn) && (pfn < gap->end_pfn))
+ return false;
+ }
+
+ return true;
+}
+
+static unsigned long hv_page_offline_check(unsigned long start_pfn,
+ unsigned long nr_pages)
+{
+ unsigned long pfn = start_pfn, count = 0;
+ struct hv_hotadd_state *has;
+ bool found;
+
+ while (pfn < start_pfn + nr_pages) {
+ /*
+ * Search for HAS which covers the pfn and when we find one
+ * count how many consequitive PFNs are covered.
+ */
+ found = false;
+ list_for_each_entry(has, &dm_device.ha_region_list, list) {
+ while ((pfn >= has->start_pfn) &&
+ (pfn < has->end_pfn) &&
+ (pfn < start_pfn + nr_pages)) {
+ found = true;
+ if (has_pfn_is_backed(has, pfn))
+ count++;
+ pfn++;
+ }
+ }
+
+ /*
+ * This PFN is not in any HAS (e.g. we're offlining a region
+ * which was present at boot), no need to account for it. Go
+ * to the next one.
+ */
+ if (!found)
+ pfn++;
+ }
+
+ return count;
+}
+
static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
void *v)
{
struct memory_notify *mem = (struct memory_notify *)v;
- unsigned long flags;
+ unsigned long flags, pfn_count;
switch (val) {
case MEM_ONLINE:
@@ -593,7 +647,19 @@ static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
case MEM_OFFLINE:
spin_lock_irqsave(&dm_device.ha_lock, flags);
- dm_device.num_pages_onlined -= mem->nr_pages;
+ pfn_count = hv_page_offline_check(mem->start_pfn,
+ mem->nr_pages);
+ if (pfn_count <= dm_device.num_pages_onlined) {
+ dm_device.num_pages_onlined -= pfn_count;
+ } else {
+ /*
+ * We're offlining more pages than we managed to online.
+ * This is unexpected. In any case don't let
+ * num_pages_onlined wrap around zero.
+ */
+ WARN_ON_ONCE(1);
+ dm_device.num_pages_onlined = 0;
+ }
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
break;
case MEM_GOING_ONLINE:
@@ -612,19 +678,9 @@ static struct notifier_block hv_memory_nb = {
/* Check if the particular page is backed and can be onlined and online it. */
static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
{
- struct hv_hotadd_gap *gap;
- unsigned long pfn = page_to_pfn(pg);
-
- /* The page is not backed. */
- if ((pfn < has->covered_start_pfn) || (pfn >= has->covered_end_pfn))
+ if (!has_pfn_is_backed(has, page_to_pfn(pg)))
return;
- /* Check for gaps. */
- list_for_each_entry(gap, &has->gap_list, list) {
- if ((pfn >= gap->start_pfn) && (pfn < gap->end_pfn))
- return;
- }
-
/* This frame is currently backed; online the page. */
__online_page_set_limits(pg);
__online_page_increment_counters(pg);
--
2.14.3
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH RESEND 4/4] hv_balloon: trace post_status
2018-02-07 17:49 [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements Vitaly Kuznetsov
` (2 preceding siblings ...)
2018-02-07 17:50 ` [PATCH RESEND 3/4] hv_balloon: fix bugs in num_pages_onlined accounting Vitaly Kuznetsov
@ 2018-02-07 17:50 ` Vitaly Kuznetsov
3 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-02-07 17:50 UTC (permalink / raw)
To: devel
Cc: Eduardo Otubo, Stephen Hemminger, Haiyang Zhang, Alex Ng, linux-kernel
Hyper-V balloon driver makes non-trivial calculations to convert Linux's
representation of free/used memory to what Hyper-V host expects to see. Add
a tracepoint to see what's being sent and where the data comes from.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/Makefile | 1 +
drivers/hv/hv_balloon.c | 6 ++++++
drivers/hv/hv_trace_balloon.h | 48 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 drivers/hv/hv_trace_balloon.h
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 14c22786b519..a1eec7177c2d 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
CFLAGS_hv_trace.o = -I$(src)
+CFLAGS_hv_balloon.o = -I$(src)
hv_vmbus-y := vmbus_drv.o \
hv.o connection.o channel.o \
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 7514a32d0de4..b3e9f13f8bc3 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -34,6 +34,9 @@
#include <linux/hyperv.h>
+#define CREATE_TRACE_POINTS
+#include "hv_trace_balloon.h"
+
/*
* We begin with definitions supporting the Dynamic Memory protocol
* with the host.
@@ -1159,6 +1162,9 @@ static void post_status(struct hv_dynmem_device *dm)
dm->num_pages_added - dm->num_pages_onlined : 0) +
compute_balloon_floor();
+ trace_balloon_status(status.num_avail, status.num_committed,
+ vm_memory_committed(), dm->num_pages_ballooned,
+ dm->num_pages_added, dm->num_pages_onlined);
/*
* If our transaction ID is no longer current, just don't
* send the status. This can happen if we were interrupted
diff --git a/drivers/hv/hv_trace_balloon.h b/drivers/hv/hv_trace_balloon.h
new file mode 100644
index 000000000000..93082888aec3
--- /dev/null
+++ b/drivers/hv/hv_trace_balloon.h
@@ -0,0 +1,48 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hyperv
+
+#if !defined(_HV_TRACE_BALLOON_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _HV_TRACE_BALLOON_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(balloon_status,
+ TP_PROTO(u64 available, u64 committed,
+ unsigned long vm_memory_committed,
+ unsigned long pages_ballooned,
+ unsigned long pages_added,
+ unsigned long pages_onlined),
+ TP_ARGS(available, committed, vm_memory_committed,
+ pages_ballooned, pages_added, pages_onlined),
+ TP_STRUCT__entry(
+ __field(u64, available)
+ __field(u64, committed)
+ __field(unsigned long, vm_memory_committed)
+ __field(unsigned long, pages_ballooned)
+ __field(unsigned long, pages_added)
+ __field(unsigned long, pages_onlined)
+ ),
+ TP_fast_assign(
+ __entry->available = available;
+ __entry->committed = committed;
+ __entry->vm_memory_committed = vm_memory_committed;
+ __entry->pages_ballooned = pages_ballooned;
+ __entry->pages_added = pages_added;
+ __entry->pages_onlined = pages_onlined;
+ ),
+ TP_printk("available %lld, committed %lld; vm_memory_committed %ld;"
+ " pages_ballooned %ld, pages_added %ld, pages_onlined %ld",
+ __entry->available, __entry->committed,
+ __entry->vm_memory_committed, __entry->pages_ballooned,
+ __entry->pages_added, __entry->pages_onlined
+ )
+ );
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE hv_trace_balloon
+#endif /* _HV_TRACE_BALLOON_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.14.3
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-07 17:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 17:49 [PATCH RESEND 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 1/4] hv_balloon: fix printk loglevel Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 2/4] hv_balloon: simplify hv_online_page()/hv_page_online_one() Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 3/4] hv_balloon: fix bugs in num_pages_onlined accounting Vitaly Kuznetsov
2018-02-07 17:50 ` [PATCH RESEND 4/4] hv_balloon: trace post_status Vitaly Kuznetsov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.