[PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Vitaly Kuznetsov]

Hyper-V memory hotplug protocol has 2M granularity and in Linux x86 we use
128M. To deal with it we implement partial section onlining by registering
custom page onlining callback (hv_online_page()). Later, when more memory
arrives we try to online the 'tail' (see hv_bring_pgs_online()).

It was found that in some cases this 'tail' onlining causes issues:

 BUG: Bad page state in process kworker/0:2  pfn:109e3a
 page:ffffe08344278e80 count:0 mapcount:1 mapping:0000000000000000 index:0x0
 flags: 0xfffff80000000()
 raw: 000fffff80000000 dead000000000100 dead000000000200 0000000000000000
 raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
 page dumped because: nonzero mapcount
 ...
 Workqueue: events hot_add_req [hv_balloon]
 Call Trace:
  dump_stack+0x5c/0x80
  bad_page.cold.112+0x7f/0xb2
  free_pcppages_bulk+0x4b8/0x690
  free_unref_page+0x54/0x70
  hv_page_online_one+0x5c/0x80 [hv_balloon]
  hot_add_req.cold.24+0x182/0x835 [hv_balloon]
  ...

Turns out that we now have deferred struct page initialization for memory
hotplug so e.g. memory_block_action() in drivers/base/memory.c does
pages_correctly_probed() check and in that check it avoids inspecting
struct pages and checks sections instead. But in Hyper-V balloon driver we
do PageReserved(pfn_to_page()) check and this is now wrong.

Switch to checking online_section_nr() instead.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/hv_balloon.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 5301fef16c31..7c6349a50ef1 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -888,12 +888,14 @@ static unsigned long handle_pg_range(unsigned long pg_start,
 			pfn_cnt -= pgs_ol;
 			/*
 			 * Check if the corresponding memory block is already
-			 * online by checking its last previously backed page.
-			 * In case it is we need to bring rest (which was not
-			 * backed previously) online too.
+			 * online. It is possible to observe struct pages still
+			 * being uninitialized here so check section instead.
+			 * In case the section is online we need to bring the
+			 * rest of pfns (which were not backed previously)
+			 * online too.
 			 */
 			if (start_pfn > has->start_pfn &&
-			    !PageReserved(pfn_to_page(start_pfn - 1)))
+			    online_section_nr(pfn_to_section_nr(start_pfn)))
 				hv_bring_pgs_online(has, start_pfn, pgs_ol);
 
 		}
-- 
2.20.1

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[David Hildenbrand]

On 04.01.19 15:19, Vitaly Kuznetsov wrote:
> Hyper-V memory hotplug protocol has 2M granularity and in Linux x86 we use
> 128M. To deal with it we implement partial section onlining by registering
> custom page onlining callback (hv_online_page()). Later, when more memory
> arrives we try to online the 'tail' (see hv_bring_pgs_online()).
> 
> It was found that in some cases this 'tail' onlining causes issues:
> 
>  BUG: Bad page state in process kworker/0:2  pfn:109e3a
>  page:ffffe08344278e80 count:0 mapcount:1 mapping:0000000000000000 index:0x0
>  flags: 0xfffff80000000()
>  raw: 000fffff80000000 dead000000000100 dead000000000200 0000000000000000
>  raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>  page dumped because: nonzero mapcount
>  ...
>  Workqueue: events hot_add_req [hv_balloon]
>  Call Trace:
>   dump_stack+0x5c/0x80
>   bad_page.cold.112+0x7f/0xb2
>   free_pcppages_bulk+0x4b8/0x690
>   free_unref_page+0x54/0x70
>   hv_page_online_one+0x5c/0x80 [hv_balloon]
>   hot_add_req.cold.24+0x182/0x835 [hv_balloon]
>   ...
> 
> Turns out that we now have deferred struct page initialization for memory
> hotplug so e.g. memory_block_action() in drivers/base/memory.c does
> pages_correctly_probed() check and in that check it avoids inspecting
> struct pages and checks sections instead. But in Hyper-V balloon driver we
> do PageReserved(pfn_to_page()) check and this is now wrong.
> 
> Switch to checking online_section_nr() instead.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  drivers/hv/hv_balloon.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 5301fef16c31..7c6349a50ef1 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -888,12 +888,14 @@ static unsigned long handle_pg_range(unsigned long pg_start,
>  			pfn_cnt -= pgs_ol;
>  			/*
>  			 * Check if the corresponding memory block is already
> -			 * online by checking its last previously backed page.
> -			 * In case it is we need to bring rest (which was not
> -			 * backed previously) online too.
> +			 * online. It is possible to observe struct pages still
> +			 * being uninitialized here so check section instead.
> +			 * In case the section is online we need to bring the
> +			 * rest of pfns (which were not backed previously)
> +			 * online too.
>  			 */
>  			if (start_pfn > has->start_pfn &&
> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>  
>  		}
> 

I wonder if you should use pfn_to_online_page() and check for PageOffline().

(I guess online_section_nr() should also do the trick)

-- 

Thanks,

David / dhildenb

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Vitaly Kuznetsov]

