* Re: [PATCH net-next] virtio_net: Add ethtool stats
From: Stephen Hemminger @ 2017-12-24 18:16 UTC (permalink / raw)
To: Toshiaki Makita
Cc: netdev, virtualization, David S . Miller, Michael S . Tsirkin
In-Reply-To: <1513744837-2672-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On Wed, 20 Dec 2017 13:40:37 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> +
> +static const struct virtnet_gstats virtnet_gstrings_stats[] = {
> + { "rx_packets", VIRTNET_NETDEV_STAT(rx_packets) },
> + { "tx_packets", VIRTNET_NETDEV_STAT(tx_packets) },
> + { "rx_bytes", VIRTNET_NETDEV_STAT(rx_bytes) },
> + { "tx_bytes", VIRTNET_NETDEV_STAT(tx_bytes) },
> + { "rx_dropped", VIRTNET_NETDEV_STAT(rx_dropped) },
> + { "rx_length_errors", VIRTNET_NETDEV_STAT(rx_length_errors) },
> + { "rx_frame_errors", VIRTNET_NETDEV_STAT(rx_frame_errors) },
> + { "tx_dropped", VIRTNET_NETDEV_STAT(tx_dropped) },
> + { "tx_fifo_errors", VIRTNET_NETDEV_STAT(tx_fifo_errors) },
> +};
> +
Please do not merge pre-existing global stats into ethtool.
It just duplicates existing functionality.
^ permalink raw reply
* [PATCH] vhost: remove unused lock check flag in vhost_dev_cleanup()
From: 夷则(Caspar) @ 2017-12-24 16:08 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Stefan Hajnoczi
Cc: 夷则(Caspar), linux-kernel, kvm, virtualization
In commit ea5d404655ba ("vhost: fix release path lockdep checks"),
Michael added a flag to check whether we should hold a lock in
vhost_dev_cleanup(), however, in commit 47283bef7ed3 ("vhost: move
memory pointer to VQs"), RCU operations have been replaced by
mutex, we can remove the no-longer-used `locked' parameter now.
Signed-off-by: Caspar Zhang <jinli.zjl@alibaba-inc.com>
---
drivers/vhost/net.c | 2 +-
drivers/vhost/scsi.c | 2 +-
drivers/vhost/test.c | 2 +-
drivers/vhost/vhost.c | 5 ++---
drivers/vhost/vhost.h | 2 +-
drivers/vhost/vsock.c | 2 +-
6 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c7bdeb655646..a354d8d731e3 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -996,7 +996,7 @@ static int vhost_net_release(struct inode *inode, struct file *f)
vhost_net_stop(n, &tx_sock, &rx_sock);
vhost_net_flush(n);
vhost_dev_stop(&n->dev);
- vhost_dev_cleanup(&n->dev, false);
+ vhost_dev_cleanup(&n->dev);
vhost_net_vq_reset(n);
if (tx_sock)
sockfd_put(tx_sock);
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 71517b3c5558..797d08916553 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1420,7 +1420,7 @@ static int vhost_scsi_release(struct inode *inode, struct file *f)
mutex_unlock(&vs->dev.mutex);
vhost_scsi_clear_endpoint(vs, &t);
vhost_dev_stop(&vs->dev);
- vhost_dev_cleanup(&vs->dev, false);
+ vhost_dev_cleanup(&vs->dev);
/* Jobs can re-queue themselves in evt kick handler. Do extra flush. */
vhost_scsi_flush(vs);
kfree(vs->dev.vqs);
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 3cc98c07dcd3..906b8f0f19f7 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -157,7 +157,7 @@ static int vhost_test_release(struct inode *inode, struct file *f)
vhost_test_stop(n, &private);
vhost_test_flush(n);
- vhost_dev_cleanup(&n->dev, false);
+ vhost_dev_cleanup(&n->dev);
/* We do an extra flush before freeing memory,
* since jobs can re-queue themselves. */
vhost_test_flush(n);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..014675c3d569 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -544,7 +544,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_umem *umem)
{
int i;
- vhost_dev_cleanup(dev, true);
+ vhost_dev_cleanup(dev);
/* Restore memory to default empty mapping. */
INIT_LIST_HEAD(&umem->umem_list);
@@ -611,8 +611,7 @@ static void vhost_clear_msg(struct vhost_dev *dev)
spin_unlock(&dev->iotlb_lock);
}
-/* Caller should have device mutex if and only if locked is set */
-void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
+void vhost_dev_cleanup(struct vhost_dev *dev)
{
int i;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 79c6e7a60a5e..ff4d918e3e0a 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -181,7 +181,7 @@ bool vhost_dev_has_owner(struct vhost_dev *dev);
long vhost_dev_check_owner(struct vhost_dev *);
struct vhost_umem *vhost_dev_reset_owner_prepare(void);
void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_umem *);
-void vhost_dev_cleanup(struct vhost_dev *, bool locked);
+void vhost_dev_cleanup(struct vhost_dev *);
void vhost_dev_stop(struct vhost_dev *);
long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, void __user *argp);
long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 5a5e981bd8e4..0d14e2ff19f1 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -599,7 +599,7 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
}
spin_unlock_bh(&vsock->send_pkt_list_lock);
- vhost_dev_cleanup(&vsock->dev, false);
+ vhost_dev_cleanup(&vsock->dev);
kfree(vsock->dev.vqs);
vhost_vsock_free(vsock);
return 0;
--
2.15.1
^ permalink raw reply related
* (unknown),
From: Solen win @ 2017-12-24 9:07 UTC (permalink / raw)
To: virtualization
[-- Attachment #1.1: Type: text/plain, Size: 23 bytes --]
Solenwin@freshdesk.com
[-- Attachment #1.2: Type: text/html, Size: 141 bytes --]
[-- 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: [virtio-dev] Re: [PATCH v20 4/7] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2017-12-24 8:16 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: aarcange, virtio-dev, riel, quan.xu0, kvm, mst, qemu-devel,
amit.shah, liliang.opensource, mawilcox, linux-kernel,
virtualization, linux-mm, yang.zhang.wz, nilal, cornelia.huck,
pbonzini, akpm, mhocko, mgorman
In-Reply-To: <5A3F5A4A.1070009@intel.com>
On 12/24/2017 03:42 PM, Wei Wang wrote:
> On 12/24/2017 12:45 PM, Tetsuo Handa wrote:
>> Matthew Wilcox wrote:
>>>> + unsigned long pfn = page_to_pfn(page);
>>>> + int ret;
>>>> +
>>>> + *pfn_min = min(pfn, *pfn_min);
>>>> + *pfn_max = max(pfn, *pfn_max);
>>>> +
>>>> + do {
>>>> + if (xb_preload(GFP_NOWAIT | __GFP_NOWARN) < 0)
>>>> + return -ENOMEM;
>>>> +
>>>> + ret = xb_set_bit(&vb->page_xb, pfn);
>>>> + xb_preload_end();
>>>> + } while (unlikely(ret == -EAGAIN));
>>> OK, so you don't need a spinlock because you're under a mutex? But you
>>> can't allocate memory because you're in the balloon driver, and so a
>>> GFP_KERNEL allocation might recurse into your driver?
>> Right. We can't (directly or indirectly) depend on
>> __GFP_DIRECT_RECLAIM && !__GFP_NORETRY
>> allocations because the balloon driver needs to handle OOM notifier
>> callback.
>>
>>> Would GFP_NOIO
>>> do the job? I'm a little hazy on exactly how the balloon driver works.
>> GFP_NOIO implies __GFP_DIRECT_RECLAIM. In the worst case, it can
>> lockup due to
>> the too small to fail memory allocation rule. GFP_NOIO |
>> __GFP_NORETRY would work
>> if there is really a guarantee that GFP_NOIO | __GFP_NORETRY never
>> depend on
>> __GFP_DIRECT_RECLAIM && !__GFP_NORETRY allocations, which is too
>> subtle for me to
>> validate. The direct reclaim dependency is too complicated to validate.
>> I consider that !__GFP_DIRECT_RECLAIM is the future-safe choice.
>
> What's the problem with (or how is it better than) the "GFP_NOWAIT |
> __GFP_NOWARN" we are using here?
>
>
>>> If you can't preload with anything better than that, I think that
>>> xb_set_bit() should attempt an allocation with GFP_NOWAIT |
>>> __GFP_NOWARN,
>>> and then you can skip the preload; it has no value for you.
>> Yes, that's why I suggest directly using kzalloc() at xb_set_bit().
>
> It has some possibilities to remove that preload if we also do the
> bitmap allocation in the xb_set_bit():
>
> bitmap = rcu_dereference_raw(*slot);
> if (!bitmap) {
> bitmap = this_cpu_xchg(ida_bitmap, NULL);
> if (!bitmap) {
> bitmap = kmalloc(sizeof(*bitmap), gfp);
> if (!bitmap)
> return -ENOMEM;
> }
> }
>
> But why not just follow the radix tree implementation style that puts
> the allocation in preload, which would be invoked with a more relaxed
> gfp in other use cases?
> Its usage in virtio_balloon is just a little special that we need to
> put the allocation within the balloon_lock, which doesn't give us the
> benefit of using a relaxed gfp in preload, but it doesn't prevent us
> from living with the current APIs (i.e. the preload + xb_set pattern).
> On the other side, if we do it as above, we have more things that need
> to consider. For example, what if the a use case just want the radix
> tree implementation style, which means it doesn't want allocation
> within xb_set(), then would we be troubled with how to avoid the
> allocation path in that case?
>
> So, I think it is better to stick with the convention by putting the
> allocation in preload. Breaking the convention should show obvious
> advantages, IMHO.
>
>
>>
>>>> @@ -173,8 +292,15 @@ static unsigned fill_balloon(struct
>>>> virtio_balloon *vb, size_t num)
>>>> while ((page = balloon_page_pop(&pages))) {
>>>> balloon_page_enqueue(&vb->vb_dev_info, page);
>>>> + if (use_sg) {
>>>> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) {
>>>> + __free_page(page);
>>>> + continue;
>>>> + }
>>>> + } else {
>>>> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
>>>> + }
>>> Is this the right behaviour?
>> I don't think so. In the worst case, we can set no bit using
>> xb_set_page().
>
>>
>>> If we can't record the page in the xb,
>>> wouldn't we rather send it across as a single page?
>>>
>> I think that we need to be able to fallback to !use_sg path when OOM.
>
> I also have different thoughts:
>
> 1) For OOM, we have leak_balloon_sg_oom (oom has nothing to do with
> fill_balloon), which does not use xbitmap to record pages, thus no
> memory allocation.
>
> 2) If the memory is already under pressure, it is pointless to
> continue inflating memory to the host. We need to give thanks to the
> memory allocation failure reported by xbitmap, which gets us a chance
> to release the inflated pages that have been demonstrated to cause the
> memory pressure of the guest.
>
Forgot to add my conclusion: I think the above behavior is correct.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v20 4/7] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2017-12-24 7:42 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: aarcange, virtio-dev, riel, quan.xu0, kvm, mst, qemu-devel,
amit.shah, liliang.opensource, mawilcox, linux-kernel,
virtualization, linux-mm, yang.zhang.wz, nilal, cornelia.huck,
pbonzini, akpm, mhocko, mgorman
In-Reply-To: <201712241345.DIG21823.SLFOOJtQFOMVFH@I-love.SAKURA.ne.jp>
On 12/24/2017 12:45 PM, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
>>> + unsigned long pfn = page_to_pfn(page);
>>> + int ret;
>>> +
>>> + *pfn_min = min(pfn, *pfn_min);
>>> + *pfn_max = max(pfn, *pfn_max);
>>> +
>>> + do {
>>> + if (xb_preload(GFP_NOWAIT | __GFP_NOWARN) < 0)
>>> + return -ENOMEM;
>>> +
>>> + ret = xb_set_bit(&vb->page_xb, pfn);
>>> + xb_preload_end();
>>> + } while (unlikely(ret == -EAGAIN));
>> OK, so you don't need a spinlock because you're under a mutex? But you
>> can't allocate memory because you're in the balloon driver, and so a
>> GFP_KERNEL allocation might recurse into your driver?
> Right. We can't (directly or indirectly) depend on __GFP_DIRECT_RECLAIM && !__GFP_NORETRY
> allocations because the balloon driver needs to handle OOM notifier callback.
>
>> Would GFP_NOIO
>> do the job? I'm a little hazy on exactly how the balloon driver works.
> GFP_NOIO implies __GFP_DIRECT_RECLAIM. In the worst case, it can lockup due to
> the too small to fail memory allocation rule. GFP_NOIO | __GFP_NORETRY would work
> if there is really a guarantee that GFP_NOIO | __GFP_NORETRY never depend on
> __GFP_DIRECT_RECLAIM && !__GFP_NORETRY allocations, which is too subtle for me to
> validate. The direct reclaim dependency is too complicated to validate.
> I consider that !__GFP_DIRECT_RECLAIM is the future-safe choice.
What's the problem with (or how is it better than) the "GFP_NOWAIT |
__GFP_NOWARN" we are using here?
>> If you can't preload with anything better than that, I think that
>> xb_set_bit() should attempt an allocation with GFP_NOWAIT | __GFP_NOWARN,
>> and then you can skip the preload; it has no value for you.
> Yes, that's why I suggest directly using kzalloc() at xb_set_bit().
It has some possibilities to remove that preload if we also do the
bitmap allocation in the xb_set_bit():
bitmap = rcu_dereference_raw(*slot);
if (!bitmap) {
bitmap = this_cpu_xchg(ida_bitmap, NULL);
if (!bitmap) {
bitmap = kmalloc(sizeof(*bitmap), gfp);
if (!bitmap)
return -ENOMEM;
}
}
But why not just follow the radix tree implementation style that puts
the allocation in preload, which would be invoked with a more relaxed
gfp in other use cases?
Its usage in virtio_balloon is just a little special that we need to put
the allocation within the balloon_lock, which doesn't give us the
benefit of using a relaxed gfp in preload, but it doesn't prevent us
from living with the current APIs (i.e. the preload + xb_set pattern).
On the other side, if we do it as above, we have more things that need
to consider. For example, what if the a use case just want the radix
tree implementation style, which means it doesn't want allocation within
xb_set(), then would we be troubled with how to avoid the allocation
path in that case?
So, I think it is better to stick with the convention by putting the
allocation in preload. Breaking the convention should show obvious
advantages, IMHO.
>
>>> @@ -173,8 +292,15 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>>>
>>> while ((page = balloon_page_pop(&pages))) {
>>> balloon_page_enqueue(&vb->vb_dev_info, page);
>>> + if (use_sg) {
>>> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) {
>>> + __free_page(page);
>>> + continue;
>>> + }
>>> + } else {
>>> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
>>> + }
>> Is this the right behaviour?
> I don't think so. In the worst case, we can set no bit using xb_set_page().
>
>> If we can't record the page in the xb,
>> wouldn't we rather send it across as a single page?
>>
> I think that we need to be able to fallback to !use_sg path when OOM.
I also have different thoughts:
1) For OOM, we have leak_balloon_sg_oom (oom has nothing to do with
fill_balloon), which does not use xbitmap to record pages, thus no
memory allocation.
2) If the memory is already under pressure, it is pointless to continue
inflating memory to the host. We need to give thanks to the memory
allocation failure reported by xbitmap, which gets us a chance to
release the inflated pages that have been demonstrated to cause the
memory pressure of the guest.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Wei Wang @ 2017-12-24 7:31 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: virtio-dev, kvm, mst, qemu-devel, mawilcox, linux-kernel,
virtualization, linux-mm, akpm, mhocko
In-Reply-To: <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp>
On 12/23/2017 10:33 PM, Tetsuo Handa wrote:
>>>> + bitmap = rcu_dereference_raw(*slot);
>>>> + if (!bitmap) {
>>>> + bitmap = this_cpu_xchg(ida_bitmap, NULL);
>>>> + if (!bitmap)
>>>> + return -ENOMEM;
>>> I can't understand this. I can understand if it were
>>>
>>> BUG_ON(!bitmap);
>>>
>>> because you called xb_preload().
>>>
>>> But
>>>
>>> /*
>>> * Regular test 2
>>> * set bit 2000, 2001, 2040
>>> * Next 1 in [0, 2048) --> 2000
>>> * Next 1 in [2000, 2002) --> 2000
>>> * Next 1 in [2002, 2041) --> 2040
>>> * Next 1 in [2002, 2040) --> none
>>> * Next 0 in [2000, 2048) --> 2002
>>> * Next 0 in [2048, 2060) --> 2048
>>> */
>>> xb_preload(GFP_KERNEL);
>>> assert(!xb_set_bit(&xb1, 2000));
>>> assert(!xb_set_bit(&xb1, 2001));
>>> assert(!xb_set_bit(&xb1, 2040));
>> [...]
>>> xb_preload_end();
>>>
>>> you are not calling xb_preload() prior to each xb_set_bit() call.
>>> This means that, if each xb_set_bit() is not surrounded with
>>> xb_preload()/xb_preload_end(), there is possibility of hitting
>>> this_cpu_xchg(ida_bitmap, NULL) == NULL.
>> This is just a lazy test. We "know" that the bits in the range 1024-2047
>> will all land in the same bitmap, so there's no need to preload for each
>> of them.
> Testcases also serves as how to use that API.
> Assuming such thing leads to incorrect usage.
If callers are aware that the bits that they going to record locate in
the same bitmap, I think they should also perform the xb_ APIs with only
one preload. So the test cases here have shown them a correct example.
We can probably add some comments above to explain this.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v20 4/7] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Matthew Wilcox @ 2017-12-24 3:21 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz, kvm, mst, penguin-kernel, liliang.opensource,
qemu-devel, virtualization, linux-mm, aarcange, virtio-dev,
mawilcox, nilal, riel, cornelia.huck, mhocko, quan.xu0,
linux-kernel, amit.shah, pbonzini, akpm, mgorman
In-Reply-To: <1513685879-21823-5-git-send-email-wei.w.wang@intel.com>
On Tue, Dec 19, 2017 at 08:17:56PM +0800, Wei Wang wrote:
> +/*
> + * Send balloon pages in sgs to host. The balloon pages are recorded in the
> + * page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
> + * The page xbitmap is searched for continuous "1" bits, which correspond
> + * to continuous pages, to chunk into sgs.
> + *
> + * @page_xb_start and @page_xb_end form the range of bits in the xbitmap that
> + * need to be searched.
> + */
> +static void tell_host_sgs(struct virtio_balloon *vb,
> + struct virtqueue *vq,
> + unsigned long page_xb_start,
> + unsigned long page_xb_end)
I'm not crazy about the naming here. I'd use pfn_min and pfn_max like
you use in the caller.
> +{
> + unsigned long pfn_start, pfn_end;
> + uint32_t max_len = round_down(UINT_MAX, PAGE_SIZE);
> + uint64_t len;
> +
> + pfn_start = page_xb_start;
And I think pfn_start is actually just 'pfn'.
'pfn_end' is perhaps just 'end'. Or 'gap'.
> + while (pfn_start < page_xb_end) {
> + pfn_start = xb_find_set(&vb->page_xb, page_xb_end + 1,
> + pfn_start);
> + if (pfn_start == page_xb_end + 1)
> + break;
> + pfn_end = xb_find_zero(&vb->page_xb, page_xb_end + 1,
> + pfn_start);
> + len = (pfn_end - pfn_start) << PAGE_SHIFT;
> +static inline int xb_set_page(struct virtio_balloon *vb,
> + struct page *page,
> + unsigned long *pfn_min,
> + unsigned long *pfn_max)
> +{
I really don't like it that you're naming things after the 'xb'.
Things should be named by something that makes sense to the user, not
after the particular implementation. If you changed the underlying
representation from an xbitmap to, say, a BTree, you wouldn't want to
rename this function to 'btree_set_page'. Maybe this function is really
"vb_set_page". Or "record_page". Or something. Someone who understands
this driver better than I do can probably weigh in with a better name.
> + unsigned long pfn = page_to_pfn(page);
> + int ret;
> +
> + *pfn_min = min(pfn, *pfn_min);
> + *pfn_max = max(pfn, *pfn_max);
> +
> + do {
> + if (xb_preload(GFP_NOWAIT | __GFP_NOWARN) < 0)
> + return -ENOMEM;
> +
> + ret = xb_set_bit(&vb->page_xb, pfn);
> + xb_preload_end();
> + } while (unlikely(ret == -EAGAIN));
OK, so you don't need a spinlock because you're under a mutex? But you
can't allocate memory because you're in the balloon driver, and so a
GFP_KERNEL allocation might recurse into your driver? Would GFP_NOIO
do the job? I'm a little hazy on exactly how the balloon driver works.
If you can't preload with anything better than that, I think that
xb_set_bit() should attempt an allocation with GFP_NOWAIT | __GFP_NOWARN,
and then you can skip the preload; it has no value for you.
> @@ -173,8 +292,15 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>
> while ((page = balloon_page_pop(&pages))) {
> balloon_page_enqueue(&vb->vb_dev_info, page);
> + if (use_sg) {
> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) {
> + __free_page(page);
> + continue;
> + }
> + } else {
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + }
Is this the right behaviour? If we can't record the page in the xb,
wouldn't we rather send it across as a single page?
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-23 14:58 UTC (permalink / raw)
To: Tetsuo Handa
Cc: virtio-dev, kvm, mst, qemu-devel, mawilcox, linux-kernel,
virtualization, linux-mm, akpm, mhocko
In-Reply-To: <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp>
On Sat, Dec 23, 2017 at 11:33:45PM +0900, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
> > On Sat, Dec 23, 2017 at 11:59:54AM +0900, Tetsuo Handa wrote:
> > > Matthew Wilcox wrote:
> > > > + bit %= IDA_BITMAP_BITS;
> > > > + radix_tree_iter_init(&iter, index);
> > > > + slot = idr_get_free_cmn(root, &iter, GFP_NOWAIT | __GFP_NOWARN, index);
> > > > + if (IS_ERR(slot)) {
> > > > + if (slot == ERR_PTR(-ENOSPC))
> > > > + return 0; /* Already set */
> > >
> > > Why already set? I guess something is there, but is it guaranteed that
> > > there is a bitmap with the "bit" set?
> >
> > Yes. For radix trees tagged with IDR_RT_MARKER, newly created slots
> > have the IDR_FREE tag set. We only clear the IDR_FREE tag once the
> > bitmap is full. So if we try to find a free slot and the tag is clear,
> > we know the bitmap is full.
> >
>
> OK. But does using IDR_FREE tag have more benefit than cost?
> You are doing
>
> if (bitmap_full(bitmap->bitmap, IDA_BITMAP_BITS))
> radix_tree_iter_tag_clear(root, &iter, IDR_FREE);
>
> for each xb_set_bit() call. How likely do we hit ERR_PTR(-ENOSPC) path?
> Isn't removing both bitmap_full() and ERR_PTR(-ENOSPC) better?
You're assuming that the purpose of using IDR_FREE is to save xb_set_bit
from walking the tree unnecessarily. It isn't; that's just a happy
side-effect. Its main purpose is to make xb_find_zero() efficient. If
we have large ranges of set bits, xb_find_zero() will be able to skip them.
> > This is just a lazy test. We "know" that the bits in the range 1024-2047
> > will all land in the same bitmap, so there's no need to preload for each
> > of them.
>
> Testcases also serves as how to use that API.
> Assuming such thing leads to incorrect usage.
Sure. Would you like to submit a patch?
> > > If bitmap == NULL at this_cpu_xchg(ida_bitmap, NULL) is allowed,
> > > you can use kzalloc(sizeof(*bitmap), GFP_NOWAIT | __GFP_NOWARN)
> > > and get rid of xb_preload()/xb_preload_end().
> >
> > No, we can't. GFP_NOWAIT | __GFP_NOWARN won't try very hard to allocate
> > memory. There's no reason to fail the call if the user is in a context
> > where they can try harder to free memory.
>
> But there is no reason to use GFP_NOWAIT at idr_get_free_cmn() if it is
> safe to use GFP_KERNEL. If we don't require xb_preload() which forces
> idr_get_free_cmn() to use GFP_NOWAIT due to possibility of preemption
> disabled by xb_preload(), we can allow passing gfp flags to xb_set_bit().
The assumption is that the user has done:
xb_preload(GFP_KERNEL);
spin_lock(my_lock);
xb_set_bit(xb, bit);
spin_unlock(my_lock);
xb_preload_end();
This is not the world's greatest interface. Once I have the XArray
finished, we'll be able to ditch the external spinlock and the preload
interface and be able to call:
xb_set_bit(xb, bit, GFP_KERNEL);
> > xb_preload also preloads radix tree nodes.
>
> But it after all forces idr_get_free_cmn() to use GFP_NOWAIT, doesn't it?
I think you don't understand how the radix tree allocates nodes. preloading
means that it will be able to access the nodes which were allocated earlier.
> Speak of initial user (i.e. virtio-balloon), xb_preload() won't be able to
> use GFP_KERNEL in order to avoid OOM lockup. Therefore, I don't see
> advantages with using xb_preload(). If xb_set_bit() receives gfp flags,
> the caller can pass GFP_KERNEL if it is safe to use GFP_KERNEL.
I haven't reviewed how virtio-balloon is using the interfaces.
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-23 3:29 UTC (permalink / raw)
To: Tetsuo Handa
Cc: virtio-dev, kvm, mst, qemu-devel, mawilcox, linux-kernel,
virtualization, linux-mm, akpm, mhocko
In-Reply-To: <201712231159.ECI73411.tFFFJOHOVMOLQS@I-love.SAKURA.ne.jp>
On Sat, Dec 23, 2017 at 11:59:54AM +0900, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
> > + bit %= IDA_BITMAP_BITS;
> > + radix_tree_iter_init(&iter, index);
> > + slot = idr_get_free_cmn(root, &iter, GFP_NOWAIT | __GFP_NOWARN, index);
> > + if (IS_ERR(slot)) {
> > + if (slot == ERR_PTR(-ENOSPC))
> > + return 0; /* Already set */
>
> Why already set? I guess something is there, but is it guaranteed that
> there is a bitmap with the "bit" set?
Yes. For radix trees tagged with IDR_RT_MARKER, newly created slots
have the IDR_FREE tag set. We only clear the IDR_FREE tag once the
bitmap is full. So if we try to find a free slot and the tag is clear,
we know the bitmap is full.
> > + bitmap = rcu_dereference_raw(*slot);
> > + if (!bitmap) {
> > + bitmap = this_cpu_xchg(ida_bitmap, NULL);
> > + if (!bitmap)
> > + return -ENOMEM;
>
> I can't understand this. I can understand if it were
>
> BUG_ON(!bitmap);
>
> because you called xb_preload().
>
> But
>
> /*
> * Regular test 2
> * set bit 2000, 2001, 2040
> * Next 1 in [0, 2048) --> 2000
> * Next 1 in [2000, 2002) --> 2000
> * Next 1 in [2002, 2041) --> 2040
> * Next 1 in [2002, 2040) --> none
> * Next 0 in [2000, 2048) --> 2002
> * Next 0 in [2048, 2060) --> 2048
> */
> xb_preload(GFP_KERNEL);
> assert(!xb_set_bit(&xb1, 2000));
> assert(!xb_set_bit(&xb1, 2001));
> assert(!xb_set_bit(&xb1, 2040));
[...]
> xb_preload_end();
>
> you are not calling xb_preload() prior to each xb_set_bit() call.
> This means that, if each xb_set_bit() is not surrounded with
> xb_preload()/xb_preload_end(), there is possibility of hitting
> this_cpu_xchg(ida_bitmap, NULL) == NULL.
This is just a lazy test. We "know" that the bits in the range 1024-2047
will all land in the same bitmap, so there's no need to preload for each
of them.
> If bitmap == NULL at this_cpu_xchg(ida_bitmap, NULL) is allowed,
> you can use kzalloc(sizeof(*bitmap), GFP_NOWAIT | __GFP_NOWARN)
> and get rid of xb_preload()/xb_preload_end().
No, we can't. GFP_NOWAIT | __GFP_NOWARN won't try very hard to allocate
memory. There's no reason to fail the call if the user is in a context
where they can try harder to free memory.
> You are using idr_get_free_cmn(GFP_NOWAIT | __GFP_NOWARN), which
> means that the caller has to be prepared for allocation failure
> when calling xb_set_bit(). Thus, there is no need to use preload
> in order to avoid failing to allocate "bitmap".
xb_preload also preloads radix tree nodes.
> Also, please clarify why it is OK to just return here.
> I don't know what
>
> radix_tree_iter_replace(root, &iter, slot, bitmap);
>
> is doing. If you created a slot but did not assign "bitmap",
> what the caller of xb_test_bit() etc. will find? If there is an
> assumption about this slot, won't this cause a problem?
xb_test_bit will find NULL if bitmap wasn't assigned. That doesn't
harm anything.
^ permalink raw reply
* [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Jason Baron via Virtualization @ 2017-12-22 21:54 UTC (permalink / raw)
To: netdev, virtualization, qemu-devel; +Cc: mst
In-Reply-To: <cover.1513974243.git.jbaron@akamai.com>
The ability to set speed and duplex for virtio_net in useful in various
scenarios as described here:
16032be virtio_net: add ethtool support for set and get of settings
However, it would be nice to be able to set this from the hypervisor,
such that virtio_net doesn't require custom guest ethtool commands.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
the hypervisor to export a linkspeed and duplex setting. The user can
subsequently overwrite it later if desired via: 'ethtool -s'.
Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 17 ++++++++++++++++-
include/uapi/linux/virtio_net.h | 5 +++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 511f833..4168d82 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2621,6 +2621,20 @@ static int virtnet_probe(struct virtio_device *vdev)
netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
virtnet_init_settings(dev);
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
+ u32 speed;
+ u8 duplex;
+
+ speed = virtio_cread32(vdev, offsetof(struct virtio_net_config,
+ speed));
+ if (ethtool_validate_speed(speed))
+ vi->speed = speed;
+ duplex = virtio_cread8(vdev,
+ offsetof(struct virtio_net_config,
+ duplex));
+ if (ethtool_validate_duplex(duplex))
+ vi->duplex = duplex;
+ }
err = register_netdev(dev);
if (err) {
@@ -2746,7 +2760,8 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
VIRTIO_NET_F_CTRL_MAC_ADDR, \
- VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+ VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+ VIRTIO_NET_F_SPEED_DUPLEX
static unsigned int features[] = {
VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index fc353b5..0f1548e 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,8 @@
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Host set linkspeed and duplex */
+
#ifndef VIRTIO_NET_NO_LEGACY
#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
#endif /* VIRTIO_NET_NO_LEGACY */
@@ -76,6 +78,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
/* Default maximum transmit unit advice */
__u16 mtu;
+ /* Host exported linkspeed and duplex */
+ __u32 speed;
+ __u8 duplex;
} __attribute__((packed));
/*
--
2.6.1
^ permalink raw reply related
* [PATCH v2 0/3] virtio_net: allow hypervisor to indicate linkspeed and duplex setting
From: Jason Baron via Virtualization @ 2017-12-22 21:54 UTC (permalink / raw)
To: netdev, virtualization, qemu-devel; +Cc: mst
In-Reply-To: <1513979641-7999-1-git-send-email-jbaron@akamai.com>
We have found it useful to be able to set the linkspeed and duplex
settings from the host-side for virtio_net. This obviates the need
for guest changes and settings for these fields, and does not require
custom ethtool commands for virtio_net.
The ability to set linkspeed and duplex is useful in various cases
as described here:
16032be virtio_net: add ethtool support for set and get of settings
Using 'ethtool -s' continues to over-write the linkspeed/duplex
settings with this patch.
The 1/3 patch is against net-next, while the 2-3/3 patch are the associated
qemu changes that would go in after as update-linux-headers.sh should
be run first. So the qemu patches are a demonstration of how I intend this
to work.
Thanks,
-Jason
linux changes:
Jason Baron (1):
virtio_net: propagate linkspeed/duplex settings from the hypervisor
drivers/net/virtio_net.c | 17 ++++++++++++++++-
include/uapi/linux/virtio_net.h | 5 +++++
2 files changed, 21 insertions(+), 1 deletion(-)
qemu changes:
Jason Baron (2):
qemu: virtio-net: use 64-bit values for feature flags
qemu: add linkspeed and duplex settings to virtio-net
hw/net/virtio-net.c | 83 +++++++++++++++++++----------
include/hw/virtio/virtio-net.h | 5 +-
include/standard-headers/linux/virtio_net.h | 4 ++
3 files changed, 64 insertions(+), 28 deletions(-)
^ permalink raw reply
* [PATCH 3/3] qemu: add linkspeed and duplex settings to virtio-net
From: Jason Baron via Virtualization @ 2017-12-22 21:53 UTC (permalink / raw)
To: netdev, virtualization, qemu-devel; +Cc: mst
In-Reply-To: <cover.1513974243.git.jbaron@akamai.com>
Although linkspeed and duplex can be set in a linux guest via 'ethtool -s',
this requires custom ethtool commands for virtio-net by default.
Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
the hypervisor to export a linkspeed and duplex setting. The user can
subsequently overwrite it later if desired via: 'ethtool -s'.
Linkspeed and duplex settings can be set as:
'-device virtio-net,speed=10000,duplex=full'
where speed is [-1...INT_MAX], and duplex is ["half"|"full"].
Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 29 +++++++++++++++++++++++++++++
include/hw/virtio/virtio-net.h | 3 +++
include/standard-headers/linux/virtio_net.h | 4 ++++
3 files changed, 36 insertions(+)
create mode 160000 pixman
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index adc20df..7fafe6e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -40,6 +40,12 @@
#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE
#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE
+/* duplex and speed defines */
+#define DUPLEX_UNKNOWN 0xff
+#define DUPLEX_HALF 0x00
+#define DUPLEX_FULL 0x01
+#define SPEED_UNKNOWN -1
+
/*
* Calculate the number of bytes up to and including the given 'field' of
* 'container'.
@@ -61,6 +67,8 @@ static VirtIOFeature feature_sizes[] = {
.end = endof(struct virtio_net_config, max_virtqueue_pairs)},
{.flags = 1ULL << VIRTIO_NET_F_MTU,
.end = endof(struct virtio_net_config, mtu)},
+ {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
+ .end = endof(struct virtio_net_config, duplex)},
{}
};
@@ -88,6 +96,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
virtio_stw_p(vdev, &netcfg.status, n->status);
virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
+ virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
+ netcfg.duplex = n->net_conf.duplex;
memcpy(netcfg.mac, n->mac, ETH_ALEN);
memcpy(config, &netcfg, n->config_size);
}
@@ -1941,6 +1951,23 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
}
+ n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX);
+ if (n->net_conf.duplex_str) {
+ if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
+ n->net_conf.duplex = DUPLEX_HALF;
+ } else if (strncmp(n->net_conf.duplex_str, "full", 5) == 0) {
+ n->net_conf.duplex = DUPLEX_FULL;
+ } else {
+ error_setg(errp, "'duplex' must be 'half' or 'full'");
+ }
+ } else {
+ n->net_conf.duplex = DUPLEX_UNKNOWN;
+ }
+ if (n->net_conf.speed < SPEED_UNKNOWN) {
+ error_setg(errp, "'speed' must be between -1 (SPEED_UNKOWN) and "
+ "INT_MAX");
+ }
+
virtio_net_set_config_size(n, n->host_features);
virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
@@ -2160,6 +2187,8 @@ static Property virtio_net_properties[] = {
DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0),
DEFINE_PROP_BOOL("x-mtu-bypass-backend", VirtIONet, mtu_bypass_backend,
true),
+ DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
+ DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index e7634c9..02484dc 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -38,6 +38,9 @@ typedef struct virtio_net_conf
uint16_t rx_queue_size;
uint16_t tx_queue_size;
uint16_t mtu;
+ int32_t speed;
+ char *duplex_str;
+ uint8_t duplex;
} virtio_net_conf;
/* Maximum packet size we can receive from tap device: header + 64k */
diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h
index 30ff249..0ff1447 100644
--- a/include/standard-headers/linux/virtio_net.h
+++ b/include/standard-headers/linux/virtio_net.h
@@ -36,6 +36,7 @@
#define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Dynamic offload configuration. */
#define VIRTIO_NET_F_MTU 3 /* Initial MTU advice */
+#define VIRTIO_NET_F_SPEED_DUPLEX 4 /* Host set linkspeed and duplex */
#define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
#define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
#define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
@@ -76,6 +77,9 @@ struct virtio_net_config {
uint16_t max_virtqueue_pairs;
/* Default maximum transmit unit advice */
uint16_t mtu;
+ /* Host exported linkspeed and duplex */
+ uint32_t speed;
+ uint8_t duplex;
} QEMU_PACKED;
/*
^ permalink raw reply related
* [PATCH 2/3] qemu: use 64-bit values for feature flags in virtio-net
From: Jason Baron via Virtualization @ 2017-12-22 21:53 UTC (permalink / raw)
To: netdev, virtualization, qemu-devel; +Cc: mst
In-Reply-To: <cover.1513974243.git.jbaron@akamai.com>
In prepartion for using some of the high order feature bits, make sure that
virtio-net uses 64-bit values everywhere.
Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 54 +++++++++++++++++++++---------------------
include/hw/virtio/virtio-net.h | 2 +-
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 38674b0..adc20df 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -48,18 +48,18 @@
(offsetof(container, field) + sizeof(((container *)0)->field))
typedef struct VirtIOFeature {
- uint32_t flags;
+ uint64_t flags;
size_t end;
} VirtIOFeature;
static VirtIOFeature feature_sizes[] = {
- {.flags = 1 << VIRTIO_NET_F_MAC,
+ {.flags = 1ULL << VIRTIO_NET_F_MAC,
.end = endof(struct virtio_net_config, mac)},
- {.flags = 1 << VIRTIO_NET_F_STATUS,
+ {.flags = 1ULL << VIRTIO_NET_F_STATUS,
.end = endof(struct virtio_net_config, status)},
- {.flags = 1 << VIRTIO_NET_F_MQ,
+ {.flags = 1ULL << VIRTIO_NET_F_MQ,
.end = endof(struct virtio_net_config, max_virtqueue_pairs)},
- {.flags = 1 << VIRTIO_NET_F_MTU,
+ {.flags = 1ULL << VIRTIO_NET_F_MTU,
.end = endof(struct virtio_net_config, mtu)},
{}
};
@@ -1938,7 +1938,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
int i;
if (n->net_conf.mtu) {
- n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
+ n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
}
virtio_net_set_config_size(n, n->host_features);
@@ -2109,45 +2109,45 @@ static const VMStateDescription vmstate_virtio_net = {
};
static Property virtio_net_properties[] = {
- DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
- DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
+ DEFINE_PROP_BIT64("guest_csum", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_CSUM, true),
- DEFINE_PROP_BIT("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
- DEFINE_PROP_BIT("guest_tso4", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
+ DEFINE_PROP_BIT64("guest_tso4", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_TSO4, true),
- DEFINE_PROP_BIT("guest_tso6", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("guest_tso6", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_TSO6, true),
- DEFINE_PROP_BIT("guest_ecn", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("guest_ecn", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_ECN, true),
- DEFINE_PROP_BIT("guest_ufo", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("guest_ufo", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_UFO, true),
- DEFINE_PROP_BIT("guest_announce", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("guest_announce", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_ANNOUNCE, true),
- DEFINE_PROP_BIT("host_tso4", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("host_tso4", VirtIONet, host_features,
VIRTIO_NET_F_HOST_TSO4, true),
- DEFINE_PROP_BIT("host_tso6", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("host_tso6", VirtIONet, host_features,
VIRTIO_NET_F_HOST_TSO6, true),
- DEFINE_PROP_BIT("host_ecn", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("host_ecn", VirtIONet, host_features,
VIRTIO_NET_F_HOST_ECN, true),
- DEFINE_PROP_BIT("host_ufo", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("host_ufo", VirtIONet, host_features,
VIRTIO_NET_F_HOST_UFO, true),
- DEFINE_PROP_BIT("mrg_rxbuf", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("mrg_rxbuf", VirtIONet, host_features,
VIRTIO_NET_F_MRG_RXBUF, true),
- DEFINE_PROP_BIT("status", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("status", VirtIONet, host_features,
VIRTIO_NET_F_STATUS, true),
- DEFINE_PROP_BIT("ctrl_vq", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_vq", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_VQ, true),
- DEFINE_PROP_BIT("ctrl_rx", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_rx", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_RX, true),
- DEFINE_PROP_BIT("ctrl_vlan", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_vlan", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_VLAN, true),
- DEFINE_PROP_BIT("ctrl_rx_extra", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_rx_extra", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_RX_EXTRA, true),
- DEFINE_PROP_BIT("ctrl_mac_addr", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_mac_addr", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_MAC_ADDR, true),
- DEFINE_PROP_BIT("ctrl_guest_offloads", VirtIONet, host_features,
+ DEFINE_PROP_BIT64("ctrl_guest_offloads", VirtIONet, host_features,
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
- DEFINE_PROP_BIT("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ, false),
+ DEFINE_PROP_BIT64("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ, false),
DEFINE_NIC_PROPERTIES(VirtIONet, nic_conf),
DEFINE_PROP_UINT32("x-txtimer", VirtIONet, net_conf.txtimer,
TX_TIMER_INTERVAL),
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index b81b6a4..e7634c9 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -67,7 +67,7 @@ typedef struct VirtIONet {
uint32_t has_vnet_hdr;
size_t host_hdr_len;
size_t guest_hdr_len;
- uint32_t host_features;
+ uint64_t host_features;
uint8_t has_ufo;
uint32_t mergeable_rx_bufs;
uint8_t promisc;
--
2.6.1
^ permalink raw reply related
* [bpf-next V2 PATCH 11/14] virtio_net: setup xdp_rxq_info
From: Jesper Dangaard Brouer @ 2017-12-22 17:12 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: Michael S. Tsirkin, netdev, Jesper Dangaard Brouer,
virtualization, dsahern, gospo, bjorn.topel, michael.chan
In-Reply-To: <151396262289.20006.1429172971820409456.stgit@firesoul>
The virtio_net driver doesn't dynamically change the RX-ring queue
layout and backing pages, but instead reject XDP setup if all the
conditions for XDP is not meet. Thus, the xdp_rxq_info also remains
fairly static. This allow us to simply add the reg/unreg to
net_device open/close functions.
Driver hook points for xdp_rxq_info:
* reg : virtnet_open
* unreg: virtnet_close
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/virtio_net.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6fb7b658a6cc..6d45d687ec6a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -31,6 +31,7 @@
#include <linux/average.h>
#include <linux/filter.h>
#include <net/route.h>
+#include <net/xdp.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -115,6 +116,8 @@ struct receive_queue {
/* Name of this receive queue: input.$index */
char name[40];
+
+ struct xdp_rxq_info xdp_rxq;
};
struct virtnet_info {
@@ -559,6 +562,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
xdp.data = xdp.data_hard_start + xdp_headroom;
xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + len;
+ xdp.rxq = &rq->xdp_rxq;
orig_data = xdp.data;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
@@ -1225,13 +1229,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- int i;
+ int i, err;
for (i = 0; i < vi->max_queue_pairs; i++) {
if (i < vi->curr_queue_pairs)
/* Make sure we have some buffers: if oom use wq. */
if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
schedule_delayed_work(&vi->refill, 0);
+
+ err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i);
+ if (err < 0)
+ return err;
+
virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi);
}
@@ -1560,6 +1569,7 @@ static int virtnet_close(struct net_device *dev)
cancel_delayed_work_sync(&vi->refill);
for (i = 0; i < vi->max_queue_pairs; i++) {
+ xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
napi_disable(&vi->rq[i].napi);
virtnet_napi_tx_disable(&vi->sq[i].napi);
}
^ permalink raw reply related
* Re: [PATCH v5 4/4] virtio_remoteproc: correct put_device virtio_device.dev
From: Cornelia Huck @ 2017-12-22 13:40 UTC (permalink / raw)
To: weiping zhang; +Cc: mst, virtualization, bjorn.andersson
In-Reply-To: <742b7e56647f9a1c9fbf0bbc0dc5b93742382371.1513825904.git.zhangweiping@didichuxing.com>
On Thu, 21 Dec 2017 20:40:58 +0800
weiping zhang <zhangweiping@didichuxing.com> wrote:
> rproc_virtio_dev_release will be called iff virtio_device.dev's
> reference count drops to 0. Here we just put vdev.dev, and then
> rproc->dev's cleanup will be done in rproc_virtio_dev_release.
>
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
> drivers/remoteproc/remoteproc_virtio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply
* Re: [PATCH v5 1/4] virtio: split device_register into device_initialize and device_add
From: Cornelia Huck @ 2017-12-22 13:39 UTC (permalink / raw)
To: weiping zhang; +Cc: virtualization, mst
In-Reply-To: <0a8c1c071eef26bc4dfdf796778fd590d0f8e7e7.1513825904.git.zhangweiping@didichuxing.com>
On Thu, 21 Dec 2017 20:39:50 +0800
weiping zhang <zhangweiping@didichuxing.com> wrote:
> In order to make caller do a simple cleanup, we split device_register
> into device_initialize and device_add. device_initialize always succeeds,
> so the caller can always use put_device when register_virtio_device faild.
>
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> ---
> drivers/virtio/virtio.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Corentin Labbe @ 2017-12-22 9:34 UTC (permalink / raw)
To: Herbert Xu
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171222090603.GB32542@gondor.apana.org.au>
On Fri, Dec 22, 2017 at 08:06:03PM +1100, Herbert Xu wrote:
> On Fri, Dec 22, 2017 at 09:41:48AM +0100, Corentin Labbe wrote:
> >
> > It's you that was suggesting using crypto_async_request:
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1474434.html
> > "The only wart with this scheme is that the drivers end up seeing
> > struct crypto_async_request and will need to convert that to the
> > respective request types but I couldn't really find a better way."
> >
> > So I wait for any suggestion.
>
> The core engine code obviously will use the base type but it should
> not be exposed to the driver authors. IOW all exposed API should
> take the final types such as aead_request before casting it.
>
For driver->engine calls(crypto_finalize_request/crypto_transfer_request_to_engine) it's easy.
But I do not see how to do it for crypto_engine_op appart re-introducing the big if/then/else that
you didnt want.
Or do you agree to set the request parameter for crypto_engine_op(prepare_request/unprepare_request/do_one_request) to void * ?
Regards
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Herbert Xu @ 2017-12-22 9:06 UTC (permalink / raw)
To: Corentin Labbe
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171222084148.GA3024@Red>
On Fri, Dec 22, 2017 at 09:41:48AM +0100, Corentin Labbe wrote:
>
> It's you that was suggesting using crypto_async_request:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1474434.html
> "The only wart with this scheme is that the drivers end up seeing
> struct crypto_async_request and will need to convert that to the
> respective request types but I couldn't really find a better way."
>
> So I wait for any suggestion.
The core engine code obviously will use the base type but it should
not be exposed to the driver authors. IOW all exposed API should
take the final types such as aead_request before casting it.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Wei Wang @ 2017-12-22 8:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: virtio-dev, kvm, mst, qemu-devel, penguin-kernel, mawilcox,
linux-kernel, virtualization, linux-mm, akpm, mhocko
In-Reply-To: <20171221210327.GB25009@bombadil.infradead.org>
On 12/22/2017 05:03 AM, Matthew Wilcox wrote:
> OK, here's a rewrite of xbitmap.
>
> Compared to the version you sent:
> - xb_find_set() is the rewrite I sent out yesterday.
> - xb_find_clear() is a new implementation. I use the IDR_FREE tag to find
> clear bits. This led to me finding a bug in radix_tree_for_each_tagged().
> - xb_zero() is also a new implementation (replacing xb_clear_bit_range).
> It should also be more efficient in deep trees.
> - Did not accept your change to xb_set_bit(); I think it's important to
> leave the radix tree node in place, so we're guaranteed to make progress
> in low-memory situations. We're expecting the caller to retry, so the
> memory should not leak.
> - Redid a lot of the kernel-doc. Thanks for adding it! I removed mentions
> of implementation details, leaving only information useful to those who
> are using it.
>
> Compared to the version I put out back in February:
> - Accepted your change to xb_preload(); I think it's an improvement.
> - Rewrote xb_set_bit() to use the radix_tree_iter_ family of functions.
> Should improve performance for deep trees.
> - Rewrote xb_clear_bit() for the same reason.
> - Left out the use of radix tree exceptional entries. Thanks for taking
> them out! Keeps it clearer for now; if they prove useful, we can put
> them back in.
> - Removed the use of __radix_tree_delete and __radix_tree_create, so drop
> the changes to those functions.
>
> Other miscellaneous notes
> - I left xb_fill() in the header file, even though there's no implementation
> yet. Wouldn't be hard to add once we have a user.
> - Used SPDX tags instead of a license notice.
>
> I think we need more test cases ... in reviewing this to send out,
> I found a bug in xb_zero(), which I've only fixed because I don't have
> time to write a test case for it.
Thanks for the improvement. I also found a small bug in xb_zero. With
the following changes, it has passed the current test cases and tested
with the virtio-balloon usage without any issue.
> +
> +/**
> + * xb_zero() - Clear a range of bits in the XBitmap.
> + * @xb: The XBitmap.
> + * @min: The first bit to clear.
> + * @max: The last bit to clear.
> + *
> + * This function is used to clear a range of bits in the xbitmap.
> + */
> +void xb_zero(struct xb *xb, unsigned long min, unsigned long max)
> +{
> + struct radix_tree_root *root = &xb->xbrt;
> + struct radix_tree_iter iter;
> + void __rcu **slot;
> + struct ida_bitmap *bitmap;
> + unsigned long index = min / IDA_BITMAP_BITS;
> + unsigned long first = min % IDA_BITMAP_BITS;
> + unsigned long maxindex = max / IDA_BITMAP_BITS;
> +
> + radix_tree_for_each_slot(slot, root, &iter, index) {
> + unsigned long nbits = IDA_BITMAP_BITS;
> + if (index > maxindex)
> + break;
> + bitmap = radix_tree_deref_slot(slot);
> + if (!bitmap)
> + continue;
> + radix_tree_iter_tag_set(root, &iter, IDR_FREE);
> +
> + if (!first && iter.index < maxindex)
> + goto delete;
> + if (iter.index == maxindex)
> + nbits = max % IDA_BITMAP_BITS + 1;
> + bitmap_clear(bitmap->bitmap, first, nbits);
It should be: bitmap_clear(.., first, nbits - first);
> diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
> index fa7ee369b3c9..adf36e34dd77 100644
> --- a/tools/testing/radix-tree/Makefile
> +++ b/tools/testing/radix-tree/Makefile
> @@ -1,12 +1,13 @@
> # SPDX-License-Identifier: GPL-2.0
>
> CFLAGS += -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address
> -LDFLAGS += -fsanitize=address
> -LDLIBS+= -lpthread -lurcu
> -TARGETS = main idr-test multiorder
> +LDFLAGS += -fsanitize=address $(LDLIBS)
> +LDLIBS := -lpthread -lurcu
> +TARGETS = main idr-test multiorder xbitmap
> CORE_OFILES := radix-tree.o idr.o linux.o test.o find_bit.o
> OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
> - tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o
> + tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o \
> + xbitmap.o
>
> ifndef SHIFT
> SHIFT=3
> @@ -25,8 +26,11 @@ idr-test: idr-test.o $(CORE_OFILES)
>
> multiorder: multiorder.o $(CORE_OFILES)
>
> +xbitmap: xbitmap.o $(CORE_OFILES)
> + $(CC) $(CFLAGS) $(LDFLAGS) $^ -o xbitmap
> +
I think the "$(CC).." line should be removed, which will fix the build
error when adding "xbitmap" to TARGET.
Best,
Wei
^ permalink raw reply
* [PATCH 4.14 065/159] x86/virt: Add enum for hypervisors to replace x86_hyper
From: Greg Kroah-Hartman @ 2017-12-22 8:45 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, rkrcmar, pv-drivers, akataria, virtualization,
Thomas Gleixner, sthemmin, moltmann, Ingo Molnar, Peter Zijlstra,
linux-graphics-maintainer, linux-input, xen-devel, arnd,
Xavier Deguillard, haiyangz, devel, boris.ostrovsky,
Juergen Gross, Greg Kroah-Hartman, dmitry.torokhov, stable,
pbonzini, Linus Torvalds
In-Reply-To: <20171222084623.668990192@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Juergen Gross <jgross@suse.com>
commit 03b2a320b19f1424e9ac9c21696be9c60b6d0d93 upstream.
The x86_hyper pointer is only used for checking whether a virtual
device is supporting the hypervisor the system is running on.
Use an enum for that purpose instead and drop the x86_hyper pointer.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: akataria@vmware.com
Cc: arnd@arndb.de
Cc: boris.ostrovsky@oracle.com
Cc: devel@linuxdriverproject.org
Cc: dmitry.torokhov@gmail.com
Cc: gregkh@linuxfoundation.org
Cc: haiyangz@microsoft.com
Cc: kvm@vger.kernel.org
Cc: kys@microsoft.com
Cc: linux-graphics-maintainer@vmware.com
Cc: linux-input@vger.kernel.org
Cc: moltmann@vmware.com
Cc: pbonzini@redhat.com
Cc: pv-drivers@vmware.com
Cc: rkrcmar@redhat.com
Cc: sthemmin@microsoft.com
Cc: virtualization@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/20171109132739.23465-3-jgross@suse.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/x86/hyperv/hv_init.c | 2 +-
arch/x86/include/asm/hypervisor.h | 23 ++++++++++++++---------
arch/x86/kernel/cpu/hypervisor.c | 12 +++++++++---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
arch/x86/kernel/cpu/vmware.c | 4 ++--
arch/x86/kernel/kvm.c | 4 ++--
arch/x86/xen/enlighten_hvm.c | 4 ++--
arch/x86/xen/enlighten_pv.c | 4 ++--
drivers/hv/vmbus_drv.c | 2 +-
drivers/input/mouse/vmmouse.c | 10 ++++------
drivers/misc/vmw_balloon.c | 2 +-
11 files changed, 40 insertions(+), 31 deletions(-)
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -113,7 +113,7 @@ void hyperv_init(void)
u64 guest_id;
union hv_x64_msr_hypercall_contents hypercall_msr;
- if (x86_hyper != &x86_hyper_ms_hyperv)
+ if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return;
/* Allocate percpu VP index */
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -29,6 +29,16 @@
/*
* x86 hypervisor information
*/
+
+enum x86_hypervisor_type {
+ X86_HYPER_NATIVE = 0,
+ X86_HYPER_VMWARE,
+ X86_HYPER_MS_HYPERV,
+ X86_HYPER_XEN_PV,
+ X86_HYPER_XEN_HVM,
+ X86_HYPER_KVM,
+};
+
struct hypervisor_x86 {
/* Hypervisor name */
const char *name;
@@ -36,6 +46,9 @@ struct hypervisor_x86 {
/* Detection routine */
uint32_t (*detect)(void);
+ /* Hypervisor type */
+ enum x86_hypervisor_type type;
+
/* init time callbacks */
struct x86_hyper_init init;
@@ -43,15 +56,7 @@ struct hypervisor_x86 {
struct x86_hyper_runtime runtime;
};
-extern const struct hypervisor_x86 *x86_hyper;
-
-/* Recognized hypervisors */
-extern const struct hypervisor_x86 x86_hyper_vmware;
-extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
-extern const struct hypervisor_x86 x86_hyper_xen_pv;
-extern const struct hypervisor_x86 x86_hyper_xen_hvm;
-extern const struct hypervisor_x86 x86_hyper_kvm;
-
+extern enum x86_hypervisor_type x86_hyper_type;
extern void init_hypervisor_platform(void);
#else
static inline void init_hypervisor_platform(void) { }
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -26,6 +26,12 @@
#include <asm/processor.h>
#include <asm/hypervisor.h>
+extern const struct hypervisor_x86 x86_hyper_vmware;
+extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
+extern const struct hypervisor_x86 x86_hyper_xen_pv;
+extern const struct hypervisor_x86 x86_hyper_xen_hvm;
+extern const struct hypervisor_x86 x86_hyper_kvm;
+
static const __initconst struct hypervisor_x86 * const hypervisors[] =
{
#ifdef CONFIG_XEN_PV
@@ -41,8 +47,8 @@ static const __initconst struct hypervis
#endif
};
-const struct hypervisor_x86 *x86_hyper;
-EXPORT_SYMBOL(x86_hyper);
+enum x86_hypervisor_type x86_hyper_type;
+EXPORT_SYMBOL(x86_hyper_type);
static inline const struct hypervisor_x86 * __init
detect_hypervisor_vendor(void)
@@ -87,6 +93,6 @@ void __init init_hypervisor_platform(voi
copy_array(&h->init, &x86_init.hyper, sizeof(h->init));
copy_array(&h->runtime, &x86_platform.hyper, sizeof(h->runtime));
- x86_hyper = h;
+ x86_hyper_type = h->type;
x86_init.hyper.init_platform();
}
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -254,9 +254,9 @@ static void __init ms_hyperv_init_platfo
#endif
}
-const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
+const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
+ .type = X86_HYPER_MS_HYPERV,
.init.init_platform = ms_hyperv_init_platform,
};
-EXPORT_SYMBOL(x86_hyper_ms_hyperv);
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -205,10 +205,10 @@ static bool __init vmware_legacy_x2apic_
(eax & (1 << VMWARE_PORT_CMD_LEGACY_X2APIC)) != 0;
}
-const __refconst struct hypervisor_x86 x86_hyper_vmware = {
+const __initconst struct hypervisor_x86 x86_hyper_vmware = {
.name = "VMware",
.detect = vmware_platform,
+ .type = X86_HYPER_VMWARE,
.init.init_platform = vmware_platform_setup,
.init.x2apic_available = vmware_legacy_x2apic_available,
};
-EXPORT_SYMBOL(x86_hyper_vmware);
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -544,12 +544,12 @@ static uint32_t __init kvm_detect(void)
return kvm_cpuid_base();
}
-const struct hypervisor_x86 x86_hyper_kvm __refconst = {
+const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.name = "KVM",
.detect = kvm_detect,
+ .type = X86_HYPER_KVM,
.init.x2apic_available = kvm_para_available,
};
-EXPORT_SYMBOL_GPL(x86_hyper_kvm);
static __init int activate_jump_labels(void)
{
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -226,12 +226,12 @@ static uint32_t __init xen_platform_hvm(
return xen_cpuid_base();
}
-const struct hypervisor_x86 x86_hyper_xen_hvm = {
+const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
.name = "Xen HVM",
.detect = xen_platform_hvm,
+ .type = X86_HYPER_XEN_HVM,
.init.init_platform = xen_hvm_guest_init,
.init.x2apic_available = xen_x2apic_para_available,
.init.init_mem_mapping = xen_hvm_init_mem_mapping,
.runtime.pin_vcpu = xen_pin_vcpu,
};
-EXPORT_SYMBOL(x86_hyper_xen_hvm);
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1459,9 +1459,9 @@ static uint32_t __init xen_platform_pv(v
return 0;
}
-const struct hypervisor_x86 x86_hyper_xen_pv = {
+const __initconst struct hypervisor_x86 x86_hyper_xen_pv = {
.name = "Xen PV",
.detect = xen_platform_pv,
+ .type = X86_HYPER_XEN_PV,
.runtime.pin_vcpu = xen_pin_vcpu,
};
-EXPORT_SYMBOL(x86_hyper_xen_pv);
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1534,7 +1534,7 @@ static int __init hv_acpi_init(void)
{
int ret, t;
- if (x86_hyper != &x86_hyper_ms_hyperv)
+ if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return -ENODEV;
init_completion(&probe_event);
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -316,11 +316,9 @@ static int vmmouse_enable(struct psmouse
/*
* Array of supported hypervisors.
*/
-static const struct hypervisor_x86 *vmmouse_supported_hypervisors[] = {
- &x86_hyper_vmware,
-#ifdef CONFIG_KVM_GUEST
- &x86_hyper_kvm,
-#endif
+static enum x86_hypervisor_type vmmouse_supported_hypervisors[] = {
+ X86_HYPER_VMWARE,
+ X86_HYPER_KVM,
};
/**
@@ -331,7 +329,7 @@ static bool vmmouse_check_hypervisor(voi
int i;
for (i = 0; i < ARRAY_SIZE(vmmouse_supported_hypervisors); i++)
- if (vmmouse_supported_hypervisors[i] == x86_hyper)
+ if (vmmouse_supported_hypervisors[i] == x86_hyper_type)
return true;
return false;
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1271,7 +1271,7 @@ static int __init vmballoon_init(void)
* Check if we are running on VMware's hypervisor and bail out
* if we are not.
*/
- if (x86_hyper != &x86_hyper_vmware)
+ if (x86_hyper_type != X86_HYPER_VMWARE)
return -ENODEV;
for (is_2m_pages = 0; is_2m_pages < VMW_BALLOON_NUM_PAGE_SIZES;
^ permalink raw reply
* [PATCH 4.14 064/159] x86/virt, x86/platform: Merge struct x86_hyper into struct x86_platform and struct x86_init
From: Greg Kroah-Hartman @ 2017-12-22 8:45 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, sthemmin, xen-devel, kvm, rkrcmar, Peter Zijlstra,
Greg Kroah-Hartman, boris.ostrovsky, rusty, akataria, stable,
virtualization, haiyangz, pbonzini, devel, Thomas Gleixner,
Linus Torvalds, Ingo Molnar
In-Reply-To: <20171222084623.668990192@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Juergen Gross <jgross@suse.com>
commit f72e38e8ec8869ac0ba5a75d7d2f897d98a1454e upstream.
Instead of x86_hyper being either NULL on bare metal or a pointer to a
struct hypervisor_x86 in case of the kernel running as a guest merge
the struct into x86_platform and x86_init.
This will remove the need for wrappers making it hard to find out what
is being called. With dummy functions added for all callbacks testing
for a NULL function pointer can be removed, too.
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: akataria@vmware.com
Cc: boris.ostrovsky@oracle.com
Cc: devel@linuxdriverproject.org
Cc: haiyangz@microsoft.com
Cc: kvm@vger.kernel.org
Cc: kys@microsoft.com
Cc: pbonzini@redhat.com
Cc: rkrcmar@redhat.com
Cc: rusty@rustcorp.com.au
Cc: sthemmin@microsoft.com
Cc: virtualization@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/20171109132739.23465-2-jgross@suse.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/x86/include/asm/hypervisor.h | 25 +++-------------
arch/x86/include/asm/x86_init.h | 24 ++++++++++++++++
arch/x86/kernel/apic/apic.c | 2 -
arch/x86/kernel/cpu/hypervisor.c | 56 ++++++++++++++++++--------------------
arch/x86/kernel/cpu/mshyperv.c | 2 -
arch/x86/kernel/cpu/vmware.c | 4 +-
arch/x86/kernel/kvm.c | 2 -
arch/x86/kernel/x86_init.c | 9 ++++++
arch/x86/mm/init.c | 2 -
arch/x86/xen/enlighten_hvm.c | 8 ++---
arch/x86/xen/enlighten_pv.c | 2 -
include/linux/hypervisor.h | 8 ++++-
12 files changed, 82 insertions(+), 62 deletions(-)
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -23,6 +23,7 @@
#ifdef CONFIG_HYPERVISOR_GUEST
#include <asm/kvm_para.h>
+#include <asm/x86_init.h>
#include <asm/xen/hypervisor.h>
/*
@@ -35,17 +36,11 @@ struct hypervisor_x86 {
/* Detection routine */
uint32_t (*detect)(void);
- /* Platform setup (run once per boot) */
- void (*init_platform)(void);
+ /* init time callbacks */
+ struct x86_hyper_init init;
- /* X2APIC detection (run once per boot) */
- bool (*x2apic_available)(void);
-
- /* pin current vcpu to specified physical cpu (run rarely) */
- void (*pin_vcpu)(int);
-
- /* called during init_mem_mapping() to setup early mappings. */
- void (*init_mem_mapping)(void);
+ /* runtime callbacks */
+ struct x86_hyper_runtime runtime;
};
extern const struct hypervisor_x86 *x86_hyper;
@@ -58,17 +53,7 @@ extern const struct hypervisor_x86 x86_h
extern const struct hypervisor_x86 x86_hyper_kvm;
extern void init_hypervisor_platform(void);
-extern bool hypervisor_x2apic_available(void);
-extern void hypervisor_pin_vcpu(int cpu);
-
-static inline void hypervisor_init_mem_mapping(void)
-{
- if (x86_hyper && x86_hyper->init_mem_mapping)
- x86_hyper->init_mem_mapping();
-}
#else
static inline void init_hypervisor_platform(void) { }
-static inline bool hypervisor_x2apic_available(void) { return false; }
-static inline void hypervisor_init_mem_mapping(void) { }
#endif /* CONFIG_HYPERVISOR_GUEST */
#endif /* _ASM_X86_HYPERVISOR_H */
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -115,6 +115,18 @@ struct x86_init_pci {
};
/**
+ * struct x86_hyper_init - x86 hypervisor init functions
+ * @init_platform: platform setup
+ * @x2apic_available: X2APIC detection
+ * @init_mem_mapping: setup early mappings during init_mem_mapping()
+ */
+struct x86_hyper_init {
+ void (*init_platform)(void);
+ bool (*x2apic_available)(void);
+ void (*init_mem_mapping)(void);
+};
+
+/**
* struct x86_init_ops - functions for platform specific setup
*
*/
@@ -127,6 +139,7 @@ struct x86_init_ops {
struct x86_init_timers timers;
struct x86_init_iommu iommu;
struct x86_init_pci pci;
+ struct x86_hyper_init hyper;
};
/**
@@ -200,6 +213,15 @@ struct x86_legacy_features {
};
/**
+ * struct x86_hyper_runtime - x86 hypervisor specific runtime callbacks
+ *
+ * @pin_vcpu: pin current vcpu to specified physical cpu (run rarely)
+ */
+struct x86_hyper_runtime {
+ void (*pin_vcpu)(int cpu);
+};
+
+/**
* struct x86_platform_ops - platform specific runtime functions
* @calibrate_cpu: calibrate CPU
* @calibrate_tsc: calibrate TSC, if different from CPU
@@ -218,6 +240,7 @@ struct x86_legacy_features {
* possible in x86_early_init_platform_quirks() by
* only using the current x86_hardware_subarch
* semantics.
+ * @hyper: x86 hypervisor specific runtime callbacks
*/
struct x86_platform_ops {
unsigned long (*calibrate_cpu)(void);
@@ -233,6 +256,7 @@ struct x86_platform_ops {
void (*apic_post_init)(void);
struct x86_legacy_features legacy;
void (*set_legacy_features)(void);
+ struct x86_hyper_runtime hyper;
};
struct pci_dev;
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1645,7 +1645,7 @@ static __init void try_to_enable_x2apic(
* under KVM
*/
if (max_physical_apicid > 255 ||
- !hypervisor_x2apic_available()) {
+ !x86_init.hyper.x2apic_available()) {
pr_info("x2apic: IRQ remapping doesn't support X2APIC mode\n");
x2apic_disable();
return;
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -44,51 +44,49 @@ static const __initconst struct hypervis
const struct hypervisor_x86 *x86_hyper;
EXPORT_SYMBOL(x86_hyper);
-static inline void __init
+static inline const struct hypervisor_x86 * __init
detect_hypervisor_vendor(void)
{
- const struct hypervisor_x86 *h, * const *p;
+ const struct hypervisor_x86 *h = NULL, * const *p;
uint32_t pri, max_pri = 0;
for (p = hypervisors; p < hypervisors + ARRAY_SIZE(hypervisors); p++) {
- h = *p;
- pri = h->detect();
- if (pri != 0 && pri > max_pri) {
+ pri = (*p)->detect();
+ if (pri > max_pri) {
max_pri = pri;
- x86_hyper = h;
+ h = *p;
}
}
- if (max_pri)
- pr_info("Hypervisor detected: %s\n", x86_hyper->name);
-}
-
-void __init init_hypervisor_platform(void)
-{
-
- detect_hypervisor_vendor();
+ if (h)
+ pr_info("Hypervisor detected: %s\n", h->name);
- if (!x86_hyper)
- return;
-
- if (x86_hyper->init_platform)
- x86_hyper->init_platform();
+ return h;
}
-bool __init hypervisor_x2apic_available(void)
+static void __init copy_array(const void *src, void *target, unsigned int size)
{
- return x86_hyper &&
- x86_hyper->x2apic_available &&
- x86_hyper->x2apic_available();
+ unsigned int i, n = size / sizeof(void *);
+ const void * const *from = (const void * const *)src;
+ const void **to = (const void **)target;
+
+ for (i = 0; i < n; i++)
+ if (from[i])
+ to[i] = from[i];
}
-void hypervisor_pin_vcpu(int cpu)
+void __init init_hypervisor_platform(void)
{
- if (!x86_hyper)
+ const struct hypervisor_x86 *h;
+
+ h = detect_hypervisor_vendor();
+
+ if (!h)
return;
- if (x86_hyper->pin_vcpu)
- x86_hyper->pin_vcpu(cpu);
- else
- WARN_ONCE(1, "vcpu pinning requested but not supported!\n");
+ copy_array(&h->init, &x86_init.hyper, sizeof(h->init));
+ copy_array(&h->runtime, &x86_platform.hyper, sizeof(h->runtime));
+
+ x86_hyper = h;
+ x86_init.hyper.init_platform();
}
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -257,6 +257,6 @@ static void __init ms_hyperv_init_platfo
const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
- .init_platform = ms_hyperv_init_platform,
+ .init.init_platform = ms_hyperv_init_platform,
};
EXPORT_SYMBOL(x86_hyper_ms_hyperv);
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -208,7 +208,7 @@ static bool __init vmware_legacy_x2apic_
const __refconst struct hypervisor_x86 x86_hyper_vmware = {
.name = "VMware",
.detect = vmware_platform,
- .init_platform = vmware_platform_setup,
- .x2apic_available = vmware_legacy_x2apic_available,
+ .init.init_platform = vmware_platform_setup,
+ .init.x2apic_available = vmware_legacy_x2apic_available,
};
EXPORT_SYMBOL(x86_hyper_vmware);
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -547,7 +547,7 @@ static uint32_t __init kvm_detect(void)
const struct hypervisor_x86 x86_hyper_kvm __refconst = {
.name = "KVM",
.detect = kvm_detect,
- .x2apic_available = kvm_para_available,
+ .init.x2apic_available = kvm_para_available,
};
EXPORT_SYMBOL_GPL(x86_hyper_kvm);
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -28,6 +28,8 @@ void x86_init_noop(void) { }
void __init x86_init_uint_noop(unsigned int unused) { }
int __init iommu_init_noop(void) { return 0; }
void iommu_shutdown_noop(void) { }
+bool __init bool_x86_init_noop(void) { return false; }
+void x86_op_int_noop(int cpu) { }
/*
* The platform setup functions are preset with the default functions
@@ -81,6 +83,12 @@ struct x86_init_ops x86_init __initdata
.init_irq = x86_default_pci_init_irq,
.fixup_irqs = x86_default_pci_fixup_irqs,
},
+
+ .hyper = {
+ .init_platform = x86_init_noop,
+ .x2apic_available = bool_x86_init_noop,
+ .init_mem_mapping = x86_init_noop,
+ },
};
struct x86_cpuinit_ops x86_cpuinit = {
@@ -101,6 +109,7 @@ struct x86_platform_ops x86_platform __r
.get_nmi_reason = default_get_nmi_reason,
.save_sched_clock_state = tsc_save_sched_clock_state,
.restore_sched_clock_state = tsc_restore_sched_clock_state,
+ .hyper.pin_vcpu = x86_op_int_noop,
};
EXPORT_SYMBOL_GPL(x86_platform);
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -671,7 +671,7 @@ void __init init_mem_mapping(void)
load_cr3(swapper_pg_dir);
__flush_tlb_all();
- hypervisor_init_mem_mapping();
+ x86_init.hyper.init_mem_mapping();
early_memtest(0, max_pfn_mapped << PAGE_SHIFT);
}
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -229,9 +229,9 @@ static uint32_t __init xen_platform_hvm(
const struct hypervisor_x86 x86_hyper_xen_hvm = {
.name = "Xen HVM",
.detect = xen_platform_hvm,
- .init_platform = xen_hvm_guest_init,
- .pin_vcpu = xen_pin_vcpu,
- .x2apic_available = xen_x2apic_para_available,
- .init_mem_mapping = xen_hvm_init_mem_mapping,
+ .init.init_platform = xen_hvm_guest_init,
+ .init.x2apic_available = xen_x2apic_para_available,
+ .init.init_mem_mapping = xen_hvm_init_mem_mapping,
+ .runtime.pin_vcpu = xen_pin_vcpu,
};
EXPORT_SYMBOL(x86_hyper_xen_hvm);
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1462,6 +1462,6 @@ static uint32_t __init xen_platform_pv(v
const struct hypervisor_x86 x86_hyper_xen_pv = {
.name = "Xen PV",
.detect = xen_platform_pv,
- .pin_vcpu = xen_pin_vcpu,
+ .runtime.pin_vcpu = xen_pin_vcpu,
};
EXPORT_SYMBOL(x86_hyper_xen_pv);
--- a/include/linux/hypervisor.h
+++ b/include/linux/hypervisor.h
@@ -7,8 +7,12 @@
* Juergen Gross <jgross@suse.com>
*/
-#ifdef CONFIG_HYPERVISOR_GUEST
-#include <asm/hypervisor.h>
+#ifdef CONFIG_X86
+#include <asm/x86_init.h>
+static inline void hypervisor_pin_vcpu(int cpu)
+{
+ x86_platform.hyper.pin_vcpu(cpu);
+}
#else
static inline void hypervisor_pin_vcpu(int cpu)
{
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Wei Wang @ 2017-12-22 8:45 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: virtio-dev, kvm, mst, qemu-devel, mawilcox, linux-kernel,
virtualization, linux-mm, akpm, mhocko
In-Reply-To: <201712212337.JFC57368.tLFOJFVSFHMOOQ@I-love.SAKURA.ne.jp>
On 12/21/2017 10:37 PM, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
>>> +/**
>>> + * xb_find_set - find the next set bit in a range of bits
>>> + * @xb: the xbitmap to search from
>>> + * @offset: the offset in the range to start searching
>>> + * @size: the size of the range
>>> + *
>>> + * Returns: the found bit or, @size if no set bit is found.
>>> + */
>>> +unsigned long xb_find_set(struct xb *xb, unsigned long size,
>>> + unsigned long offset)
>>> +{
>>> + struct radix_tree_root *root = &xb->xbrt;
>>> + struct radix_tree_node *node;
>>> + void __rcu **slot;
>>> + struct ida_bitmap *bitmap;
>>> + unsigned long index = offset / IDA_BITMAP_BITS;
>>> + unsigned long index_end = size / IDA_BITMAP_BITS;
>>> + unsigned long bit = offset % IDA_BITMAP_BITS;
>>> +
>>> + if (unlikely(offset >= size))
>>> + return size;
>>> +
>>> + while (index <= index_end) {
>>> + unsigned long ret;
>>> + unsigned int nbits = size - index * IDA_BITMAP_BITS;
>>> +
>>> + bitmap = __radix_tree_lookup(root, index, &node, &slot);
>>> +
>>> + if (!node && !bitmap)
>>> + return size;
>>> +
>>> + if (bitmap) {
>>> + if (nbits > IDA_BITMAP_BITS)
>>> + nbits = IDA_BITMAP_BITS;
>>> +
>>> + ret = find_next_bit(bitmap->bitmap, nbits, bit);
>>> + if (ret != nbits)
>>> + return ret + index * IDA_BITMAP_BITS;
>>> + }
>>> + bit = 0;
>>> + index++;
>>> + }
>>> +
>>> + return size;
>>> +}
>>> +EXPORT_SYMBOL(xb_find_set);
>> This is going to be slower than the implementation I sent yesterday. If I
>> call:
>> xb_init(xb);
>> xb_set_bit(xb, ULONG_MAX);
>> xb_find_set(xb, ULONG_MAX, 0);
>>
>> it's going to call __radix_tree_lookup() 16 quadrillion times.
>> My implementation will walk the tree precisely once.
>>
> Yes. Wei's patch still can not work.
> We should start reviewing Matthew's implementation.
It runs without any issue on my machine. I didn't generate an "xbitmap"
executable (I just found adding xbitmap executable causes a build error
due to a Makefile error), instead, I tested it within "main" and it
passed all the tests.
Matthew has implemented a new version, let's start from there.
Best,
Wei
^ permalink raw reply
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Siwei Liu @ 2017-12-22 8:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: mst, sridhar.samudrala, Alexander Duyck, virtualization, netdev,
David Miller
In-Reply-To: <20171220205204.4853f1d1@cakuba.netronome.com>
On Wed, Dec 20, 2017 at 8:52 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Wed, 20 Dec 2017 18:16:30 -0800, Siwei Liu wrote:
>> > The plan is to remove the delay and do the naming in the kernel.
>> > This was suggested by Lennart since udev is only doing naming policy
>> > because kernel names were not repeatable.
>> >
>> > This makes the VF show up as "ethN_vf" on Hyper-V which is user friendly.
>> >
>> > Patch is pending.
>>
>> While it's good to show VF with specific naming to indicate
>> enslavement, I wonder wouldn't it be better to hide this netdev at all
>> from the user space? IMHO this extra device is useless when being
>> enslaved and we may delegate controls (e.g. ethtool) over to the
>> para-virtual device instead? That way it's possible to eliminate the
>> possibility of additional udev setup or modification?
>>
>> I'm not sure if this is consistent with Windows guest or not, but I
>> don't find it _Linux_ user friendly that ethtool doesn't work on the
>> composite interface any more, and I have to end up with finding out
>> the correct enslaved VF I must operate on.
>
> Hiding "low level" netdevs comes up from time to time, and is more
> widely applicable than just to VF bonds. We should find a generic
> solution to that problem.
Wholeheartedly agreed.
Be it a generic virtual bond or virtio-net specific one, there should
be some common code between netvsc and virtio for this type of work.
For avoiding duplicated bugs, consistent (Linux) user experience,
future code refactoring/management, and whatever...
One thing worth to note is that, unlike the Hyper-V netvsc backend
there's currently no equivalent (fine-grained) Linux ndo_* driver
interface that is able to move around MAC address/VLAN filters at
run-time specifically. The OID_RECEIVE_FILTER_MOVE_FILTER request I
mean. That translates to one substantial difference in VF
plumbing/unplumbing sequence: you cannot move the MAC address around
to paravirt device until VF is fully unplugged out of the guest OS. I
don't know what backend changes to be proposed for virtio-net as
helper, but the common code needs to work with both flavors of data
path switching backends and do its job correctly.
Regards,
-Siwei
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Corentin Labbe @ 2017-12-22 8:41 UTC (permalink / raw)
To: Herbert Xu
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171222065724.GA27149@gondor.apana.org.au>
On Fri, Dec 22, 2017 at 05:57:24PM +1100, Herbert Xu wrote:
> On Wed, Nov 29, 2017 at 09:41:18AM +0100, Corentin Labbe wrote:
> > The crypto engine could actually only enqueue hash and ablkcipher request.
> > This patch permit it to enqueue any type of crypto_async_request.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>
> This is going the wrong way. We do not want to expose any of the
> base types such as crypto_alg, crypto_async_request to end-users
> and that includes drivers. Only core API code should touch these
> base types.
>
Hello
It's you that was suggesting using crypto_async_request:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1474434.html
"The only wart with this scheme is that the drivers end up seeing
struct crypto_async_request and will need to convert that to the
respective request types but I couldn't really find a better way."
So I wait for any suggestion.
Regards
Corentin Labbe
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Herbert Xu @ 2017-12-22 6:57 UTC (permalink / raw)
To: Corentin Labbe
Cc: alexandre.torgue, mst, linux-kernel, fabien.dessenne,
virtualization, linux-crypto, mcoquelin.stm32, davem,
linux-arm-kernel
In-Reply-To: <20171129084121.9385-2-clabbe.montjoie@gmail.com>
On Wed, Nov 29, 2017 at 09:41:18AM +0100, Corentin Labbe wrote:
> The crypto engine could actually only enqueue hash and ablkcipher request.
> This patch permit it to enqueue any type of crypto_async_request.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
This is going the wrong way. We do not want to expose any of the
base types such as crypto_alg, crypto_async_request to end-users
and that includes drivers. Only core API code should touch these
base types.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ 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