This patch-set addresses minor issues and bugs in the VMware balloon
driver. Patches 1-5 address bugs that were *not* reported by customers,
but we would like to get them to 4.18 if possible.
Patches 1-4 are cc'd to stable. Patch 5 might be too big to be
considered for stable.
We will send separately later a patch-set with new features to be set on
top of this set.
v1->v2:
* Reordering patches to have bug fixes first (Greg)
* Fixing SPDX tag
* Cc'ing stable when appropriate
Nadav Amit (7):
vmw_balloon: fix inflation of 64-bit GFNs
vmw_balloon: do not use 2MB without batching
vmw_balloon: VMCI_DOORBELL_SET does not check status
vmw_balloon: fix VMCI use when balloon built into kernel
vmw_balloon: remove inflation rate limiting
vmw_balloon: update copyright message
vmw_balloon: update maintainers list
MAINTAINERS | 2 +-
drivers/misc/vmw_balloon.c | 168 ++++++++++++-------------------------
2 files changed, 56 insertions(+), 114 deletions(-)
--
2.17.0
When vmballoon_vmci_init() sets a doorbell using VMCI_DOORBELL_SET, for
some reason it does not consider the status and looks at the result.
However, the hypervisor does not update the result - it updates the
status. This might cause VMCI doorbell not to be enabled, resulting in
degraded performance.
Fixes: 48e3d668b790 ("VMware balloon: Enable notification via VMCI")
Cc: [email protected]
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 60ab83d3d0ef..a7df4c24a28d 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1036,29 +1036,30 @@ static void vmballoon_vmci_cleanup(struct vmballoon *b)
*/
static int vmballoon_vmci_init(struct vmballoon *b)
{
- int error = 0;
+ unsigned long error, dummy;
- if ((b->capabilities & VMW_BALLOON_SIGNALLED_WAKEUP_CMD) != 0) {
- error = vmci_doorbell_create(&b->vmci_doorbell,
- VMCI_FLAG_DELAYED_CB,
- VMCI_PRIVILEGE_FLAG_RESTRICTED,
- vmballoon_doorbell, b);
-
- if (error == VMCI_SUCCESS) {
- VMWARE_BALLOON_CMD(VMCI_DOORBELL_SET,
- b->vmci_doorbell.context,
- b->vmci_doorbell.resource, error);
- STATS_INC(b->stats.doorbell_set);
- }
- }
+ if ((b->capabilities & VMW_BALLOON_SIGNALLED_WAKEUP_CMD) == 0)
+ return 0;
- if (error != 0) {
- vmballoon_vmci_cleanup(b);
+ error = vmci_doorbell_create(&b->vmci_doorbell, VMCI_FLAG_DELAYED_CB,
+ VMCI_PRIVILEGE_FLAG_RESTRICTED,
+ vmballoon_doorbell, b);
- return -EIO;
- }
+ if (error != VMCI_SUCCESS)
+ goto fail;
+
+ error = VMWARE_BALLOON_CMD(VMCI_DOORBELL_SET, b->vmci_doorbell.context,
+ b->vmci_doorbell.resource, dummy);
+
+ STATS_INC(b->stats.doorbell_set);
+
+ if (error != VMW_BALLOON_SUCCESS)
+ goto fail;
return 0;
+fail:
+ vmballoon_vmci_cleanup(b);
+ return -EIO;
}
/*
--
2.17.0
If the hypervisor sets 2MB batching is on, while batching is cleared,
the balloon code breaks. In this case the legacy mechanism is used with
2MB page. The VM would report a 2MB page is ballooned, and the
hypervisor would only take the first 4KB.
While the hypervisor should not report such settings, make the code more
robust by not enabling 2MB support without batching.
Fixes: 365bd7ef7ec8e ("VMware balloon: Support 2m page ballooning.")
Cc: [email protected]
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 28e77ab1e136..60ab83d3d0ef 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -341,7 +341,13 @@ static bool vmballoon_send_start(struct vmballoon *b, unsigned long req_caps)
success = false;
}
- if (b->capabilities & VMW_BALLOON_BATCHED_2M_CMDS)
+ /*
+ * 2MB pages are only supported with batching. If batching is for some
+ * reason disabled, do not use 2MB pages, since otherwise the legacy
+ * mechanism is used with 2MB pages, causing a failure.
+ */
+ if ((b->capabilities & VMW_BALLOON_BATCHED_2M_CMDS) &&
+ (b->capabilities & VMW_BALLOON_BATCHED_CMDS))
b->supported_page_sizes = 2;
else
b->supported_page_sizes = 1;
--
2.17.0
When balloon batching is not supported by the hypervisor, the guest
frame number (GFN) must fit in 32-bit. However, due to a bug, this check
was mistakenly ignored. In practice, when total RAM is greater than
16TB, the balloon does not work currently, making this bug unlikely to
happen.
Fixes: ef0f8f112984 ("VMware balloon: partially inline vmballoon_reserve_page.")
Cc: [email protected]
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index efd733472a35..28e77ab1e136 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -450,7 +450,7 @@ static int vmballoon_send_lock_page(struct vmballoon *b, unsigned long pfn,
pfn32 = (u32)pfn;
if (pfn32 != pfn)
- return -1;
+ return -EINVAL;
STATS_INC(b->stats.lock[false]);
@@ -460,7 +460,7 @@ static int vmballoon_send_lock_page(struct vmballoon *b, unsigned long pfn,
pr_debug("%s - ppn %lx, hv returns %ld\n", __func__, pfn, status);
STATS_INC(b->stats.lock_fail[false]);
- return 1;
+ return -EIO;
}
static int vmballoon_send_batched_lock(struct vmballoon *b,
@@ -597,11 +597,12 @@ static int vmballoon_lock_page(struct vmballoon *b, unsigned int num_pages,
locked = vmballoon_send_lock_page(b, page_to_pfn(page), &hv_status,
target);
- if (locked > 0) {
+ if (locked) {
STATS_INC(b->stats.refused_alloc[false]);
- if (hv_status == VMW_BALLOON_ERROR_RESET ||
- hv_status == VMW_BALLOON_ERROR_PPN_NOTNEEDED) {
+ if (locked == -EIO &&
+ (hv_status == VMW_BALLOON_ERROR_RESET ||
+ hv_status == VMW_BALLOON_ERROR_PPN_NOTNEEDED)) {
vmballoon_free_page(page, false);
return -EIO;
}
@@ -617,7 +618,7 @@ static int vmballoon_lock_page(struct vmballoon *b, unsigned int num_pages,
} else {
vmballoon_free_page(page, false);
}
- return -EIO;
+ return locked;
}
/* track allocated page */
--
2.17.0
Currently, when all modules, including VMCI and VMware balloon are built
into the kernel, the initialization of the balloon happens before the
VMCI is probed. As a result, the balloon fails to initialize the VMCI
doorbell, which it uses to get asynchronous requests for balloon size
changes.
The problem can be seen in the logs, in the form of the following
message:
"vmw_balloon: failed to initialize vmci doorbell"
The driver would work correctly but slightly less efficiently, probing
for requests periodically. This patch changes the balloon to be
initialized using late_initcall() instead of module_init() to address
this issue. It does not address a situation in which VMCI is built as a
module and the balloon is built into the kernel.
Fixes: 48e3d668b790 ("VMware balloon: Enable notification via VMCI")
Cc: [email protected]
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index a7df4c24a28d..e7cfc85f6961 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1297,7 +1297,14 @@ static int __init vmballoon_init(void)
return 0;
}
-module_init(vmballoon_init);
+
+/*
+ * Using late_initcall() instead of module_init() allows the balloon to use the
+ * VMCI doorbell even when the balloon is built into the kernel. Otherwise the
+ * VMCI is probed only after the balloon is initialized. If the balloon is used
+ * as a module, late_initcall() is equivalent to module_init().
+ */
+late_initcall(vmballoon_init);
static void __exit vmballoon_exit(void)
{
--
2.17.0
Since commit 33d268ed0019 ("VMware balloon: Do not limit the amount of
frees and allocations in non-sleep mode."), the allocations are not
increased, and therefore balloon inflation rate limiting is in practice
broken.
While we can restore rate limiting, in practice we see that it can
result in adverse effect, as the hypervisor throttles down the VM if it
does not respond well enough, or alternatively causes it to perform very
poorly as the host swaps out the VM memory. Throttling the VM down can
even have a cascading effect, in which the VM reclaims memory even
slower and consequentially throttled down even further.
We therefore remove all the rate limiting mechanisms, including the slow
allocation cycles, as they are likely to do more harm than good.
Fixes: 33d268ed0019 ("VMware balloon: Do not limit the amount of frees and allocations in non-sleep mode.")
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 92 +++++---------------------------------
1 file changed, 11 insertions(+), 81 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index e7cfc85f6961..400a1ccefc8e 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -54,25 +54,6 @@ MODULE_ALIAS("dmi:*:svnVMware*:*");
MODULE_ALIAS("vmware_vmmemctl");
MODULE_LICENSE("GPL");
-/*
- * Various constants controlling rate of inflaint/deflating balloon,
- * measured in pages.
- */
-
-/*
- * Rates of memory allocaton when guest experiences memory pressure
- * (driver performs sleeping allocations).
- */
-#define VMW_BALLOON_RATE_ALLOC_MIN 512U
-#define VMW_BALLOON_RATE_ALLOC_MAX 2048U
-#define VMW_BALLOON_RATE_ALLOC_INC 16U
-
-/*
- * When guest is under memory pressure, use a reduced page allocation
- * rate for next several cycles.
- */
-#define VMW_BALLOON_SLOW_CYCLES 4
-
/*
* Use __GFP_HIGHMEM to allow pages from HIGHMEM zone. We don't
* allow wait (__GFP_RECLAIM) for NOSLEEP page allocations. Use
@@ -284,12 +265,6 @@ struct vmballoon {
/* reset flag */
bool reset_required;
- /* adjustment rates (pages per second) */
- unsigned int rate_alloc;
-
- /* slowdown page allocations for next few cycles */
- unsigned int slow_allocation_cycles;
-
unsigned long capabilities;
struct vmballoon_batch_page *batch_page;
@@ -797,8 +772,6 @@ static void vmballoon_add_batched_page(struct vmballoon *b, int idx,
*/
static void vmballoon_inflate(struct vmballoon *b)
{
- unsigned rate;
- unsigned int allocations = 0;
unsigned int num_pages = 0;
int error = 0;
gfp_t flags = VMW_PAGE_ALLOC_NOSLEEP;
@@ -825,17 +798,9 @@ static void vmballoon_inflate(struct vmballoon *b)
* Start with no sleep allocation rate which may be higher
* than sleeping allocation rate.
*/
- if (b->slow_allocation_cycles) {
- rate = b->rate_alloc;
- is_2m_pages = false;
- } else {
- rate = UINT_MAX;
- is_2m_pages =
- b->supported_page_sizes == VMW_BALLOON_NUM_PAGE_SIZES;
- }
+ is_2m_pages = b->supported_page_sizes == VMW_BALLOON_NUM_PAGE_SIZES;
- pr_debug("%s - goal: %d, no-sleep rate: %u, sleep rate: %d\n",
- __func__, b->target - b->size, rate, b->rate_alloc);
+ pr_debug("%s - goal: %d", __func__, b->target - b->size);
while (!b->reset_required &&
b->size + num_pages * vmballoon_page_size(is_2m_pages)
@@ -868,31 +833,24 @@ static void vmballoon_inflate(struct vmballoon *b)
if (flags == VMW_PAGE_ALLOC_CANSLEEP) {
/*
* CANSLEEP page allocation failed, so guest
- * is under severe memory pressure. Quickly
- * decrease allocation rate.
+ * is under severe memory pressure. We just log
+ * the event, but do not stop the inflation
+ * due to its negative impact on performance.
*/
- b->rate_alloc = max(b->rate_alloc / 2,
- VMW_BALLOON_RATE_ALLOC_MIN);
STATS_INC(b->stats.sleep_alloc_fail);
break;
}
/*
* NOSLEEP page allocation failed, so the guest is
- * under memory pressure. Let us slow down page
- * allocations for next few cycles so that the guest
- * gets out of memory pressure. Also, if we already
- * allocated b->rate_alloc pages, let's pause,
- * otherwise switch to sleeping allocations.
+ * under memory pressure. Slowing down page alloctions
+ * seems to be reasonable, but doing so might actually
+ * cause the hypervisor to throttle us down, resulting
+ * in degraded performance. We will count on the
+ * scheduler and standard memory management mechanisms
+ * for now.
*/
- b->slow_allocation_cycles = VMW_BALLOON_SLOW_CYCLES;
-
- if (allocations >= b->rate_alloc)
- break;
-
flags = VMW_PAGE_ALLOC_CANSLEEP;
- /* Lower rate for sleeping allocations. */
- rate = b->rate_alloc;
continue;
}
@@ -906,28 +864,11 @@ static void vmballoon_inflate(struct vmballoon *b)
}
cond_resched();
-
- if (allocations >= rate) {
- /* We allocated enough pages, let's take a break. */
- break;
- }
}
if (num_pages > 0)
b->ops->lock(b, num_pages, is_2m_pages, &b->target);
- /*
- * We reached our goal without failures so try increasing
- * allocation rate.
- */
- if (error == 0 && allocations >= b->rate_alloc) {
- unsigned int mult = allocations / b->rate_alloc;
-
- b->rate_alloc =
- min(b->rate_alloc + mult * VMW_BALLOON_RATE_ALLOC_INC,
- VMW_BALLOON_RATE_ALLOC_MAX);
- }
-
vmballoon_release_refused_pages(b, true);
vmballoon_release_refused_pages(b, false);
}
@@ -1122,9 +1063,6 @@ static void vmballoon_work(struct work_struct *work)
if (b->reset_required)
vmballoon_reset(b);
- if (b->slow_allocation_cycles > 0)
- b->slow_allocation_cycles--;
-
if (!b->reset_required && vmballoon_send_get_target(b, &target)) {
/* update target, adjust size */
b->target = target;
@@ -1168,11 +1106,6 @@ static int vmballoon_debug_show(struct seq_file *f, void *offset)
"current: %8d pages\n",
b->target, b->size);
- /* format rate info */
- seq_printf(f,
- "rateSleepAlloc: %8d pages/sec\n",
- b->rate_alloc);
-
seq_printf(f,
"\n"
"timer: %8u\n"
@@ -1279,9 +1212,6 @@ static int __init vmballoon_init(void)
INIT_LIST_HEAD(&balloon.page_sizes[is_2m_pages].refused_pages);
}
- /* initialize rates */
- balloon.rate_alloc = VMW_BALLOON_RATE_ALLOC_MAX;
-
INIT_DELAYED_WORK(&balloon.dwork, vmballoon_work);
error = vmballoon_debugfs_init(&balloon);
--
2.17.0
Philip Moltman is no longer a maintainer of the VMware balloon. Setting
Nadav Amit as one instead.
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
MAINTAINERS | 2 +-
drivers/misc/vmw_balloon.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5c374d97c967..78009d21293a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15136,7 +15136,7 @@ F: include/linux/vme*
VMWARE BALLOON DRIVER
M: Xavier Deguillard <[email protected]>
-M: Philip Moltmann <[email protected]>
+M: Nadav Amit <[email protected]>
M: "VMware, Inc." <[email protected]>
L: [email protected]
S: Maintained
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index b982059ce7b7..a10eab56a47c 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -15,7 +15,7 @@
* details.
*
* Maintained by: Xavier Deguillard <[email protected]>
- * Philip Moltmann <[email protected]>
+ * Nadav Amit <[email protected]>
*/
/*
--
2.17.0
Removing the paragraph about writing to the Free Software Foundation's
mailing address from the sample GPL notice according to checkpatch
request.
In addition, updating the year and adding a license tag.
Reviewed-by: Xavier Deguillard <[email protected]>
Signed-off-by: Nadav Amit <[email protected]>
---
drivers/misc/vmw_balloon.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 400a1ccefc8e..b982059ce7b7 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1,7 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* VMware Balloon driver.
*
- * Copyright (C) 2000-2014, VMware, Inc. All Rights Reserved.
+ * Copyright (C) 2000-2018, VMware, Inc. All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -13,10 +14,6 @@
* NON INFRINGEMENT. See the GNU General Public License for more
* details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
* Maintained by: Xavier Deguillard <[email protected]>
* Philip Moltmann <[email protected]>
*/
--
2.17.0
On Wed, Jun 13, 2018 at 06:54:06AM -0700, Nadav Amit wrote:
> When balloon batching is not supported by the hypervisor, the guest
> frame number (GFN) must fit in 32-bit. However, due to a bug, this check
> was mistakenly ignored. In practice, when total RAM is greater than
> 16TB, the balloon does not work currently, making this bug unlikely to
> happen.
>
> Fixes: ef0f8f112984 ("VMware balloon: partially inline vmballoon_reserve_page.")
>
> Cc: [email protected]
Again, no blank line between these things.
Also, please break this up into two different patch series. One for
4.18-final and one for 4.19-rc1. You should always split up bugfixes
from new features/cleanups as that is what I have to do when sending
them to Linus.
thanks,
greg k-h
On Wed, Jun 13, 2018 at 06:54:11AM -0700, Nadav Amit wrote:
> Removing the paragraph about writing to the Free Software Foundation's
> mailing address from the sample GPL notice according to checkpatch
> request.
>
> In addition, updating the year and adding a license tag.
>
> Reviewed-by: Xavier Deguillard <[email protected]>
> Signed-off-by: Nadav Amit <[email protected]>
> ---
> drivers/misc/vmw_balloon.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index 400a1ccefc8e..b982059ce7b7 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -1,7 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * VMware Balloon driver.
> *
> - * Copyright (C) 2000-2014, VMware, Inc. All Rights Reserved.
> + * Copyright (C) 2000-2018, VMware, Inc. All Rights Reserved.
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License as published by the
> @@ -13,10 +14,6 @@
> * NON INFRINGEMENT. See the GNU General Public License for more
> * details.
You still have a lot of boiler-plate text in here that can be removed.
Please do so.
thanks,
greg k-h
at 10:24 PM, Greg Kroah-Hartman <[email protected]> wrote:
> On Wed, Jun 13, 2018 at 06:54:11AM -0700, Nadav Amit wrote:
>> Removing the paragraph about writing to the Free Software Foundation's
>> mailing address from the sample GPL notice according to checkpatch
>> request.
>>
>> In addition, updating the year and adding a license tag.
>>
>> Reviewed-by: Xavier Deguillard <[email protected]>
>> Signed-off-by: Nadav Amit <[email protected]>
>> ---
>> drivers/misc/vmw_balloon.c | 7 ++-----
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
>> index 400a1ccefc8e..b982059ce7b7 100644
>> --- a/drivers/misc/vmw_balloon.c
>> +++ b/drivers/misc/vmw_balloon.c
>> @@ -1,7 +1,8 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> /*
>> * VMware Balloon driver.
>> *
>> - * Copyright (C) 2000-2014, VMware, Inc. All Rights Reserved.
>> + * Copyright (C) 2000-2018, VMware, Inc. All Rights Reserved.
>> *
>> * This program is free software; you can redistribute it and/or modify it
>> * under the terms of the GNU General Public License as published by the
>> @@ -13,10 +14,6 @@
>> * NON INFRINGEMENT. See the GNU General Public License for more
>> * details.
>
> You still have a lot of boiler-plate text in here that can be removed.
> Please do so.
I will remove the maintainers, since it is really unsuitable.
But what else do you want me to remove? This is a standard GPL license. If I
am required to remove the GPL license, I will have to run checks to ensure
it is appropriate.
Thanks,
Nadav
at 10:23 PM, Greg Kroah-Hartman <[email protected]> wrote:
> On Wed, Jun 13, 2018 at 06:54:06AM -0700, Nadav Amit wrote:
>> When balloon batching is not supported by the hypervisor, the guest
>> frame number (GFN) must fit in 32-bit. However, due to a bug, this check
>> was mistakenly ignored. In practice, when total RAM is greater than
>> 16TB, the balloon does not work currently, making this bug unlikely to
>> happen.
>>
>> Fixes: ef0f8f112984 ("VMware balloon: partially inline vmballoon_reserve_page.")
>>
>> Cc: [email protected]
>
> Again, no blank line between these things.
>
> Also, please break this up into two different patch series. One for
> 4.18-final and one for 4.19-rc1. You should always split up bugfixes
> from new features/cleanups as that is what I have to do when sending
> them to Linus.
I have only sent what I thought is appropriate for 4.18 (cleanup/features
will be sent separately):
* 5 bug fixes (1-5/7)
* 1 update the maintainer list (7/7)
* 1 update to the comment to prevent checkpatch from complaining (6/7)
If you think any patch is unsuitable to 4.18 - please say which.
Thanks,
Nadav
On Thu, Jun 14, 2018 at 05:33:46AM +0000, Nadav Amit wrote:
> at 10:24 PM, Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Wed, Jun 13, 2018 at 06:54:11AM -0700, Nadav Amit wrote:
> >> Removing the paragraph about writing to the Free Software Foundation's
> >> mailing address from the sample GPL notice according to checkpatch
> >> request.
> >>
> >> In addition, updating the year and adding a license tag.
> >>
> >> Reviewed-by: Xavier Deguillard <[email protected]>
> >> Signed-off-by: Nadav Amit <[email protected]>
> >> ---
> >> drivers/misc/vmw_balloon.c | 7 ++-----
> >> 1 file changed, 2 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> >> index 400a1ccefc8e..b982059ce7b7 100644
> >> --- a/drivers/misc/vmw_balloon.c
> >> +++ b/drivers/misc/vmw_balloon.c
> >> @@ -1,7 +1,8 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> /*
> >> * VMware Balloon driver.
> >> *
> >> - * Copyright (C) 2000-2014, VMware, Inc. All Rights Reserved.
> >> + * Copyright (C) 2000-2018, VMware, Inc. All Rights Reserved.
> >> *
> >> * This program is free software; you can redistribute it and/or modify it
> >> * under the terms of the GNU General Public License as published by the
> >> @@ -13,10 +14,6 @@
> >> * NON INFRINGEMENT. See the GNU General Public License for more
> >> * details.
> >
> > You still have a lot of boiler-plate text in here that can be removed.
> > Please do so.
>
> I will remove the maintainers, since it is really unsuitable.
>
> But what else do you want me to remove? This is a standard GPL license. If I
> am required to remove the GPL license, I will have to run checks to ensure
> it is appropriate.
Yes, all of the license text should be removed. Right now we have 700+
differnt wordings of "This file is released under the GPL" in the kernel
tree, and we need to clean that up. Look at all of the other SPDX
cleanups we have done already to get an idea of what is needed here.
As one example, look at 0c3b34a569f8 ("USB: typec: Remove remaining
redundant license text") which can be done after the correct SPDX line
is in the file. That's the end-goal for all kernel files.
thanks,
greg k-h
On Thu, Jun 14, 2018 at 05:41:01AM +0000, Nadav Amit wrote:
> at 10:23 PM, Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Wed, Jun 13, 2018 at 06:54:06AM -0700, Nadav Amit wrote:
> >> When balloon batching is not supported by the hypervisor, the guest
> >> frame number (GFN) must fit in 32-bit. However, due to a bug, this check
> >> was mistakenly ignored. In practice, when total RAM is greater than
> >> 16TB, the balloon does not work currently, making this bug unlikely to
> >> happen.
> >>
> >> Fixes: ef0f8f112984 ("VMware balloon: partially inline vmballoon_reserve_page.")
> >>
> >> Cc: [email protected]
> >
> > Again, no blank line between these things.
> >
> > Also, please break this up into two different patch series. One for
> > 4.18-final and one for 4.19-rc1. You should always split up bugfixes
> > from new features/cleanups as that is what I have to do when sending
> > them to Linus.
>
> I have only sent what I thought is appropriate for 4.18 (cleanup/features
> will be sent separately):
>
> * 5 bug fixes (1-5/7)
> * 1 update the maintainer list (7/7)
> * 1 update to the comment to prevent checkpatch from complaining (6/7)
>
> If you think any patch is unsuitable to 4.18 - please say which.
How do I know which patch goes to which branch? Please make it so
obvious I can not get it wrong. Remember, I get 1000 emails a day, you
don't want me to have to make a judgement call about anything, as I will
mess it up :)
thanks,
greg k-h
at 10:50 PM, Greg Kroah-Hartman <[email protected]> wrote:
> On Thu, Jun 14, 2018 at 05:41:01AM +0000, Nadav Amit wrote:
>> at 10:23 PM, Greg Kroah-Hartman <[email protected]> wrote:
>>
>>> On Wed, Jun 13, 2018 at 06:54:06AM -0700, Nadav Amit wrote:
>>>> When balloon batching is not supported by the hypervisor, the guest
>>>> frame number (GFN) must fit in 32-bit. However, due to a bug, this check
>>>> was mistakenly ignored. In practice, when total RAM is greater than
>>>> 16TB, the balloon does not work currently, making this bug unlikely to
>>>> happen.
>>>>
>>>> Fixes: ef0f8f112984 ("VMware balloon: partially inline vmballoon_reserve_page.")
>>>>
>>>> Cc: [email protected]
>>>
>>> Again, no blank line between these things.
>>>
>>> Also, please break this up into two different patch series. One for
>>> 4.18-final and one for 4.19-rc1. You should always split up bugfixes
>>> from new features/cleanups as that is what I have to do when sending
>>> them to Linus.
>>
>> I have only sent what I thought is appropriate for 4.18 (cleanup/features
>> will be sent separately):
>>
>> * 5 bug fixes (1-5/7)
>> * 1 update the maintainer list (7/7)
>> * 1 update to the comment to prevent checkpatch from complaining (6/7)
>>
>> If you think any patch is unsuitable to 4.18 - please say which.
>
> How do I know which patch goes to which branch? Please make it so
> obvious I can not get it wrong. Remember, I get 1000 emails a day, you
> don't want me to have to make a judgement call about anything, as I will
> mess it up :)
I appreciate your time, and understand you even need to read students’
dissertations. ;-)
I will write it clearly in the cover-letter in the next version, and move
the license changes to 4.19.
Thanks,
Nadav
On Thu, 14 Jun 2018 05:33:46 -0000, Nadav Amit said:
> >> In addition, updating the year and adding a license tag.
> >> +// SPDX-License-Identifier: GPL-2.0
> > You still have a lot of boiler-plate text in here that can be removed.
> > Please do so.
> But what else do you want me to remove? This is a standard GPL license. If I
> am required to remove the GPL license, I will have to run checks to ensure
> it is appropriate.
You mean the checks that you should have done when you stuck the SPDX
tag on the file?
(Hint: Think of the SPDX tag as a '#include gplv2.license.blurb" :)
at 1:35 PM, [email protected] wrote:
> On Thu, 14 Jun 2018 05:33:46 -0000, Nadav Amit said:
>
>>>> In addition, updating the year and adding a license tag.
>
>>>> +// SPDX-License-Identifier: GPL-2.0
>
>>> You still have a lot of boiler-plate text in here that can be removed.
>>> Please do so.
>
>> But what else do you want me to remove? This is a standard GPL license. If I
>> am required to remove the GPL license, I will have to run checks to ensure
>> it is appropriate.
>
> You mean the checks that you should have done when you stuck the SPDX
> tag on the file?
>
> (Hint: Think of the SPDX tag as a '#include gplv2.license.blurb" :)
I matched the SPDX to the license we have, tut I don’t know whether my
company regards an SPDX tag as a sufficient means to protect copyrights,
which allows to remove all the rest of the text.
Just trying not to get fired for a stupid reason ;)
On Fri, Jun 15, 2018 at 08:38:57PM +0000, Nadav Amit wrote:
> at 1:35 PM, [email protected] wrote:
>
> > On Thu, 14 Jun 2018 05:33:46 -0000, Nadav Amit said:
> >
> >>>> In addition, updating the year and adding a license tag.
> >
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >
> >>> You still have a lot of boiler-plate text in here that can be removed.
> >>> Please do so.
> >
> >> But what else do you want me to remove? This is a standard GPL license. If I
> >> am required to remove the GPL license, I will have to run checks to ensure
> >> it is appropriate.
> >
> > You mean the checks that you should have done when you stuck the SPDX
> > tag on the file?
> >
> > (Hint: Think of the SPDX tag as a '#include gplv2.license.blurb" :)
>
> I matched the SPDX to the license we have, tut I don’t know whether my
> company regards an SPDX tag as a sufficient means to protect copyrights,
> which allows to remove all the rest of the text.
Copyrights have nothing to do with SPDX lines, please don't get
copyrights and licenses confused, they are two totally different things.
Removing the license boilerplate and only using a SPDX line for the
license marking does not change the copyright of a file at all. And if
you use the correct SPDX line, it will not change the license of the
file at all either.
If your company's lawyers have any questions about this, I am glad to
discuss this with them and point them at the lawyers that created the
SPDX group.
But really, your company lawyers already know all about this, you should
not have any problems at all. If you are worried about this, please
contact your open source group and they will be glad to help out. I
have names if you want me to send them to you off-list :)
thanks,
greg k-h