David Hildenbrand <david@redhat.com> writes:

> On 04.01.19 15:19, Vitaly Kuznetsov wrote:
>> Hyper-V memory hotplug protocol has 2M granularity and in Linux x86 we use
>> 128M. To deal with it we implement partial section onlining by registering
>> custom page onlining callback (hv_online_page()). Later, when more memory
>> arrives we try to online the 'tail' (see hv_bring_pgs_online()).
>> 
>> It was found that in some cases this 'tail' onlining causes issues:
>> 
>>  BUG: Bad page state in process kworker/0:2  pfn:109e3a
>>  page:ffffe08344278e80 count:0 mapcount:1 mapping:0000000000000000 index:0x0
>>  flags: 0xfffff80000000()
>>  raw: 000fffff80000000 dead000000000100 dead000000000200 0000000000000000
>>  raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>  page dumped because: nonzero mapcount
>>  ...
>>  Workqueue: events hot_add_req [hv_balloon]
>>  Call Trace:
>>   dump_stack+0x5c/0x80
>>   bad_page.cold.112+0x7f/0xb2
>>   free_pcppages_bulk+0x4b8/0x690
>>   free_unref_page+0x54/0x70
>>   hv_page_online_one+0x5c/0x80 [hv_balloon]
>>   hot_add_req.cold.24+0x182/0x835 [hv_balloon]
>>   ...
>> 
>> Turns out that we now have deferred struct page initialization for memory
>> hotplug so e.g. memory_block_action() in drivers/base/memory.c does
>> pages_correctly_probed() check and in that check it avoids inspecting
>> struct pages and checks sections instead. But in Hyper-V balloon driver we
>> do PageReserved(pfn_to_page()) check and this is now wrong.
>> 
>> Switch to checking online_section_nr() instead.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  drivers/hv/hv_balloon.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> index 5301fef16c31..7c6349a50ef1 100644
>> --- a/drivers/hv/hv_balloon.c
>> +++ b/drivers/hv/hv_balloon.c
>> @@ -888,12 +888,14 @@ static unsigned long handle_pg_range(unsigned long pg_start,
>>  			pfn_cnt -= pgs_ol;
>>  			/*
>>  			 * Check if the corresponding memory block is already
>> -			 * online by checking its last previously backed page.
>> -			 * In case it is we need to bring rest (which was not
>> -			 * backed previously) online too.
>> +			 * online. It is possible to observe struct pages still
>> +			 * being uninitialized here so check section instead.
>> +			 * In case the section is online we need to bring the
>> +			 * rest of pfns (which were not backed previously)
>> +			 * online too.
>>  			 */
>>  			if (start_pfn > has->start_pfn &&
>> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
>> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>>  
>>  		}
>> 
>
> I wonder if you should use pfn_to_online_page() and check for PageOffline().
>
> (I guess online_section_nr() should also do the trick)

I'm worried a bit about racing with mm code here as we're not doing
mem_hotplug_begin()/done() so I'd slightly prefer keeping
online_section_nr() (pfn_to_online_page() also uses it but then it gets
to the particular struct page). Moreover, with pfn_to_online_page() we
will be looking at some other pfn - because the start_pfn is definitelly
offline (pre-patch we were looking at start_pfn-1). Just looking at the
whole section seems cleaner.

P.S. I still think about bringing mem_hotplug_begin()/done() to
hv_balloon but that's going to be a separate discussion, here I want to
have a small fix backportable to stable.

Thanks,

-- 
Vitaly

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Sasha Levin]

