* Re: [RFC] virtio-iommu version 0.5
From: Linu Cherian @ 2017-10-24 6:27 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: virtio-dev, lorenzo.pieralisi, ashok.raj, kvm, mst, marc.zyngier,
will.deacon, Jayachandran.Nair, virtualization, eric.auger, iommu,
sunil.goutham, linu.cherian, robin.murphy, eric.auger.pro
In-Reply-To: <20171023093241.20113-1-jean-philippe.brucker@arm.com>
Hi Jean,
On Mon Oct 23, 2017 at 10:32:41AM +0100, Jean-Philippe Brucker wrote:
> This is version 0.5 of the virtio-iommu specification, the paravirtualized
> IOMMU. This version addresses feedback from v0.4 and adds an event virtqueue.
> Please find the specification, LaTeX sources and pdf, at:
> git://linux-arm.org/virtio-iommu.git viommu/v0.5
> http://linux-arm.org/git?p=virtio-iommu.git;a=blob;f=dist/v0.5/virtio-iommu-v0.5.pdf
>
> A detailed changelog since v0.4 follows. You can find the pdf diff at:
> http://linux-arm.org/git?p=virtio-iommu.git;a=blob;f=dist/diffs/virtio-iommu-pdf-diff-v0.4-v0.5.pdf
>
> * Add an event virtqueue for the device to report translation faults to
> the driver. For the moment only unrecoverable faults are available but
> future versions will extend it.
> * Simplify PROBE request by removing the ack part, and flattening RESV
> properties.
> * Rename "address space" to "domain". The change might seem futile but
> allows to introduce PASIDs and other features cleanly in the next
> versions. In the same vein, the few remaining "device" occurrences were
> replaced by "endpoint", to avoid any confusion with "the device"
> referring to the virtio device across the document.
> * Add implementation notes for RESV_MEM properties.
> * Update ACPI table definition.
> * Fix typos and clarify a few things.
>
> I will publish the Linux driver for v0.5 shortly. Then for next versions
> I'll focus on optimizations and adding support for hardware acceleration.
>
> Existing implementations are simple and can certainly be optimized, even
> without architectural changes. But the architecture itself can also be
> improved in a number of ways. Currently it is designed to work well with
> VFIO. However, having explicit MAP requests is less efficient* than page
> tables for emulated and PV endpoints, and the current architecture doesn't
> address this. Binding page tables is an obvious way to improve throughput
> in that case, but we can explore cleverer (and possibly simpler) ways to
> do it.
>
> So first we'll work on getting the base device and driver merged, then
> we'll analyze and compare several ideas for improving performance.
>
> Thanks,
> Jean
>
> * I have yet to study this behaviour, and would be interested in any
> prior art on the subject of analyzing devices DMA patterns (virtio and
> others)
From the spec,
Under future extensions.
"Page Table Handover, to allow guests to manage their own page tables and share them with the MMU"
Had few questions on this.
1. Did you mean SVM support for vfio-pci devices attached to guest processes here.
2. Can you give some hints on how this is going to work , since virtio-iommu guest kernel
driver need to create stage 1 page table as required by hardware which is not the case now.
CMIIW.
--
Linu cherian
^ permalink raw reply
* Re: [PATCH v1 3/3] virtio-balloon: stop inflating when OOM occurs
From: Wei Wang @ 2017-10-24 1:58 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: penguin-kernel, linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <20171022062159-mutt-send-email-mst@kernel.org>
On 10/23/2017 01:13 AM, Michael S. Tsirkin wrote:
> On Fri, Oct 20, 2017 at 07:54:26PM +0800, Wei Wang wrote:
>> This patch forces the cease of the inflating work when OOM occurs.
>> The fundamental idea of memory ballooning is to take out some guest
>> pages when the guest has low memory utilization, so it is sensible to
>> inflate nothing when the guest is already under memory pressure.
>>
>> On the other hand, the policy is determined by the admin or the
>> orchestration layer from the host. That is, the host is expected to
>> re-start the memory inflating request at a proper time later when
>> the guest has enough memory to inflate, for example, by checking
>> the memory stats reported by the balloon.
> Is there any other way to do it? And if so can't we just have guest do
> it automatically? Maybe the issue is really that fill attempts to
> allocate memory aggressively instead of checking availability.
> Maybe with deflate on oom it should check availability?
>
I think it might not be easy to do it in the guest in practice.
For example, the host asks for 4G from the guest, and the guest checks
that it has 4G that can be inflated at that point. While it is inflating
and 2G
is done inflating, another new task on the guest comes out and
takes the remaining 2G to use. Now the guest has nothing to inflate.
This would raise the questions:
1) what is the point of checking the availability?
Maybe we could just let the guest inflate as much as it can, that is, till
balloon_page_enqueue() returns NULL, then stop inflating.
2) How long would the host has to wait for this guest to get the
remaining 2G?
If I understand "guest do it automatically" correctly: now the guest is
responsible
for giving another 2G, which he owes to the host in this case - not
giving up inflating
whenever there is some free memory. Maybe in the next 1 hour it wouldn't
have any
memory available to give to the host. The time seems non-deterministic.
If we leave it to the host to define the policy, I think it would be easier.
Once the host finds that the guest can only offer 2G, then it can just
give up asking
for memory from this guest, and continue to check other guests to see if
it can get
some memory there to satisfy the needs.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v1 1/3] virtio-balloon: replace the coarse-grained balloon_lock
From: Wei Wang @ 2017-10-24 1:46 UTC (permalink / raw)
To: Tetsuo Handa, mst; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <201710222050.GIF35945.FHOMQFOVSFLtOJ@I-love.SAKURA.ne.jp>
On 10/22/2017 07:50 PM, Tetsuo Handa wrote:
> Wei Wang wrote:
>>>> @@ -162,20 +160,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>>>> msleep(200);
>>>> break;
>>>> }
>>>> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
>>>> - vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
>>>> + set_page_pfns(vb, pfns + num_pfns, page);
>>>> if (!virtio_has_feature(vb->vdev,
>>>> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
>>>> adjust_managed_page_count(page, -1);
>>>> }
>>>>
>>>> - num_allocated_pages = vb->num_pfns;
>>>> + mutex_lock(&vb->inflate_lock);
>>>> /* Did we get any? */
>>>> - if (vb->num_pfns != 0)
>>>> - tell_host(vb, vb->inflate_vq);
>>>> - mutex_unlock(&vb->balloon_lock);
>>>> + if (num_pfns != 0)
>>>> + tell_host(vb, vb->inflate_vq, pfns, num_pfns);
>>>> + mutex_unlock(&vb->inflate_lock);
>>>> + atomic64_add(num_pfns, &vb->num_pages);
>>> Isn't this addition too late? If leak_balloon() is called due to
>>> out_of_memory(), it will fail to find up to dated vb->num_pages value.
>> Not really. I think the old way of implementation above:
>> "vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE"
>> isn't quite accurate, because "vb->num_page" should reflect the number of
>> pages that have already been inflated, which means those pages have
>> already been given to the host via "tell_host()".
>>
>> If we update "vb->num_page" earlier before tell_host(), then it will
>> include the pages
>> that haven't been given to the host, which I think shouldn't be counted
>> as inflated pages.
>>
>> On the other hand, OOM will use leak_balloon() to release the pages that
>> should
>> have already been inflated.
> But leak_balloon() finds max inflated pages from vb->num_pages, doesn't it?
>
>>>>
>>>> /* We can only do one array worth at a time. */
>>>> - num = min(num, ARRAY_SIZE(vb->pfns));
>>>> + num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
>>>>
>>>> - mutex_lock(&vb->balloon_lock);
>>>> /* We can't release more pages than taken */
>>>> - num = min(num, (size_t)vb->num_pages);
>>>> - for (vb->num_pfns = 0; vb->num_pfns < num;
>>>> - vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>>>> + num = min_t(size_t, num, atomic64_read(&vb->num_pages));
>>>> + for (num_pfns = 0; num_pfns < num;
>>>> + num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>>>> page = balloon_page_dequeue(vb_dev_info);
>>> If balloon_page_dequeue() can be concurrently called by both host's request
>>> and guest's OOM event, is (!dequeued_page) test in balloon_page_dequeue() safe?
>>
>> I'm not sure about the question. The "dequeue_page" is a local variable
>> in the function, why would it be unsafe for two invocations (the shared
>> b_dev_info->pages are operated under a lock)?
> I'm not MM person nor virtio person. I'm commenting from point of view of
> safe programming. My question is, isn't there possibility of hitting
>
> if (unlikely(list_empty(&b_dev_info->pages) &&
> !b_dev_info->isolated_pages))
> BUG();
>
> when things run concurrently.
Thanks for the comments. I'm not 100% confident about all the possible
corner cases here at present
(e.g. why is the b_dev_info->page_lock released and re-gained in
balloon_page_dequeue()), and
Michael has given a preference of the solution, so I plan not to stick
with this one.
Best,
Wei
^ permalink raw reply
* [RFC] virtio-iommu version 0.5
From: Jean-Philippe Brucker @ 2017-10-23 9:32 UTC (permalink / raw)
To: iommu, kvm, virtualization, virtio-dev
Cc: lorenzo.pieralisi, ashok.raj, mst, marc.zyngier, will.deacon,
Jayachandran.Nair, eric.auger, robin.murphy, eric.auger.pro
This is version 0.5 of the virtio-iommu specification, the paravirtualized
IOMMU. This version addresses feedback from v0.4 and adds an event virtqueue.
Please find the specification, LaTeX sources and pdf, at:
git://linux-arm.org/virtio-iommu.git viommu/v0.5
http://linux-arm.org/git?p=virtio-iommu.git;a=blob;f=dist/v0.5/virtio-iommu-v0.5.pdf
A detailed changelog since v0.4 follows. You can find the pdf diff at:
http://linux-arm.org/git?p=virtio-iommu.git;a=blob;f=dist/diffs/virtio-iommu-pdf-diff-v0.4-v0.5.pdf
* Add an event virtqueue for the device to report translation faults to
the driver. For the moment only unrecoverable faults are available but
future versions will extend it.
* Simplify PROBE request by removing the ack part, and flattening RESV
properties.
* Rename "address space" to "domain". The change might seem futile but
allows to introduce PASIDs and other features cleanly in the next
versions. In the same vein, the few remaining "device" occurrences were
replaced by "endpoint", to avoid any confusion with "the device"
referring to the virtio device across the document.
* Add implementation notes for RESV_MEM properties.
* Update ACPI table definition.
* Fix typos and clarify a few things.
I will publish the Linux driver for v0.5 shortly. Then for next versions
I'll focus on optimizations and adding support for hardware acceleration.
Existing implementations are simple and can certainly be optimized, even
without architectural changes. But the architecture itself can also be
improved in a number of ways. Currently it is designed to work well with
VFIO. However, having explicit MAP requests is less efficient* than page
tables for emulated and PV endpoints, and the current architecture doesn't
address this. Binding page tables is an obvious way to improve throughput
in that case, but we can explore cleverer (and possibly simpler) ways to
do it.
So first we'll work on getting the base device and driver merged, then
we'll analyze and compare several ideas for improving performance.
Thanks,
Jean
* I have yet to study this behaviour, and would be interested in any
prior art on the subject of analyzing devices DMA patterns (virtio and
others)
^ permalink raw reply
* Re: [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics
From: Tomáš Golembiovský @ 2017-10-22 18:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, kvm, qemu-devel, virtualization, linux-mm, Shaohua Li,
Huang Ying
In-Reply-To: <20171019160405-mutt-send-email-mst@kernel.org>
On Thu, 19 Oct 2017 16:12:20 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Sep 21, 2017 at 02:55:41PM +0200, Tomáš Golembiovský wrote:
> > Add a new fields, VIRTIO_BALLOON_S_BUFFERS and VIRTIO_BALLOON_S_CACHED,
> > to virtio_balloon memory statistics protocol. The values correspond to
> > 'Buffers' and 'Cached' in /proc/meminfo.
> >
> > To be able to compute the value of 'Cached' memory it is necessary to
> > export total_swapcache_pages() to modules.
> >
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
>
> Does 'Buffers' actually make sense? It's a temporary storage -
> wouldn't it be significantly out of date by the time
> host receives it?
That would be best answered by somebody from kernel. But my personal
opinion is that it would not be out of date. The amount of memory
dedicated to Buffers does not seem to fluctuate too much.
Tomas
> > ---
> > drivers/virtio/virtio_balloon.c | 11 +++++++++++
> > include/uapi/linux/virtio_balloon.h | 4 +++-
> > mm/swap_state.c | 1 +
> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index f0b3a0b9d42f..c2558ec47a62 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -244,12 +244,19 @@ static unsigned int update_balloon_stats(struct virtio_balloon *vb)
> > struct sysinfo i;
> > unsigned int idx = 0;
> > long available;
> > + long cached;
> >
> > all_vm_events(events);
> > si_meminfo(&i);
> >
> > available = si_mem_available();
> >
> > + cached = global_node_page_state(NR_FILE_PAGES) -
> > + total_swapcache_pages() - i.bufferram;
> > + if (cached < 0)
> > + cached = 0;
> > +
> > +
> > #ifdef CONFIG_VM_EVENT_COUNTERS
> > update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
> > pages_to_bytes(events[PSWPIN]));
> > @@ -264,6 +271,10 @@ static unsigned int update_balloon_stats(struct virtio_balloon *vb)
> > pages_to_bytes(i.totalram));
> > update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
> > pages_to_bytes(available));
> > + update_stat(vb, idx++, VIRTIO_BALLOON_S_BUFFERS,
> > + pages_to_bytes(i.bufferram));
> > + update_stat(vb, idx++, VIRTIO_BALLOON_S_CACHED,
> > + pages_to_bytes(cached));
> >
> > return idx;
> > }
> > diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> > index 343d7ddefe04..d5dc8a56a497 100644
> > --- a/include/uapi/linux/virtio_balloon.h
> > +++ b/include/uapi/linux/virtio_balloon.h
> > @@ -52,7 +52,9 @@ struct virtio_balloon_config {
> > #define VIRTIO_BALLOON_S_MEMFREE 4 /* Total amount of free memory */
> > #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> > #define VIRTIO_BALLOON_S_AVAIL 6 /* Available memory as in /proc */
> > -#define VIRTIO_BALLOON_S_NR 7
> > +#define VIRTIO_BALLOON_S_BUFFERS 7 /* Buffers memory as in /proc */
> > +#define VIRTIO_BALLOON_S_CACHED 8 /* Cached memory as in /proc */
> > +#define VIRTIO_BALLOON_S_NR 9
> >
> > /*
> > * Memory statistics structure.
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 71ce2d1ccbf7..f3a4ff7d6c52 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -95,6 +95,7 @@ unsigned long total_swapcache_pages(void)
> > rcu_read_unlock();
> > return ret;
> > }
> > +EXPORT_SYMBOL_GPL(total_swapcache_pages);
> >
> > static atomic_t swapin_readahead_hits = ATOMIC_INIT(4);
>
> Need an ack from MM crowd on that.
>
> > --
> > 2.14.1
--
Tomáš Golembiovský <tgolembi@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v1 3/3] virtio-balloon: stop inflating when OOM occurs
From: Michael S. Tsirkin @ 2017-10-22 17:13 UTC (permalink / raw)
To: Wei Wang; +Cc: penguin-kernel, linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <1508500466-21165-4-git-send-email-wei.w.wang@intel.com>
On Fri, Oct 20, 2017 at 07:54:26PM +0800, Wei Wang wrote:
> This patch forces the cease of the inflating work when OOM occurs.
> The fundamental idea of memory ballooning is to take out some guest
> pages when the guest has low memory utilization, so it is sensible to
> inflate nothing when the guest is already under memory pressure.
>
> On the other hand, the policy is determined by the admin or the
> orchestration layer from the host. That is, the host is expected to
> re-start the memory inflating request at a proper time later when
> the guest has enough memory to inflate, for example, by checking
> the memory stats reported by the balloon.
Is there any other way to do it? And if so can't we just have guest do
it automatically? Maybe the issue is really that fill attempts to
allocate memory aggressively instead of checking availability.
Maybe with deflate on oom it should check availability?
> If another inflating
> requests is sent to guest when the guest is still under memory
> pressure, still no pages will be inflated.
Any such changes are hypervisor-visible and need a new feature bit.
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
> drivers/virtio/virtio_balloon.c | 33 +++++++++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index ab55cf8..cf29663 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -63,6 +63,15 @@ struct virtio_balloon {
> spinlock_t stop_update_lock;
> bool stop_update;
>
> + /*
> + * The balloon driver enters the oom mode if the oom notifier is
> + * invoked. Entering the oom mode will force the exit of current
> + * inflating work. When a later inflating request is received from
> + * the host, the success of memory allocation via balloon_page_enqueue
> + * will turn off the mode.
> + */
> + bool oom_mode;
> +
> /* Waiting for host to ack the pages we released. */
> wait_queue_head_t acked;
>
> @@ -142,22 +151,22 @@ static void set_page_pfns(struct virtio_balloon *vb,
> static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> {
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> + struct page *page;
> + size_t orig_num;
> unsigned int num_pfns;
> __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
>
> + orig_num = num;
> /* We can only do one array worth at a time. */
> num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
>
> for (num_pfns = 0; num_pfns < num;
> num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> - struct page *page = balloon_page_enqueue(vb_dev_info);
> -
> + page = balloon_page_enqueue(vb_dev_info);
> if (!page) {
> dev_info_ratelimited(&vb->vdev->dev,
> "Out of puff! Can't get %u pages\n",
> VIRTIO_BALLOON_PAGES_PER_PAGE);
> - /* Sleep for at least 1/5 of a second before retry. */
> - msleep(200);
> break;
> }
> set_page_pfns(vb, pfns + num_pfns, page);
> @@ -166,6 +175,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> adjust_managed_page_count(page, -1);
> }
>
> + /*
> + * The oom_mode is set, but we've already been able to get some
> + * pages, so it is time to turn it off here.
> + */
> + if (unlikely(READ_ONCE(vb->oom_mode) && page))
> + WRITE_ONCE(vb->oom_mode, false);
> +
> mutex_lock(&vb->inflate_lock);
> /* Did we get any? */
> if (num_pfns != 0)
> @@ -173,6 +189,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> mutex_unlock(&vb->inflate_lock);
> atomic64_add(num_pfns, &vb->num_pages);
>
> + /*
> + * If oom_mode is on, return the original @num passed by
> + * update_balloon_size_func to stop the inflating.
> + */
> + if (READ_ONCE(vb->oom_mode))
> + return orig_num;
> +
> return num_pfns;
> }
>
> @@ -365,6 +388,7 @@ static int virtballoon_oom_notify(struct notifier_block *self,
> if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> return NOTIFY_OK;
>
> + WRITE_ONCE(vb->oom_mode, true);
> freed = parm;
>
> /* Don't deflate more than the number of inflated pages */
> @@ -549,6 +573,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
> spin_lock_init(&vb->stop_update_lock);
> vb->stop_update = false;
> + vb->oom_mode = false;
> atomic64_set(&vb->num_pages, 0);
> mutex_init(&vb->inflate_lock);
> mutex_init(&vb->deflate_lock);
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v1 1/3] virtio-balloon: replace the coarse-grained balloon_lock
From: Tetsuo Handa @ 2017-10-22 11:50 UTC (permalink / raw)
To: wei.w.wang, mst; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <59EC7FF5.6070906@intel.com>
Wei Wang wrote:
> >> @@ -162,20 +160,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> >> msleep(200);
> >> break;
> >> }
> >> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> >> - vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> >> + set_page_pfns(vb, pfns + num_pfns, page);
> >> if (!virtio_has_feature(vb->vdev,
> >> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> >> adjust_managed_page_count(page, -1);
> >> }
> >>
> >> - num_allocated_pages = vb->num_pfns;
> >> + mutex_lock(&vb->inflate_lock);
> >> /* Did we get any? */
> >> - if (vb->num_pfns != 0)
> >> - tell_host(vb, vb->inflate_vq);
> >> - mutex_unlock(&vb->balloon_lock);
> >> + if (num_pfns != 0)
> >> + tell_host(vb, vb->inflate_vq, pfns, num_pfns);
> >> + mutex_unlock(&vb->inflate_lock);
> >> + atomic64_add(num_pfns, &vb->num_pages);
> > Isn't this addition too late? If leak_balloon() is called due to
> > out_of_memory(), it will fail to find up to dated vb->num_pages value.
>
> Not really. I think the old way of implementation above:
> "vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE"
> isn't quite accurate, because "vb->num_page" should reflect the number of
> pages that have already been inflated, which means those pages have
> already been given to the host via "tell_host()".
>
> If we update "vb->num_page" earlier before tell_host(), then it will
> include the pages
> that haven't been given to the host, which I think shouldn't be counted
> as inflated pages.
>
> On the other hand, OOM will use leak_balloon() to release the pages that
> should
> have already been inflated.
But leak_balloon() finds max inflated pages from vb->num_pages, doesn't it?
>
> >>
> >> /* We can only do one array worth at a time. */
> >> - num = min(num, ARRAY_SIZE(vb->pfns));
> >> + num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
> >>
> >> - mutex_lock(&vb->balloon_lock);
> >> /* We can't release more pages than taken */
> >> - num = min(num, (size_t)vb->num_pages);
> >> - for (vb->num_pfns = 0; vb->num_pfns < num;
> >> - vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> >> + num = min_t(size_t, num, atomic64_read(&vb->num_pages));
> >> + for (num_pfns = 0; num_pfns < num;
> >> + num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> >> page = balloon_page_dequeue(vb_dev_info);
> > If balloon_page_dequeue() can be concurrently called by both host's request
> > and guest's OOM event, is (!dequeued_page) test in balloon_page_dequeue() safe?
>
>
> I'm not sure about the question. The "dequeue_page" is a local variable
> in the function, why would it be unsafe for two invocations (the shared
> b_dev_info->pages are operated under a lock)?
I'm not MM person nor virtio person. I'm commenting from point of view of
safe programming. My question is, isn't there possibility of hitting
if (unlikely(list_empty(&b_dev_info->pages) &&
!b_dev_info->isolated_pages))
BUG();
when things run concurrently.
Wei Wang wrote:
> On 10/22/2017 12:11 PM, Tetsuo Handa wrote:
> > Michael S. Tsirkin wrote:
> >>> - num_freed_pages = leak_balloon(vb, oom_pages);
> >>> +
> >>> + /* Don't deflate more than the number of inflated pages */
> >>> + while (npages && atomic64_read(&vb->num_pages))
> >>> + npages -= leak_balloon(vb, npages);
> > don't we need to abort if leak_balloon() returned 0 for some reason?
>
> I don't think so. Returning 0 should be a normal case when the host tries
> to give back some pages to the guest, but there is no pages that have ever
> been inflated. For example, right after booting the guest, the host sends a
> deflating request to give the guest 1G memory, leak_balloon should return 0,
> and guest wouldn't get 1 more G memory.
>
My question is, isn't there possibility of leak_balloon() returning 0 for
reasons other than vb->num_pages == 0 ? If yes, this can cause infinite loop
(i.e. lockups) when things run concurrently.
^ permalink raw reply
* Re: [PATCH v1 2/3] virtio-balloon: deflate up to oom_pages on OOM
From: Wei Wang @ 2017-10-22 11:31 UTC (permalink / raw)
To: Tetsuo Handa, mst; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <201710221311.FFI17148.VStOJQLHOFFMOF@I-love.SAKURA.ne.jp>
On 10/22/2017 12:11 PM, Tetsuo Handa wrote:
> Michael S. Tsirkin wrote:
>>> - num_freed_pages = leak_balloon(vb, oom_pages);
>>> +
>>> + /* Don't deflate more than the number of inflated pages */
>>> + while (npages && atomic64_read(&vb->num_pages))
>>> + npages -= leak_balloon(vb, npages);
> don't we need to abort if leak_balloon() returned 0 for some reason?
I don't think so. Returning 0 should be a normal case when the host tries
to give back some pages to the guest, but there is no pages that have ever
been inflated. For example, right after booting the guest, the host sends a
deflating request to give the guest 1G memory, leak_balloon should return 0,
and guest wouldn't get 1 more G memory.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v1 1/3] virtio-balloon: replace the coarse-grained balloon_lock
From: Wei Wang @ 2017-10-22 11:24 UTC (permalink / raw)
To: Tetsuo Handa, mst; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <201710221420.FHG17654.OOMFQSFJVFHLtO@I-love.SAKURA.ne.jp>
On 10/22/2017 01:20 PM, Tetsuo Handa wrote:
> Wei Wang wrote:
>> The balloon_lock was used to synchronize the access demand to elements
>> of struct virtio_balloon and its queue operations (please see commit
>> e22504296d). This prevents the concurrent run of the leak_balloon and
>> fill_balloon functions, thereby resulting in a deadlock issue on OOM:
>>
>> fill_balloon: take balloon_lock and wait for OOM to get some memory;
>> oom_notify: release some inflated memory via leak_balloon();
>> leak_balloon: wait for balloon_lock to be released by fill_balloon.
>>
>> This patch breaks the lock into two fine-grained inflate_lock and
>> deflate_lock, and eliminates the unnecessary use of the shared data
>> (i.e. vb->pnfs, vb->num_pfns). This enables leak_balloon and
>> fill_balloon to run concurrently and solves the deadlock issue.
>>
>> @@ -162,20 +160,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>> msleep(200);
>> break;
>> }
>> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
>> - vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
>> + set_page_pfns(vb, pfns + num_pfns, page);
>> if (!virtio_has_feature(vb->vdev,
>> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
>> adjust_managed_page_count(page, -1);
>> }
>>
>> - num_allocated_pages = vb->num_pfns;
>> + mutex_lock(&vb->inflate_lock);
>> /* Did we get any? */
>> - if (vb->num_pfns != 0)
>> - tell_host(vb, vb->inflate_vq);
>> - mutex_unlock(&vb->balloon_lock);
>> + if (num_pfns != 0)
>> + tell_host(vb, vb->inflate_vq, pfns, num_pfns);
>> + mutex_unlock(&vb->inflate_lock);
>> + atomic64_add(num_pfns, &vb->num_pages);
> Isn't this addition too late? If leak_balloon() is called due to
> out_of_memory(), it will fail to find up to dated vb->num_pages value.
Not really. I think the old way of implementation above:
"vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE"
isn't quite accurate, because "vb->num_page" should reflect the number of
pages that have already been inflated, which means those pages have
already been given to the host via "tell_host()".
If we update "vb->num_page" earlier before tell_host(), then it will
include the pages
that haven't been given to the host, which I think shouldn't be counted
as inflated pages.
On the other hand, OOM will use leak_balloon() to release the pages that
should
have already been inflated.
In addition, I think we would also need to move balloon_page_insert(),
which puts the
page onto the inflated page list, after tell_host().
>>
>> - return num_allocated_pages;
>> + return num_pfns;
>> }
>>
>> static void release_pages_balloon(struct virtio_balloon *vb,
>> @@ -194,38 +192,39 @@ static void release_pages_balloon(struct virtio_balloon *vb,
>>
>> static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
>> {
>> - unsigned num_freed_pages;
>> struct page *page;
>> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
>> LIST_HEAD(pages);
>> + unsigned int num_pfns;
>> + __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
> This array consumes 1024 bytes of kernel stack, doesn't it?
> leak_balloon() might be called from out_of_memory() where kernel stack
> is already largely consumed before entering __alloc_pages_nodemask().
> For reducing possibility of stack overflow, since out_of_memory() is
> serialized by oom_lock, I suggest using static (maybe kmalloc()ed as
> vb->oom_pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX]) buffer when called from
> out_of_memory().
In that case, we might as well to use
vb->inflate_pfns = kmalloc(VIRTIO_BALLOON_ARRAY_PFNS_MAX..);
vb->deflate_pfns = kmalloc(VIRTIO_BALLOON_ARRAY_PFNS_MAX..);
which are allocated in probe().
>>
>> /* We can only do one array worth at a time. */
>> - num = min(num, ARRAY_SIZE(vb->pfns));
>> + num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
>>
>> - mutex_lock(&vb->balloon_lock);
>> /* We can't release more pages than taken */
>> - num = min(num, (size_t)vb->num_pages);
>> - for (vb->num_pfns = 0; vb->num_pfns < num;
>> - vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>> + num = min_t(size_t, num, atomic64_read(&vb->num_pages));
>> + for (num_pfns = 0; num_pfns < num;
>> + num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>> page = balloon_page_dequeue(vb_dev_info);
> If balloon_page_dequeue() can be concurrently called by both host's request
> and guest's OOM event, is (!dequeued_page) test in balloon_page_dequeue() safe?
I'm not sure about the question. The "dequeue_page" is a local variable
in the function, why would it be unsafe for two invocations (the shared
b_dev_info->pages are operated under a lock)?
> Is such concurrency needed?
Thanks for this question, it triggers another optimization, which I want to
introduce if this direction could be accepted:
I think it is not quite necessary to deflate pages in OOM-->leak_balloon()
when the host request leak_ballon() is running. In that case, I think OOM
can just count the pages that are deflated by the host request.
The implementation logic will be simple, here is the major part:
1) Introduce a "vb->deflating" flag, to tell whether deflating is in
progress
2) At the beginning of leak_balloon():
if (READ_ONCE(vb->deflating)) {
npages = atomic64_read(&vb->num_pages);
/* Wait till the other run of leak_balloon() returns */
while (READ_ONCE(vb->deflating));
npages = npages - atomic64_read(&vb->num_pages)
} else {
WRITE_ONCE(vb->deflating, true);
}
...
3) At the end of leak_balloon():
WRITE_ONCE(vb->deflating, false);
(The above vb->deflating doesn't have to be in vb though, it can be a
static variable inside leak_balloon(). we can
discuss more about the implementation when reaching that step)
Best,
Wei
^ permalink raw reply
* Re: [PATCH v1 0/3] Virtio-balloon Improvement
From: Wei Wang @ 2017-10-22 11:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: penguin-kernel, linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <20171022061307-mutt-send-email-mst@kernel.org>
On 10/22/2017 11:19 AM, Michael S. Tsirkin wrote:
> On Fri, Oct 20, 2017 at 07:54:23PM +0800, Wei Wang wrote:
>> This patch series intends to summarize the recent contributions made by
>> Michael S. Tsirkin, Tetsuo Handa, Michal Hocko etc. via reporting and
>> discussing the related deadlock issues on the mailinglist. Please check
>> each patch for details.
>>
>> >From a high-level point of view, this patch series achieves:
>> 1) eliminate the deadlock issue fundamentally caused by the inability
>> to run leak_balloon and fill_balloon concurrently;
> We need to think about this carefully. Is it an issue that
> leak can now bypass fill? It seems that we can now
> try to leak a page before fill was seen by host,
> but I did not look into it deeply.
>
> I really like my patch for this better at least for
> current kernel. I agree we need to work more on 2+3.
Yes, we can check more. But from the original intention:
(copied from the commit e22504296d)
balloon_lock (mutex) : synchronizes the access demand to elements of
struct virtio_balloon and its queue
operations;
This implementation has covered what balloon_lock achieves. We have
inflating and deflating decoupled and use a small lock for each vq
respectively.
I also tested inflating 20G, and before it's done, requested to
deflating 20G, all work fine.
>
>> 2) enable OOM to release more than 256 inflated pages; and
> Does just this help enough? How about my patch + 2?
> Tetsuo, what do you think?
>
>> 3) stop inflating when the guest is under severe memory pressure
>> (i.e. OOM).
> But when do we finally inflate? Question is how does host know it needs
> to resend an interrupt, and when should it do it?
I think "when to inflate again" should be a policy defined by the
orchestration
layer software on the host. A reasonable inflating request should be
sent to a
guest on the condition that this guest has enough free memory to inflate
(virtio-balloon memory stats has already supported to report that info).
If the policy defines to inflate guest memory without considering
whether the guest
is even under memory pressure. The mechanism we provide here is to offer
no pages
to the host in that case. I think this should be reasonable.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v1 1/3] virtio-balloon: replace the coarse-grained balloon_lock
From: Tetsuo Handa @ 2017-10-22 5:20 UTC (permalink / raw)
To: wei.w.wang, mst; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <1508500466-21165-2-git-send-email-wei.w.wang@intel.com>
Wei Wang wrote:
> The balloon_lock was used to synchronize the access demand to elements
> of struct virtio_balloon and its queue operations (please see commit
> e22504296d). This prevents the concurrent run of the leak_balloon and
> fill_balloon functions, thereby resulting in a deadlock issue on OOM:
>
> fill_balloon: take balloon_lock and wait for OOM to get some memory;
> oom_notify: release some inflated memory via leak_balloon();
> leak_balloon: wait for balloon_lock to be released by fill_balloon.
>
> This patch breaks the lock into two fine-grained inflate_lock and
> deflate_lock, and eliminates the unnecessary use of the shared data
> (i.e. vb->pnfs, vb->num_pfns). This enables leak_balloon and
> fill_balloon to run concurrently and solves the deadlock issue.
>
> @@ -162,20 +160,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> msleep(200);
> break;
> }
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> - vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> + set_page_pfns(vb, pfns + num_pfns, page);
> if (!virtio_has_feature(vb->vdev,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> adjust_managed_page_count(page, -1);
> }
>
> - num_allocated_pages = vb->num_pfns;
> + mutex_lock(&vb->inflate_lock);
> /* Did we get any? */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->inflate_vq);
> - mutex_unlock(&vb->balloon_lock);
> + if (num_pfns != 0)
> + tell_host(vb, vb->inflate_vq, pfns, num_pfns);
> + mutex_unlock(&vb->inflate_lock);
> + atomic64_add(num_pfns, &vb->num_pages);
Isn't this addition too late? If leak_balloon() is called due to
out_of_memory(), it will fail to find up to dated vb->num_pages value.
>
> - return num_allocated_pages;
> + return num_pfns;
> }
>
> static void release_pages_balloon(struct virtio_balloon *vb,
> @@ -194,38 +192,39 @@ static void release_pages_balloon(struct virtio_balloon *vb,
>
> static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> {
> - unsigned num_freed_pages;
> struct page *page;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> LIST_HEAD(pages);
> + unsigned int num_pfns;
> + __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
This array consumes 1024 bytes of kernel stack, doesn't it?
leak_balloon() might be called from out_of_memory() where kernel stack
is already largely consumed before entering __alloc_pages_nodemask().
For reducing possibility of stack overflow, since out_of_memory() is
serialized by oom_lock, I suggest using static (maybe kmalloc()ed as
vb->oom_pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX]) buffer when called from
out_of_memory().
>
> /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
>
> - mutex_lock(&vb->balloon_lock);
> /* We can't release more pages than taken */
> - num = min(num, (size_t)vb->num_pages);
> - for (vb->num_pfns = 0; vb->num_pfns < num;
> - vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> + num = min_t(size_t, num, atomic64_read(&vb->num_pages));
> + for (num_pfns = 0; num_pfns < num;
> + num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> page = balloon_page_dequeue(vb_dev_info);
If balloon_page_dequeue() can be concurrently called by both host's request
and guest's OOM event, is (!dequeued_page) test in balloon_page_dequeue() safe?
Is such concurrency needed?
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + set_page_pfns(vb, pfns + num_pfns, page);
> list_add(&page->lru, &pages);
> - vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
>
> - num_freed_pages = vb->num_pfns;
> /*
> * Note that if
> * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
> * is true, we *have* to do it in this order
> */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->deflate_vq);
> + mutex_lock(&vb->deflate_lock);
> + if (num_pfns != 0)
> + tell_host(vb, vb->deflate_vq, pfns, num_pfns);
> + mutex_unlock(&vb->deflate_lock);
> release_pages_balloon(vb, &pages);
> - mutex_unlock(&vb->balloon_lock);
> - return num_freed_pages;
> + atomic64_sub(num_pfns, &vb->num_pages);
Isn't this subtraction too late?
> +
> + return num_pfns;
> }
>
> static inline void update_stat(struct virtio_balloon *vb, int idx,
> @@ -465,6 +464,7 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> struct virtio_balloon *vb = container_of(vb_dev_info,
> struct virtio_balloon, vb_dev_info);
> unsigned long flags;
> + __virtio32 pfns[VIRTIO_BALLOON_PAGES_PER_PAGE];
If this is called from memory allocation path, maybe kmalloc()ed buffer is safer.
^ permalink raw reply
* Re: [PATCH v1 2/3] virtio-balloon: deflate up to oom_pages on OOM
From: Tetsuo Handa @ 2017-10-22 4:11 UTC (permalink / raw)
To: mst, wei.w.wang; +Cc: linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <20171022062119-mutt-send-email-mst@kernel.org>
Michael S. Tsirkin wrote:
> On Fri, Oct 20, 2017 at 07:54:25PM +0800, Wei Wang wrote:
> > The current implementation only deflates 256 pages even when a user
> > specifies more than that via the oom_pages module param. This patch
> > enables the deflating of up to oom_pages pages if there are enough
> > inflated pages.
>
> This seems reasonable. Does this by itself help?
At least
> > - num_freed_pages = leak_balloon(vb, oom_pages);
> > +
> > + /* Don't deflate more than the number of inflated pages */
> > + while (npages && atomic64_read(&vb->num_pages))
> > + npages -= leak_balloon(vb, npages);
don't we need to abort if leak_balloon() returned 0 for some reason?
^ permalink raw reply
* Re: [PATCH v1 2/3] virtio-balloon: deflate up to oom_pages on OOM
From: Michael S. Tsirkin @ 2017-10-22 3:21 UTC (permalink / raw)
To: Wei Wang; +Cc: penguin-kernel, linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <1508500466-21165-3-git-send-email-wei.w.wang@intel.com>
On Fri, Oct 20, 2017 at 07:54:25PM +0800, Wei Wang wrote:
> The current implementation only deflates 256 pages even when a user
> specifies more than that via the oom_pages module param. This patch
> enables the deflating of up to oom_pages pages if there are enough
> inflated pages.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
This seems reasonable. Does this by itself help?
> ---
> drivers/virtio/virtio_balloon.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 1ecd15a..ab55cf8 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -43,8 +43,8 @@
> #define OOM_VBALLOON_DEFAULT_PAGES 256
> #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
>
> -static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> -module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> +static unsigned int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> +module_param(oom_pages, uint, 0600);
> MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
>
> #ifdef CONFIG_BALLOON_COMPACTION
> @@ -359,16 +359,20 @@ static int virtballoon_oom_notify(struct notifier_block *self,
> {
> struct virtio_balloon *vb;
> unsigned long *freed;
> - unsigned num_freed_pages;
> + unsigned int npages = oom_pages;
>
> vb = container_of(self, struct virtio_balloon, nb);
> if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> return NOTIFY_OK;
>
> freed = parm;
> - num_freed_pages = leak_balloon(vb, oom_pages);
> +
> + /* Don't deflate more than the number of inflated pages */
> + while (npages && atomic64_read(&vb->num_pages))
> + npages -= leak_balloon(vb, npages);
> +
> update_balloon_size(vb);
> - *freed += num_freed_pages;
> + *freed += oom_pages - npages;
>
> return NOTIFY_OK;
> }
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v1 0/3] Virtio-balloon Improvement
From: Michael S. Tsirkin @ 2017-10-22 3:19 UTC (permalink / raw)
To: Wei Wang; +Cc: penguin-kernel, linux-mm, virtualization, linux-kernel, mhocko
In-Reply-To: <1508500466-21165-1-git-send-email-wei.w.wang@intel.com>
On Fri, Oct 20, 2017 at 07:54:23PM +0800, Wei Wang wrote:
> This patch series intends to summarize the recent contributions made by
> Michael S. Tsirkin, Tetsuo Handa, Michal Hocko etc. via reporting and
> discussing the related deadlock issues on the mailinglist. Please check
> each patch for details.
>
> >From a high-level point of view, this patch series achieves:
> 1) eliminate the deadlock issue fundamentally caused by the inability
> to run leak_balloon and fill_balloon concurrently;
We need to think about this carefully. Is it an issue that
leak can now bypass fill? It seems that we can now
try to leak a page before fill was seen by host,
but I did not look into it deeply.
I really like my patch for this better at least for
current kernel. I agree we need to work more on 2+3.
> 2) enable OOM to release more than 256 inflated pages; and
Does just this help enough? How about my patch + 2?
Tetsuo, what do you think?
> 3) stop inflating when the guest is under severe memory pressure
> (i.e. OOM).
But when do we finally inflate? Question is how does host know it needs
to resend an interrupt, and when should it do it?
> Here is an example of the benefit brought by this patch series:
> The guest sets virtio_balloon.oom_pages=100000. When the host requests
> to inflate 7.9G of an 8G idle guest, the guest can still run normally
> since OOM can guarantee at least 100000 pages (400MB) for the guest.
> Without the above patches, the guest will kill all the killable
> processes and fall into kernel panic finally.
>
> Wei Wang (3):
> virtio-balloon: replace the coarse-grained balloon_lock
> virtio-balloon: deflate up to oom_pages on OOM
> virtio-balloon: stop inflating when OOM occurs
>
> drivers/virtio/virtio_balloon.c | 149 ++++++++++++++++++++++++----------------
> 1 file changed, 91 insertions(+), 58 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply
* Call for papers - WorldCIST'18 - Naples, Italy
From: ML @ 2017-10-21 20:59 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 8035 bytes --]
* Extended versions of best selected papers will be published in JCR/SCI/SSCI journals
---------------------------------------------------------------------------------------------------
WorldCist'18 - 6th World Conference on Information Systems and Technologies
Naples, Italy, 27 - 29 March 2018
http://www.worldcist.org/
-----------------------------------------------------------------------------------------------------
SCOPE
The WorldCist'18 - 6th World Conference on Information Systems and Technologies (http://www.worldcist.org/), to be held at Naples, Italy, 27 - 29 March 2018, is a global forum for researchers and practitioners to present and discuss the most recent innovations, trends, results, experiences and concerns in the several perspectives of Information Systems and Technologies.
We are pleased to invite you to submit your papers to WorldCist'18. All submissions will be reviewed on the basis of relevance, originality, importance and clarity.
THEMES
Submitted papers should be related with one or more of the main themes proposed for the Conference:
A) Information and Knowledge Management (IKM);
B) Organizational Models and Information Systems (OMIS);
C) Software and Systems Modeling (SSM);
D) Software Systems, Architectures, Applications and Tools (SSAAT);
E) Multimedia Systems and Applications (MSA);
F) Computer Networks, Mobility and Pervasive Systems (CNMPS);
G) Intelligent and Decision Support Systems (IDSS);
H) Big Data Analytics and Applications (BDAA);
I) Human-Computer Interaction (HCI);
J) Ethics, Computers and Security (ECS)
K) Health Informatics (HIS);
L) Information Technologies in Education (ITE);
M) Information Technologies in Radiocommunications (ITR).
N) Technologies for Biomedical Applications (TBA)
TYPES of SUBMISSIONS and DECISIONS
Types of Submissions and Decisions
Four types of papers can be submitted:
Full paper: Finished or consolidated R&D works, to be included in one of the Conference themes. These papers are assigned a 10-page limit.
Short paper: Ongoing works with relevant preliminary results, open to discussion. These papers are assigned a 7-page limit.
Poster paper: Initial work with relevant ideas, open to discussion. These papers are assigned to a 4-page limit.
Company paper: Companies' papers that show practical experience, R & D, tools, etc., focused on some topics of the conference. These papers are assigned to a 4-page limit.
Submitted papers must comply with the format of Advances in Intelligent Systems and Computing Series (see Instructions for Authors at Springer Website or download a DOC example) be written in English, must not have been published before, not be under review for any other conference or publication and not include any information leading to the authors’ identification. Therefore, the authors’ names, affiliations and bibliographic references should not be included in the version for evaluation by the Program Committee. This information should only be included in the camera-ready version, saved in Word or Latex format and also in PDF format. These files must be accompanied by the Consent to Publication form filled out, in a ZIP file, and uploaded at the conference management system.
All papers will be subjected to a “double-blind review” by at least two members of the Program Committee.
Based on Program Committee evaluation, a paper can be rejected or accepted by the Conference Chairs. In the later case, it can be accepted as the type originally submitted or as another type. Thus, full papers can be accepted as short papers or poster papers only. Similarly, short papers can be accepted as poster papers only. In these cases, the authors will be allowed to maintain the original number of pages in the camera-ready version.
The authors of accepted poster papers must also build and print a poster to be exhibited during the Conference. This poster must follow an A1 or A2 vertical format. The Conference can includes Work Sessions where these posters are presented and orally discussed, with a 5 minute limit per poster.
The authors of accepted full papers will have 15 minutes to present their work in a Conference Work Session; approximately 5 minutes of discussion will follow each presentation. The authors of accepted short papers and company papers will have 11 minutes to present their work in a Conference Work Session; approximately 4 minutes of discussion will follow each presentation.
PUBLICATION & INDEXING
To ensure that a full paper, short paper, poster paper or company paper is published, at least one of the authors must be fully registered by the 7th of January 2018, and the paper must comply with the suggested layout and page-limit. Additionally, all recommended changes must be addressed by the authors before they submit the camera-ready version.
No more than one paper per registration will be published. An extra fee must be paid for publication of additional papers, with a maximum of one additional paper per registration. One registration permits only the participation of one author in the conference.
Full and short papers will be published in Proceedings by Springer, in Advances in Intelligent Systems and Computing Series. Poster and company papers will be published by AISTI.
Published full and short papers will be submitted for indexation by ISI, EI-Compendex, SCOPUS, DBLP and Google Scholar, among others, and will be available in the SpringerLink Digital Library.
The authors of the best selected papers will be invited to extend them for publication in international journals indexed by ISI/SCI/SSCI, SCOPUS and DBLP, among others, such as:
- International Journal of Neural Systems (IF: 6.333 / Q1)
- Integrated Computer-Aided Engineering (IF: 5.264 / Q1)
- Omega - The International Journal of Management Science (IF: 4.029 / Q1)
- Future Generation Computer Systems (IF: 3.997 / Q1)
- International Journal of Information Management (IF: 3.872 / Q1)
- Telematics and Informatics (IF: 3.398 / Q1)
- Journal of Grid Computing (IF: 2.766 / Q1)
- Ethics and Information Technology (IF: 1.500 / Q1)
- Journal of Medical Systems (IF: 2.456 / Q2)
- Computer Languages, Systems & Structures (IF: 1.615 / Q2)
- International Journal of Critical Infrastructure Protection (IF: 1.5 / Q2)
- Informatica - An International Journal (IF: 1.052 / Q2)
- Annals of Telecommunications (IF: 1.412 / Q3)
- Journal of Intelligent & Fuzzy Systems (IF: 1.261 / Q3)
- International Journal of Computers Communications & Control (IF: 1.374 / Q3)
- Expert Systems - Journal of Knowledge Engineering (IF: 1.18 / Q3)
- Computational and Mathematical Organization Theory (IF: 0.769 / Q3)
- Program - Electronic Library and Information Systems (IF: 0.556 / Q3)
- Intelligent Service Robotics (IF: 0.875 / Q4)
- Studies in Informatics and Control (IF: 0.776 / Q4)
- Computing and Informatics (IF: 0.488 / Q4)
- Information Technology and Control (IF: 0.475 / Q4)
- Journal of Database Management (IF: 0.462 / Q4)
- Romanian Journal of Information Science and Technology (IF: 0.365 / Q4)
- Computer Methods in Biomechanics and Biomedical Engineering - Imaging & Visualization (ISI - Emerging Sources Citation Index)
- Journal of Information Systems Engineering & Management
IMPORTANT DATES
Paper Submission: November 12, 2017
Notification of Acceptance: December 26, 2017
Payment of Registration, to ensure the inclusion of an accepted paper in the conference proceedings: January 7, 2018.
Camera-ready Submission: January 7, 2018
---
WorldCIST'18 website: http://www.worldcist.org/
------
---
PS: If you do not wish to receive any more notices from WorldCIST just reply to this message with the word REMOVE in the subject line.
---
This email has been checked for viruses by AVG.
http://www.avg.com
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v1 06/27] x86/entry/64: Adapt assembly for PIE support
From: Andy Lutomirski @ 2017-10-20 17:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
linux-doc, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr , Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Thomas Gleixner, the arch/x86 maintainers,
Herbert Xu, Daniel Borkmann
In-Reply-To: <20171020152028.syq6woeet6it3z3h@gmail.com>
> On Oct 20, 2017, at 5:20 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Thomas Garnier <thgarnie@google.com> wrote:
>
>>>> */
>>>> - cmpq $.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
>>>> + leaq .Lentry_SYSCALL_64_after_fastpath_call(%rip), %r11
>>>> + cmpq %r11, (%rsp)
>>>> jne 1f
>
>>> This patch seems to add extra overhead to the syscall fast-path even when PIE is
>>> disabled, right?
>>
>> It does add extra instructions when one is not possible, I preferred
>> that over ifdefing but I can change it.
>
> So my problem is, this pattern repeats in many other places as well, but sprinking
> various pieces of assembly code with #ifdefs would be very bad as well.
>
> I have no good idea how to solve this.
>
> Thanks,
Ugh, brain was off. This is a bit messy. We could use a macro for this, too, I suppose.
>
> Ingo
^ permalink raw reply
* Re: [PATCH v1 06/27] x86/entry/64: Adapt assembly for PIE support
From: Andy Lutomirski @ 2017-10-20 16:27 UTC (permalink / raw)
To: Ingo Molnar
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
linux-doc, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr , Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Thomas Gleixner, the arch/x86 maintainers,
Herbert Xu, Daniel Borkmann
In-Reply-To: <20171020152028.syq6woeet6it3z3h@gmail.com>
> On Oct 20, 2017, at 5:20 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Thomas Garnier <thgarnie@google.com> wrote:
>
>>>> */
>>>> - cmpq $.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
>>>> + leaq .Lentry_SYSCALL_64_after_fastpath_call(%rip), %r11
>>>> + cmpq %r11, (%rsp)
>>>> jne 1f
>
>>> This patch seems to add extra overhead to the syscall fast-path even when PIE is
>>> disabled, right?
>>
>> It does add extra instructions when one is not possible, I preferred
>> that over ifdefing but I can change it.
>
> So my problem is, this pattern repeats in many other places as well, but sprinking
> various pieces of assembly code with #ifdefs would be very bad as well.
>
> I have no good idea how to solve this.
>
How about:
.macro JMP_TO_LABEL ...
> Thanks,
>
> Ingo
^ permalink raw reply
* Re: [PATCH v1 06/27] x86/entry/64: Adapt assembly for PIE support
From: Ingo Molnar @ 2017-10-20 15:20 UTC (permalink / raw)
To: Thomas Garnier
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
linux-doc, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr, Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Thomas Gleixner, the arch/x86 maintainers,
Herbert Xu, Daniel Borkmann
In-Reply-To: <CAJcbSZFScsqOORMGXFQdsqcN5xbfHWpzSHGdxQB=45zgSDryLw@mail.gmail.com>
* Thomas Garnier <thgarnie@google.com> wrote:
> >> */
> >> - cmpq $.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
> >> + leaq .Lentry_SYSCALL_64_after_fastpath_call(%rip), %r11
> >> + cmpq %r11, (%rsp)
> >> jne 1f
> > This patch seems to add extra overhead to the syscall fast-path even when PIE is
> > disabled, right?
>
> It does add extra instructions when one is not possible, I preferred
> that over ifdefing but I can change it.
So my problem is, this pattern repeats in many other places as well, but sprinking
various pieces of assembly code with #ifdefs would be very bad as well.
I have no good idea how to solve this.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v1 01/27] x86/crypto: Adapt assembly for PIE support
From: Thomas Garnier via Virtualization @ 2017-10-20 14:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
Linux Doc Mailing List, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr, Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Ingo Molnar, x86@kernel.org, Herbert Xu,
Daniel Borkmann
In-Reply-To: <CAKv+Gu9XMnNA0UoGfFMQmC9=Ryh6dcOduxH+tq49bcdvBwhyQw@mail.gmail.com>
On Fri, Oct 20, 2017 at 1:28 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 20 October 2017 at 09:24, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> * Thomas Garnier <thgarnie@google.com> wrote:
>>
>>> Change the assembly code to use only relative references of symbols for the
>>> kernel to be PIE compatible.
>>>
>>> Position Independent Executable (PIE) support will allow to extended the
>>> KASLR randomization range below the -2G memory limit.
>>
>>> diff --git a/arch/x86/crypto/aes-x86_64-asm_64.S b/arch/x86/crypto/aes-x86_64-asm_64.S
>>> index 8739cf7795de..86fa068e5e81 100644
>>> --- a/arch/x86/crypto/aes-x86_64-asm_64.S
>>> +++ b/arch/x86/crypto/aes-x86_64-asm_64.S
>>> @@ -48,8 +48,12 @@
>>> #define R10 %r10
>>> #define R11 %r11
>>>
>>> +/* Hold global for PIE suport */
>>> +#define RBASE %r12
>>> +
>>> #define prologue(FUNC,KEY,B128,B192,r1,r2,r5,r6,r7,r8,r9,r10,r11) \
>>> ENTRY(FUNC); \
>>> + pushq RBASE; \
>>> movq r1,r2; \
>>> leaq KEY+48(r8),r9; \
>>> movq r10,r11; \
>>> @@ -74,54 +78,63 @@
>>> movl r6 ## E,4(r9); \
>>> movl r7 ## E,8(r9); \
>>> movl r8 ## E,12(r9); \
>>> + popq RBASE; \
>>> ret; \
>>> ENDPROC(FUNC);
>>>
>>> +#define round_mov(tab_off, reg_i, reg_o) \
>>> + leaq tab_off(%rip), RBASE; \
>>> + movl (RBASE,reg_i,4), reg_o;
>>> +
>>> +#define round_xor(tab_off, reg_i, reg_o) \
>>> + leaq tab_off(%rip), RBASE; \
>>> + xorl (RBASE,reg_i,4), reg_o;
>>> +
>>> #define round(TAB,OFFSET,r1,r2,r3,r4,r5,r6,r7,r8,ra,rb,rc,rd) \
>>> movzbl r2 ## H,r5 ## E; \
>>> movzbl r2 ## L,r6 ## E; \
>>> - movl TAB+1024(,r5,4),r5 ## E;\
>>> + round_mov(TAB+1024, r5, r5 ## E)\
>>> movw r4 ## X,r2 ## X; \
>>> - movl TAB(,r6,4),r6 ## E; \
>>> + round_mov(TAB, r6, r6 ## E) \
>>> roll $16,r2 ## E; \
>>> shrl $16,r4 ## E; \
>>> movzbl r4 ## L,r7 ## E; \
>>> movzbl r4 ## H,r4 ## E; \
>>> xorl OFFSET(r8),ra ## E; \
>>> xorl OFFSET+4(r8),rb ## E; \
>>> - xorl TAB+3072(,r4,4),r5 ## E;\
>>> - xorl TAB+2048(,r7,4),r6 ## E;\
>>> + round_xor(TAB+3072, r4, r5 ## E)\
>>> + round_xor(TAB+2048, r7, r6 ## E)\
>>> movzbl r1 ## L,r7 ## E; \
>>> movzbl r1 ## H,r4 ## E; \
>>> - movl TAB+1024(,r4,4),r4 ## E;\
>>> + round_mov(TAB+1024, r4, r4 ## E)\
>>> movw r3 ## X,r1 ## X; \
>>> roll $16,r1 ## E; \
>>> shrl $16,r3 ## E; \
>>> - xorl TAB(,r7,4),r5 ## E; \
>>> + round_xor(TAB, r7, r5 ## E) \
>>> movzbl r3 ## L,r7 ## E; \
>>> movzbl r3 ## H,r3 ## E; \
>>> - xorl TAB+3072(,r3,4),r4 ## E;\
>>> - xorl TAB+2048(,r7,4),r5 ## E;\
>>> + round_xor(TAB+3072, r3, r4 ## E)\
>>> + round_xor(TAB+2048, r7, r5 ## E)\
>>> movzbl r1 ## L,r7 ## E; \
>>> movzbl r1 ## H,r3 ## E; \
>>> shrl $16,r1 ## E; \
>>> - xorl TAB+3072(,r3,4),r6 ## E;\
>>> - movl TAB+2048(,r7,4),r3 ## E;\
>>> + round_xor(TAB+3072, r3, r6 ## E)\
>>> + round_mov(TAB+2048, r7, r3 ## E)\
>>> movzbl r1 ## L,r7 ## E; \
>>> movzbl r1 ## H,r1 ## E; \
>>> - xorl TAB+1024(,r1,4),r6 ## E;\
>>> - xorl TAB(,r7,4),r3 ## E; \
>>> + round_xor(TAB+1024, r1, r6 ## E)\
>>> + round_xor(TAB, r7, r3 ## E) \
>>> movzbl r2 ## H,r1 ## E; \
>>> movzbl r2 ## L,r7 ## E; \
>>> shrl $16,r2 ## E; \
>>> - xorl TAB+3072(,r1,4),r3 ## E;\
>>> - xorl TAB+2048(,r7,4),r4 ## E;\
>>> + round_xor(TAB+3072, r1, r3 ## E)\
>>> + round_xor(TAB+2048, r7, r4 ## E)\
>>> movzbl r2 ## H,r1 ## E; \
>>> movzbl r2 ## L,r2 ## E; \
>>> xorl OFFSET+8(r8),rc ## E; \
>>> xorl OFFSET+12(r8),rd ## E; \
>>> - xorl TAB+1024(,r1,4),r3 ## E;\
>>> - xorl TAB(,r2,4),r4 ## E;
>>> + round_xor(TAB+1024, r1, r3 ## E)\
>>> + round_xor(TAB, r2, r4 ## E)
>>
>> This appears to be adding unconditional overhead to a function that was moved to
>> assembly to improve its performance.
>>
It adds couple extra instructions, how much overhead it creates is
hard for me to tell. It would increase the code complexity if
everything is ifdef.
>
> I did some benchmarking on this code a while ago and, interestingly,
> it was slower than the generic C implementation (on a Pentium E2200),
> so we may want to consider whether we still need this driver in the
> first place.
Interesting.
--
Thomas
^ permalink raw reply
* Re: [PATCH v1 06/27] x86/entry/64: Adapt assembly for PIE support
From: Thomas Garnier via Virtualization @ 2017-10-20 14:47 UTC (permalink / raw)
To: Ingo Molnar
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
linux-doc, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr, Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Thomas Gleixner, the arch/x86 maintainers,
Herbert Xu, Daniel Borkmann
In-Reply-To: <20171020082646.bkxrps35sb3gq2nr@gmail.com>
On Fri, Oct 20, 2017 at 1:26 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Thomas Garnier <thgarnie@google.com> wrote:
>
>> Change the assembly code to use only relative references of symbols for the
>> kernel to be PIE compatible.
>>
>> Position Independent Executable (PIE) support will allow to extended the
>> KASLR randomization range below the -2G memory limit.
>>
>> Signed-off-by: Thomas Garnier <thgarnie@google.com>
>> ---
>> arch/x86/entry/entry_64.S | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index 49167258d587..15bd5530d2ae 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -194,12 +194,15 @@ entry_SYSCALL_64_fastpath:
>> ja 1f /* return -ENOSYS (already in pt_regs->ax) */
>> movq %r10, %rcx
>>
>> + /* Ensures the call is position independent */
>> + leaq sys_call_table(%rip), %r11
>> +
>> /*
>> * This call instruction is handled specially in stub_ptregs_64.
>> * It might end up jumping to the slow path. If it jumps, RAX
>> * and all argument registers are clobbered.
>> */
>> - call *sys_call_table(, %rax, 8)
>> + call *(%r11, %rax, 8)
>> .Lentry_SYSCALL_64_after_fastpath_call:
>>
>> movq %rax, RAX(%rsp)
>> @@ -334,7 +337,8 @@ ENTRY(stub_ptregs_64)
>> * RAX stores a pointer to the C function implementing the syscall.
>> * IRQs are on.
>> */
>> - cmpq $.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
>> + leaq .Lentry_SYSCALL_64_after_fastpath_call(%rip), %r11
>> + cmpq %r11, (%rsp)
>> jne 1f
>>
>> /*
>> @@ -1172,7 +1176,8 @@ ENTRY(error_entry)
>> movl %ecx, %eax /* zero extend */
>> cmpq %rax, RIP+8(%rsp)
>> je .Lbstep_iret
>> - cmpq $.Lgs_change, RIP+8(%rsp)
>> + leaq .Lgs_change(%rip), %rcx
>> + cmpq %rcx, RIP+8(%rsp)
>> jne .Lerror_entry_done
>>
>> /*
>> @@ -1383,10 +1388,10 @@ ENTRY(nmi)
>> * resume the outer NMI.
>> */
>>
>> - movq $repeat_nmi, %rdx
>> + leaq repeat_nmi(%rip), %rdx
>> cmpq 8(%rsp), %rdx
>> ja 1f
>> - movq $end_repeat_nmi, %rdx
>> + leaq end_repeat_nmi(%rip), %rdx
>> cmpq 8(%rsp), %rdx
>> ja nested_nmi_out
>> 1:
>> @@ -1440,7 +1445,8 @@ nested_nmi:
>> pushq %rdx
>> pushfq
>> pushq $__KERNEL_CS
>> - pushq $repeat_nmi
>> + leaq repeat_nmi(%rip), %rdx
>> + pushq %rdx
>>
>> /* Put stack back */
>> addq $(6*8), %rsp
>> @@ -1479,7 +1485,9 @@ first_nmi:
>> addq $8, (%rsp) /* Fix up RSP */
>> pushfq /* RFLAGS */
>> pushq $__KERNEL_CS /* CS */
>> - pushq $1f /* RIP */
>> + pushq %rax /* Support Position Independent Code */
>> + leaq 1f(%rip), %rax /* RIP */
>> + xchgq %rax, (%rsp) /* Restore RAX, put 1f */
>> INTERRUPT_RETURN /* continues at repeat_nmi below */
>> UNWIND_HINT_IRET_REGS
>
> This patch seems to add extra overhead to the syscall fast-path even when PIE is
> disabled, right?
It does add extra instructions when one is not possible, I preferred
that over ifdefing but I can change it.
>
> Thanks,
>
> Ingo
--
Thomas
^ permalink raw reply
* [PATCH v1 3/3] virtio-balloon: stop inflating when OOM occurs
From: Wei Wang @ 2017-10-20 11:54 UTC (permalink / raw)
To: mst, penguin-kernel, mhocko, linux-kernel, linux-mm,
virtualization
In-Reply-To: <1508500466-21165-1-git-send-email-wei.w.wang@intel.com>
This patch forces the cease of the inflating work when OOM occurs.
The fundamental idea of memory ballooning is to take out some guest
pages when the guest has low memory utilization, so it is sensible to
inflate nothing when the guest is already under memory pressure.
On the other hand, the policy is determined by the admin or the
orchestration layer from the host. That is, the host is expected to
re-start the memory inflating request at a proper time later when
the guest has enough memory to inflate, for example, by checking
the memory stats reported by the balloon. If another inflating
requests is sent to guest when the guest is still under memory
pressure, still no pages will be inflated.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michal Hocko <mhocko@kernel.org>
---
drivers/virtio/virtio_balloon.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index ab55cf8..cf29663 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -63,6 +63,15 @@ struct virtio_balloon {
spinlock_t stop_update_lock;
bool stop_update;
+ /*
+ * The balloon driver enters the oom mode if the oom notifier is
+ * invoked. Entering the oom mode will force the exit of current
+ * inflating work. When a later inflating request is received from
+ * the host, the success of memory allocation via balloon_page_enqueue
+ * will turn off the mode.
+ */
+ bool oom_mode;
+
/* Waiting for host to ack the pages we released. */
wait_queue_head_t acked;
@@ -142,22 +151,22 @@ static void set_page_pfns(struct virtio_balloon *vb,
static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
{
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+ struct page *page;
+ size_t orig_num;
unsigned int num_pfns;
__virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
+ orig_num = num;
/* We can only do one array worth at a time. */
num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
for (num_pfns = 0; num_pfns < num;
num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
- struct page *page = balloon_page_enqueue(vb_dev_info);
-
+ page = balloon_page_enqueue(vb_dev_info);
if (!page) {
dev_info_ratelimited(&vb->vdev->dev,
"Out of puff! Can't get %u pages\n",
VIRTIO_BALLOON_PAGES_PER_PAGE);
- /* Sleep for at least 1/5 of a second before retry. */
- msleep(200);
break;
}
set_page_pfns(vb, pfns + num_pfns, page);
@@ -166,6 +175,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
adjust_managed_page_count(page, -1);
}
+ /*
+ * The oom_mode is set, but we've already been able to get some
+ * pages, so it is time to turn it off here.
+ */
+ if (unlikely(READ_ONCE(vb->oom_mode) && page))
+ WRITE_ONCE(vb->oom_mode, false);
+
mutex_lock(&vb->inflate_lock);
/* Did we get any? */
if (num_pfns != 0)
@@ -173,6 +189,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
mutex_unlock(&vb->inflate_lock);
atomic64_add(num_pfns, &vb->num_pages);
+ /*
+ * If oom_mode is on, return the original @num passed by
+ * update_balloon_size_func to stop the inflating.
+ */
+ if (READ_ONCE(vb->oom_mode))
+ return orig_num;
+
return num_pfns;
}
@@ -365,6 +388,7 @@ static int virtballoon_oom_notify(struct notifier_block *self,
if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
return NOTIFY_OK;
+ WRITE_ONCE(vb->oom_mode, true);
freed = parm;
/* Don't deflate more than the number of inflated pages */
@@ -549,6 +573,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
spin_lock_init(&vb->stop_update_lock);
vb->stop_update = false;
+ vb->oom_mode = false;
atomic64_set(&vb->num_pages, 0);
mutex_init(&vb->inflate_lock);
mutex_init(&vb->deflate_lock);
--
2.7.4
^ permalink raw reply related
* [PATCH v1 2/3] virtio-balloon: deflate up to oom_pages on OOM
From: Wei Wang @ 2017-10-20 11:54 UTC (permalink / raw)
To: mst, penguin-kernel, mhocko, linux-kernel, linux-mm,
virtualization
In-Reply-To: <1508500466-21165-1-git-send-email-wei.w.wang@intel.com>
The current implementation only deflates 256 pages even when a user
specifies more than that via the oom_pages module param. This patch
enables the deflating of up to oom_pages pages if there are enough
inflated pages.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/virtio/virtio_balloon.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 1ecd15a..ab55cf8 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -43,8 +43,8 @@
#define OOM_VBALLOON_DEFAULT_PAGES 256
#define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
-static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
-module_param(oom_pages, int, S_IRUSR | S_IWUSR);
+static unsigned int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
+module_param(oom_pages, uint, 0600);
MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
#ifdef CONFIG_BALLOON_COMPACTION
@@ -359,16 +359,20 @@ static int virtballoon_oom_notify(struct notifier_block *self,
{
struct virtio_balloon *vb;
unsigned long *freed;
- unsigned num_freed_pages;
+ unsigned int npages = oom_pages;
vb = container_of(self, struct virtio_balloon, nb);
if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
return NOTIFY_OK;
freed = parm;
- num_freed_pages = leak_balloon(vb, oom_pages);
+
+ /* Don't deflate more than the number of inflated pages */
+ while (npages && atomic64_read(&vb->num_pages))
+ npages -= leak_balloon(vb, npages);
+
update_balloon_size(vb);
- *freed += num_freed_pages;
+ *freed += oom_pages - npages;
return NOTIFY_OK;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v1 1/3] virtio-balloon: replace the coarse-grained balloon_lock
From: Wei Wang @ 2017-10-20 11:54 UTC (permalink / raw)
To: mst, penguin-kernel, mhocko, linux-kernel, linux-mm,
virtualization
In-Reply-To: <1508500466-21165-1-git-send-email-wei.w.wang@intel.com>
The balloon_lock was used to synchronize the access demand to elements
of struct virtio_balloon and its queue operations (please see commit
e22504296d). This prevents the concurrent run of the leak_balloon and
fill_balloon functions, thereby resulting in a deadlock issue on OOM:
fill_balloon: take balloon_lock and wait for OOM to get some memory;
oom_notify: release some inflated memory via leak_balloon();
leak_balloon: wait for balloon_lock to be released by fill_balloon.
This patch breaks the lock into two fine-grained inflate_lock and
deflate_lock, and eliminates the unnecessary use of the shared data
(i.e. vb->pnfs, vb->num_pfns). This enables leak_balloon and
fill_balloon to run concurrently and solves the deadlock issue.
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michal Hocko <mhocko@kernel.org>
---
drivers/virtio/virtio_balloon.c | 102 +++++++++++++++++++++-------------------
1 file changed, 53 insertions(+), 49 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index f0b3a0b..1ecd15a 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -67,7 +67,7 @@ struct virtio_balloon {
wait_queue_head_t acked;
/* Number of balloon pages we've told the Host we're not using. */
- unsigned int num_pages;
+ atomic64_t num_pages;
/*
* The pages we've told the Host we're not using are enqueued
* at vb_dev_info->pages list.
@@ -76,12 +76,9 @@ struct virtio_balloon {
*/
struct balloon_dev_info vb_dev_info;
- /* Synchronize access/update to this struct virtio_balloon elements */
- struct mutex balloon_lock;
-
- /* The array of pfns we tell the Host about. */
- unsigned int num_pfns;
- __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
+ /* Synchronize access to inflate_vq and deflate_vq respectively */
+ struct mutex inflate_lock;
+ struct mutex deflate_lock;
/* Memory statistics */
struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
@@ -111,12 +108,13 @@ static void balloon_ack(struct virtqueue *vq)
wake_up(&vb->acked);
}
-static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
+static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq,
+ __virtio32 pfns[], unsigned int num_pfns)
{
struct scatterlist sg;
unsigned int len;
- sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
+ sg_init_one(&sg, pfns, sizeof(pfns[0]) * num_pfns);
/* We should always be able to add one buffer to an empty queue. */
virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
@@ -144,14 +142,14 @@ static void set_page_pfns(struct virtio_balloon *vb,
static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
{
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
- unsigned num_allocated_pages;
+ unsigned int num_pfns;
+ __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
/* We can only do one array worth at a time. */
- num = min(num, ARRAY_SIZE(vb->pfns));
+ num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
- mutex_lock(&vb->balloon_lock);
- for (vb->num_pfns = 0; vb->num_pfns < num;
- vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
+ for (num_pfns = 0; num_pfns < num;
+ num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
struct page *page = balloon_page_enqueue(vb_dev_info);
if (!page) {
@@ -162,20 +160,20 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
msleep(200);
break;
}
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
- vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
+ set_page_pfns(vb, pfns + num_pfns, page);
if (!virtio_has_feature(vb->vdev,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
adjust_managed_page_count(page, -1);
}
- num_allocated_pages = vb->num_pfns;
+ mutex_lock(&vb->inflate_lock);
/* Did we get any? */
- if (vb->num_pfns != 0)
- tell_host(vb, vb->inflate_vq);
- mutex_unlock(&vb->balloon_lock);
+ if (num_pfns != 0)
+ tell_host(vb, vb->inflate_vq, pfns, num_pfns);
+ mutex_unlock(&vb->inflate_lock);
+ atomic64_add(num_pfns, &vb->num_pages);
- return num_allocated_pages;
+ return num_pfns;
}
static void release_pages_balloon(struct virtio_balloon *vb,
@@ -194,38 +192,39 @@ static void release_pages_balloon(struct virtio_balloon *vb,
static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
{
- unsigned num_freed_pages;
struct page *page;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
LIST_HEAD(pages);
+ unsigned int num_pfns;
+ __virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
/* We can only do one array worth at a time. */
- num = min(num, ARRAY_SIZE(vb->pfns));
+ num = min_t(size_t, num, VIRTIO_BALLOON_ARRAY_PFNS_MAX);
- mutex_lock(&vb->balloon_lock);
/* We can't release more pages than taken */
- num = min(num, (size_t)vb->num_pages);
- for (vb->num_pfns = 0; vb->num_pfns < num;
- vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
+ num = min_t(size_t, num, atomic64_read(&vb->num_pages));
+ for (num_pfns = 0; num_pfns < num;
+ num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
page = balloon_page_dequeue(vb_dev_info);
if (!page)
break;
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ set_page_pfns(vb, pfns + num_pfns, page);
list_add(&page->lru, &pages);
- vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
- num_freed_pages = vb->num_pfns;
/*
* Note that if
* virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
* is true, we *have* to do it in this order
*/
- if (vb->num_pfns != 0)
- tell_host(vb, vb->deflate_vq);
+ mutex_lock(&vb->deflate_lock);
+ if (num_pfns != 0)
+ tell_host(vb, vb->deflate_vq, pfns, num_pfns);
+ mutex_unlock(&vb->deflate_lock);
release_pages_balloon(vb, &pages);
- mutex_unlock(&vb->balloon_lock);
- return num_freed_pages;
+ atomic64_sub(num_pfns, &vb->num_pages);
+
+ return num_pfns;
}
static inline void update_stat(struct virtio_balloon *vb, int idx,
@@ -327,12 +326,12 @@ static inline s64 towards_target(struct virtio_balloon *vb)
num_pages = le32_to_cpu((__force __le32)num_pages);
target = num_pages;
- return target - vb->num_pages;
+ return target - atomic64_read(&vb->num_pages);
}
static void update_balloon_size(struct virtio_balloon *vb)
{
- u32 actual = vb->num_pages;
+ u32 actual = atomic64_read(&vb->num_pages);
/* Legacy balloon config space is LE, unlike all other devices. */
if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
@@ -465,6 +464,7 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
struct virtio_balloon *vb = container_of(vb_dev_info,
struct virtio_balloon, vb_dev_info);
unsigned long flags;
+ __virtio32 pfns[VIRTIO_BALLOON_PAGES_PER_PAGE];
/*
* In order to avoid lock contention while migrating pages concurrently
@@ -474,8 +474,12 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
* recursion in the case it ends up triggering memory compaction
* while it is attempting to inflate the ballon.
*/
- if (!mutex_trylock(&vb->balloon_lock))
+ if (!mutex_trylock(&vb->inflate_lock))
+ return -EAGAIN;
+ if (!mutex_trylock(&vb->deflate_lock)) {
+ mutex_unlock(&vb->inflate_lock);
return -EAGAIN;
+ }
get_page(newpage); /* balloon reference */
@@ -485,17 +489,16 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
vb_dev_info->isolated_pages--;
__count_vm_event(BALLOON_MIGRATE);
spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
- vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
- set_page_pfns(vb, vb->pfns, newpage);
- tell_host(vb, vb->inflate_vq);
+
+ set_page_pfns(vb, pfns, newpage);
+ tell_host(vb, vb->inflate_vq, pfns, VIRTIO_BALLOON_PAGES_PER_PAGE);
+ mutex_unlock(&vb->inflate_lock);
/* balloon's page migration 2nd step -- deflate "page" */
balloon_page_delete(page);
- vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
- set_page_pfns(vb, vb->pfns, page);
- tell_host(vb, vb->deflate_vq);
-
- mutex_unlock(&vb->balloon_lock);
+ set_page_pfns(vb, pfns, page);
+ tell_host(vb, vb->deflate_vq, pfns, VIRTIO_BALLOON_PAGES_PER_PAGE);
+ mutex_unlock(&vb->deflate_lock);
put_page(page); /* balloon reference */
@@ -542,8 +545,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
spin_lock_init(&vb->stop_update_lock);
vb->stop_update = false;
- vb->num_pages = 0;
- mutex_init(&vb->balloon_lock);
+ atomic64_set(&vb->num_pages, 0);
+ mutex_init(&vb->inflate_lock);
+ mutex_init(&vb->deflate_lock);
init_waitqueue_head(&vb->acked);
vb->vdev = vdev;
@@ -596,8 +600,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
static void remove_common(struct virtio_balloon *vb)
{
/* There might be pages left in the balloon: free them. */
- while (vb->num_pages)
- leak_balloon(vb, vb->num_pages);
+ while (atomic64_read(&vb->num_pages))
+ leak_balloon(vb, atomic64_read(&vb->num_pages));
update_balloon_size(vb);
/* Now we reset the device so we can clean up the queues. */
--
2.7.4
^ permalink raw reply related
* [PATCH v1 0/3] Virtio-balloon Improvement
From: Wei Wang @ 2017-10-20 11:54 UTC (permalink / raw)
To: mst, penguin-kernel, mhocko, linux-kernel, linux-mm,
virtualization
This patch series intends to summarize the recent contributions made by
Michael S. Tsirkin, Tetsuo Handa, Michal Hocko etc. via reporting and
discussing the related deadlock issues on the mailinglist. Please check
each patch for details.
From a high-level point of view, this patch series achieves:
1) eliminate the deadlock issue fundamentally caused by the inability
to run leak_balloon and fill_balloon concurrently;
2) enable OOM to release more than 256 inflated pages; and
3) stop inflating when the guest is under severe memory pressure
(i.e. OOM).
Here is an example of the benefit brought by this patch series:
The guest sets virtio_balloon.oom_pages=100000. When the host requests
to inflate 7.9G of an 8G idle guest, the guest can still run normally
since OOM can guarantee at least 100000 pages (400MB) for the guest.
Without the above patches, the guest will kill all the killable
processes and fall into kernel panic finally.
Wei Wang (3):
virtio-balloon: replace the coarse-grained balloon_lock
virtio-balloon: deflate up to oom_pages on OOM
virtio-balloon: stop inflating when OOM occurs
drivers/virtio/virtio_balloon.c | 149 ++++++++++++++++++++++++----------------
1 file changed, 91 insertions(+), 58 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v1 01/27] x86/crypto: Adapt assembly for PIE support
From: Ard Biesheuvel @ 2017-10-20 8:28 UTC (permalink / raw)
To: Ingo Molnar
Cc: Nicolas Pitre, Michal Hocko, Radim Krčmář,
Linux Doc Mailing List, Daniel Micay, Len Brown, Peter Zijlstra,
Christopher Li, Jan H . Schönherr, Alexei Starovoitov,
virtualization, David Howells, Paul Gortmaker, Waiman Long,
Pavel Machek, H . Peter Anvin, Kernel Hardening,
Christoph Lameter, Thomas Gleixner, x86@kernel.org, Herbert Xu,
Daniel Borkmann
In-Reply-To: <20171020082420.lsvu7mqjrgnahm5t@gmail.com>
On 20 October 2017 at 09:24, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Thomas Garnier <thgarnie@google.com> wrote:
>
>> Change the assembly code to use only relative references of symbols for the
>> kernel to be PIE compatible.
>>
>> Position Independent Executable (PIE) support will allow to extended the
>> KASLR randomization range below the -2G memory limit.
>
>> diff --git a/arch/x86/crypto/aes-x86_64-asm_64.S b/arch/x86/crypto/aes-x86_64-asm_64.S
>> index 8739cf7795de..86fa068e5e81 100644
>> --- a/arch/x86/crypto/aes-x86_64-asm_64.S
>> +++ b/arch/x86/crypto/aes-x86_64-asm_64.S
>> @@ -48,8 +48,12 @@
>> #define R10 %r10
>> #define R11 %r11
>>
>> +/* Hold global for PIE suport */
>> +#define RBASE %r12
>> +
>> #define prologue(FUNC,KEY,B128,B192,r1,r2,r5,r6,r7,r8,r9,r10,r11) \
>> ENTRY(FUNC); \
>> + pushq RBASE; \
>> movq r1,r2; \
>> leaq KEY+48(r8),r9; \
>> movq r10,r11; \
>> @@ -74,54 +78,63 @@
>> movl r6 ## E,4(r9); \
>> movl r7 ## E,8(r9); \
>> movl r8 ## E,12(r9); \
>> + popq RBASE; \
>> ret; \
>> ENDPROC(FUNC);
>>
>> +#define round_mov(tab_off, reg_i, reg_o) \
>> + leaq tab_off(%rip), RBASE; \
>> + movl (RBASE,reg_i,4), reg_o;
>> +
>> +#define round_xor(tab_off, reg_i, reg_o) \
>> + leaq tab_off(%rip), RBASE; \
>> + xorl (RBASE,reg_i,4), reg_o;
>> +
>> #define round(TAB,OFFSET,r1,r2,r3,r4,r5,r6,r7,r8,ra,rb,rc,rd) \
>> movzbl r2 ## H,r5 ## E; \
>> movzbl r2 ## L,r6 ## E; \
>> - movl TAB+1024(,r5,4),r5 ## E;\
>> + round_mov(TAB+1024, r5, r5 ## E)\
>> movw r4 ## X,r2 ## X; \
>> - movl TAB(,r6,4),r6 ## E; \
>> + round_mov(TAB, r6, r6 ## E) \
>> roll $16,r2 ## E; \
>> shrl $16,r4 ## E; \
>> movzbl r4 ## L,r7 ## E; \
>> movzbl r4 ## H,r4 ## E; \
>> xorl OFFSET(r8),ra ## E; \
>> xorl OFFSET+4(r8),rb ## E; \
>> - xorl TAB+3072(,r4,4),r5 ## E;\
>> - xorl TAB+2048(,r7,4),r6 ## E;\
>> + round_xor(TAB+3072, r4, r5 ## E)\
>> + round_xor(TAB+2048, r7, r6 ## E)\
>> movzbl r1 ## L,r7 ## E; \
>> movzbl r1 ## H,r4 ## E; \
>> - movl TAB+1024(,r4,4),r4 ## E;\
>> + round_mov(TAB+1024, r4, r4 ## E)\
>> movw r3 ## X,r1 ## X; \
>> roll $16,r1 ## E; \
>> shrl $16,r3 ## E; \
>> - xorl TAB(,r7,4),r5 ## E; \
>> + round_xor(TAB, r7, r5 ## E) \
>> movzbl r3 ## L,r7 ## E; \
>> movzbl r3 ## H,r3 ## E; \
>> - xorl TAB+3072(,r3,4),r4 ## E;\
>> - xorl TAB+2048(,r7,4),r5 ## E;\
>> + round_xor(TAB+3072, r3, r4 ## E)\
>> + round_xor(TAB+2048, r7, r5 ## E)\
>> movzbl r1 ## L,r7 ## E; \
>> movzbl r1 ## H,r3 ## E; \
>> shrl $16,r1 ## E; \
>> - xorl TAB+3072(,r3,4),r6 ## E;\
>> - movl TAB+2048(,r7,4),r3 ## E;\
>> + round_xor(TAB+3072, r3, r6 ## E)\
>> + round_mov(TAB+2048, r7, r3 ## E)\
>> movzbl r1 ## L,r7 ## E; \
>> movzbl r1 ## H,r1 ## E; \
>> - xorl TAB+1024(,r1,4),r6 ## E;\
>> - xorl TAB(,r7,4),r3 ## E; \
>> + round_xor(TAB+1024, r1, r6 ## E)\
>> + round_xor(TAB, r7, r3 ## E) \
>> movzbl r2 ## H,r1 ## E; \
>> movzbl r2 ## L,r7 ## E; \
>> shrl $16,r2 ## E; \
>> - xorl TAB+3072(,r1,4),r3 ## E;\
>> - xorl TAB+2048(,r7,4),r4 ## E;\
>> + round_xor(TAB+3072, r1, r3 ## E)\
>> + round_xor(TAB+2048, r7, r4 ## E)\
>> movzbl r2 ## H,r1 ## E; \
>> movzbl r2 ## L,r2 ## E; \
>> xorl OFFSET+8(r8),rc ## E; \
>> xorl OFFSET+12(r8),rd ## E; \
>> - xorl TAB+1024(,r1,4),r3 ## E;\
>> - xorl TAB(,r2,4),r4 ## E;
>> + round_xor(TAB+1024, r1, r3 ## E)\
>> + round_xor(TAB, r2, r4 ## E)
>
> This appears to be adding unconditional overhead to a function that was moved to
> assembly to improve its performance.
>
I did some benchmarking on this code a while ago and, interestingly,
it was slower than the generic C implementation (on a Pentium E2200),
so we may want to consider whether we still need this driver in the
first place.
^ 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