On Mon, Jan 07, 2019 at 02:44:30PM +0100, Vitaly Kuznetsov wrote:
>David Hildenbrand <david@redhat.com> writes:
>> On 04.01.19 15:19, Vitaly Kuznetsov wrote:
>>> Hyper-V memory hotplug protocol has 2M granularity and in Linux x86 we use
>>> 128M. To deal with it we implement partial section onlining by registering
>>> custom page onlining callback (hv_online_page()). Later, when more memory
>>> arrives we try to online the 'tail' (see hv_bring_pgs_online()).
>>>
>>> It was found that in some cases this 'tail' onlining causes issues:
>>>
>>>  BUG: Bad page state in process kworker/0:2  pfn:109e3a
>>>  page:ffffe08344278e80 count:0 mapcount:1 mapping:0000000000000000 index:0x0
>>>  flags: 0xfffff80000000()
>>>  raw: 000fffff80000000 dead000000000100 dead000000000200 0000000000000000
>>>  raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>  page dumped because: nonzero mapcount
>>>  ...
>>>  Workqueue: events hot_add_req [hv_balloon]
>>>  Call Trace:
>>>   dump_stack+0x5c/0x80
>>>   bad_page.cold.112+0x7f/0xb2
>>>   free_pcppages_bulk+0x4b8/0x690
>>>   free_unref_page+0x54/0x70
>>>   hv_page_online_one+0x5c/0x80 [hv_balloon]
>>>   hot_add_req.cold.24+0x182/0x835 [hv_balloon]
>>>   ...
>>>
>>> Turns out that we now have deferred struct page initialization for memory
>>> hotplug so e.g. memory_block_action() in drivers/base/memory.c does
>>> pages_correctly_probed() check and in that check it avoids inspecting
>>> struct pages and checks sections instead. But in Hyper-V balloon driver we
>>> do PageReserved(pfn_to_page()) check and this is now wrong.
>>>
>>> Switch to checking online_section_nr() instead.
>>>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> ---
>>>  drivers/hv/hv_balloon.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>>> index 5301fef16c31..7c6349a50ef1 100644
>>> --- a/drivers/hv/hv_balloon.c
>>> +++ b/drivers/hv/hv_balloon.c
>>> @@ -888,12 +888,14 @@ static unsigned long handle_pg_range(unsigned long pg_start,
>>>  			pfn_cnt -= pgs_ol;
>>>  			/*
>>>  			 * Check if the corresponding memory block is already
>>> -			 * online by checking its last previously backed page.
>>> -			 * In case it is we need to bring rest (which was not
>>> -			 * backed previously) online too.
>>> +			 * online. It is possible to observe struct pages still
>>> +			 * being uninitialized here so check section instead.
>>> +			 * In case the section is online we need to bring the
>>> +			 * rest of pfns (which were not backed previously)
>>> +			 * online too.
>>>  			 */
>>>  			if (start_pfn > has->start_pfn &&
>>> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
>>> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>>>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>>>
>>>  		}
>>>
>>
>> I wonder if you should use pfn_to_online_page() and check for PageOffline().
>>
>> (I guess online_section_nr() should also do the trick)
>
>I'm worried a bit about racing with mm code here as we're not doing
>mem_hotplug_begin()/done() so I'd slightly prefer keeping
>online_section_nr() (pfn_to_online_page() also uses it but then it gets
>to the particular struct page). Moreover, with pfn_to_online_page() we
>will be looking at some other pfn - because the start_pfn is definitelly
>offline (pre-patch we were looking at start_pfn-1). Just looking at the
>whole section seems cleaner.
>
>P.S. I still think about bringing mem_hotplug_begin()/done() to
>hv_balloon but that's going to be a separate discussion, here I want to
>have a small fix backportable to stable.

This should probably be marked for stable then :)

--
Thanks,
Sasha

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Vitaly Kuznetsov]

Sasha Levin <sashal@kernel.org> writes:

> On Mon, Jan 07, 2019 at 02:44:30PM +0100, Vitaly Kuznetsov wrote:
>>P.S. I still think about bringing mem_hotplug_begin()/done() to
>>hv_balloon but that's going to be a separate discussion, here I want to
>>have a small fix backportable to stable.
>
> This should probably be marked for stable then :)
>

Yes, please :-)

I didn't test old kernels but this seems to be an old issue,
e.g. DEFERRED_STRUCT_PAGE_INIT appeared in 4.2 - not sure if memory
hotplug code was using it back then but, oh well, it's all history now.

(I remember Greg disliked when people were tagging patches for stable@
themselves, he prefered maintainers deciding if the particular commit
deserves stable@ or not - but as you have a tree now we may as well have
different rules :-)

-- 
Vitaly

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Greg KH]

On Mon, Jan 07, 2019 at 07:38:20PM +0100, Vitaly Kuznetsov wrote:
> (I remember Greg disliked when people were tagging patches for stable@
> themselves, he prefered maintainers deciding if the particular commit
> deserves stable@ or not - but as you have a tree now we may as well have
> different rules :-)

I don't recall ever saying I disliked that, unless people were tagging
stuff that were obviously not stable material.  Which, given the history
of this codebase, was probably the case :)

greg k-h

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Dan Carpenter]

On Mon, Jan 07, 2019 at 07:38:20PM +0100, Vitaly Kuznetsov wrote:
> (I remember Greg disliked when people were tagging patches for stable@
> themselves, he prefered maintainers deciding if the particular commit
> deserves stable@ or not - but as you have a tree now we may as well have
> different rules :-)

You're thinking of Dave Miller.  So far as I know he's the only
maintainer who handles stable that way.  It's a lot of work to do the
things Dave does.  #understatement

regards,
dan carpenter

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Vitaly Kuznetsov]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Mon, Jan 07, 2019 at 07:38:20PM +0100, Vitaly Kuznetsov wrote:
>> (I remember Greg disliked when people were tagging patches for stable@
>> themselves, he prefered maintainers deciding if the particular commit
>> deserves stable@ or not - but as you have a tree now we may as well have
>> different rules :-)
>
> You're thinking of Dave Miller.  So far as I know he's the only
> maintainer who handles stable that way.

This may actually be the case, you think "one of the the mighty Linux
overlords" but your fingers type "Greg" instread. Sorry for the
confusion :-)

-- 
Vitaly

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Sasha Levin]

On Mon, Jan 07, 2019 at 07:38:20PM +0100, Vitaly Kuznetsov wrote:
>Sasha Levin <sashal@kernel.org> writes:
>
>> On Mon, Jan 07, 2019 at 02:44:30PM +0100, Vitaly Kuznetsov wrote:
>>>P.S. I still think about bringing mem_hotplug_begin()/done() to
>>>hv_balloon but that's going to be a separate discussion, here I want to
>>>have a small fix backportable to stable.
>>
>> This should probably be marked for stable then :)
>>
>
>Yes, please :-)

Queued up for hyperv-fixes with a -stable tag, thank you!

--
Thanks,
Sasha

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[David Hildenbrand]

On 07.01.19 14:44, Vitaly Kuznetsov wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 04.01.19 15:19, Vitaly Kuznetsov wrote:
>>> Hyper-V memory hotplug protocol has 2M granularity and in Linux x86 we use
>>> 128M. To deal with it we implement partial section onlining by registering
>>> custom page onlining callback (hv_online_page()). Later, when more memory
>>> arrives we try to online the 'tail' (see hv_bring_pgs_online()).
>>>
>>> It was found that in some cases this 'tail' onlining causes issues:
>>>
>>>  BUG: Bad page state in process kworker/0:2  pfn:109e3a
>>>  page:ffffe08344278e80 count:0 mapcount:1 mapping:0000000000000000 index:0x0
>>>  flags: 0xfffff80000000()
>>>  raw: 000fffff80000000 dead000000000100 dead000000000200 0000000000000000
>>>  raw: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>  page dumped because: nonzero mapcount
>>>  ...
>>>  Workqueue: events hot_add_req [hv_balloon]
>>>  Call Trace:
>>>   dump_stack+0x5c/0x80
>>>   bad_page.cold.112+0x7f/0xb2
>>>   free_pcppages_bulk+0x4b8/0x690
>>>   free_unref_page+0x54/0x70
>>>   hv_page_online_one+0x5c/0x80 [hv_balloon]
>>>   hot_add_req.cold.24+0x182/0x835 [hv_balloon]
>>>   ...
>>>
>>> Turns out that we now have deferred struct page initialization for memory
>>> hotplug so e.g. memory_block_action() in drivers/base/memory.c does
>>> pages_correctly_probed() check and in that check it avoids inspecting
>>> struct pages and checks sections instead. But in Hyper-V balloon driver we
>>> do PageReserved(pfn_to_page()) check and this is now wrong.
>>>
>>> Switch to checking online_section_nr() instead.
>>>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> ---
>>>  drivers/hv/hv_balloon.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>>> index 5301fef16c31..7c6349a50ef1 100644
>>> --- a/drivers/hv/hv_balloon.c
>>> +++ b/drivers/hv/hv_balloon.c
>>> @@ -888,12 +888,14 @@ static unsigned long handle_pg_range(unsigned long pg_start,
>>>  			pfn_cnt -= pgs_ol;
>>>  			/*
>>>  			 * Check if the corresponding memory block is already
>>> -			 * online by checking its last previously backed page.
>>> -			 * In case it is we need to bring rest (which was not
>>> -			 * backed previously) online too.
>>> +			 * online. It is possible to observe struct pages still
>>> +			 * being uninitialized here so check section instead.
>>> +			 * In case the section is online we need to bring the
>>> +			 * rest of pfns (which were not backed previously)
>>> +			 * online too.
>>>  			 */
>>>  			if (start_pfn > has->start_pfn &&
>>> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
>>> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>>>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>>>  
>>>  		}
>>>
>>
>> I wonder if you should use pfn_to_online_page() and check for PageOffline().
>>
>> (I guess online_section_nr() should also do the trick)
> 
> I'm worried a bit about racing with mm code here as we're not doing
> mem_hotplug_begin()/done() so I'd slightly prefer keeping
> online_section_nr() (pfn_to_online_page() also uses it but then it gets
> to the particular struct page). Moreover, with pfn_to_online_page() we
> will be looking at some other pfn - because the start_pfn is definitelly
> offline (pre-patch we were looking at start_pfn-1). Just looking at the
> whole section seems cleaner.

Fine with me. I guess the section can never be offlined as it still
contains reserved pages if not fully "fake-onlined" by HV code already.

But we could still consider mem_hotplug_begin()/done() as we could have
a online section although online_pages() has not completed yet. So we
could actually touch some "semi onlined section".

Acked-by: David Hildenbrand <david@redhat.com>

> 
> P.S. I still think about bringing mem_hotplug_begin()/done() to
> hv_balloon but that's going to be a separate discussion, here I want to
> have a small fix backportable to stable.
> 
> Thanks,
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH] hv_balloon: avoid touching uninitialized struct page during tail onlining

[Vitaly Kuznetsov]

David Hildenbrand <david@redhat.com> writes:

> On 07.01.19 14:44, Vitaly Kuznetsov wrote:
>> David Hildenbrand <david@redhat.com> writes:
>> 
...
>>> On 04.01.19 15:19, Vitaly Kuznetsov wrote:
>>>>  			if (start_pfn > has->start_pfn &&
>>>> -			    !PageReserved(pfn_to_page(start_pfn - 1)))
>>>> +			    online_section_nr(pfn_to_section_nr(start_pfn)))
>>>>  				hv_bring_pgs_online(has, start_pfn, pgs_ol);
>>>>  
>>>>  		}
>>>>
>>>
>>> I wonder if you should use pfn_to_online_page() and check for PageOffline().
>>>
>>> (I guess online_section_nr() should also do the trick)
>> 
>> I'm worried a bit about racing with mm code here as we're not doing
>> mem_hotplug_begin()/done() so I'd slightly prefer keeping
>> online_section_nr() (pfn_to_online_page() also uses it but then it gets
>> to the particular struct page). Moreover, with pfn_to_online_page() we
>> will be looking at some other pfn - because the start_pfn is definitelly
>> offline (pre-patch we were looking at start_pfn-1). Just looking at the
>> whole section seems cleaner.
>
> Fine with me. I guess the section can never be offlined as it still
> contains reserved pages if not fully "fake-onlined" by HV code already.
>
> But we could still consider mem_hotplug_begin()/done() as we could have
> a online section although online_pages() has not completed yet. So we
> could actually touch some "semi onlined section".

Yes, exactly, if we race with section onlining here we may never online
the tail so it will remain 'semi onlined'. I'm going to propose
exporting mem_hotplug_begin()/done() to modules to fix this (in a
separate patch because I anticipate some pushback :-)

>
> Acked-by: David Hildenbrand <david@redhat.com>
>

Thanks!

-- 
Vitaly