* 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
* Re: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Michael Ellerman @ 2017-12-22 2:12 UTC (permalink / raw)
To: Vincent Legoll; +Cc: virtualization, Linux Kernel ML, mst
In-Reply-To: <CAEwRq=pVCUPGq4=JLOHQ4gQE0FC+Fotci8+DR_VupgB1BJJB1w@mail.gmail.com>
Vincent Legoll <vincent.legoll@gmail.com> writes:
> Hello,
>
> thanks for the help, and sorry for the poor patch,
>
> On Thu, Dec 21, 2017 at 11:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> This breaks all existing .configs *and* defconfigs that use VIRTIO.
>>
>> Please don't do that.
>>
>> If you make it default y then everything will continue to work.
>
> Do you want me to respin it with the added default ?
Yes please, either that or revert it.
cheers
^ permalink raw reply
* Call for papers - MICRADS 2018 - Salinas, Ecuador
From: MarLe @ 2017-12-22 0:02 UTC (permalink / raw)
To: virtualization
* Proceedings by Springer and indexed by Scopus, etc.
---------------------------------------------------------------------------------------------------------------------------
MICRADS´18 - The 2018 Multidisciplinary International Conference of Research Applied to Defense and Security
Salinas, Ecuador, 18 - 20 April 2018
http://www.micrads.org/
------------------------------------------------------------------------------------------------------------------------------------
SCOPE
MICRADS´18 - The 2018 Multidisciplinary International Conference of Research Applied to Defense and Security, to be held at Salinas, Ecuador, 18 - 20 April 2018, is an international forum for researchers and practitioners to present and discuss the most recent innovations, trends, results, experiences and concerns in the several perspectives of Defense and Security.
We are pleased to invite you to submit your papers to MICRADS´18. They can be written in English, Spanish or Portuguese. All submissions will be reviewed on the basis of relevance, originality, importance and clarity.
TOPICS
Submitted papers should be related with one or more of the main themes proposed for the Conference:
A) Information and Communication Technology in Education;
B) Computer vision in military applications;
C) Engineering Analysis and Signal Processing;
D) Cybersecurity and Cyberdefense;
E) Maritime Security and Safety;
F) Strategy, geopolitics and Oceanopolitics;
G) Defense planning;
H) Leadership (e-leadership);
I) Defense Economics;
J) Defense Logistics;
K) Health informatics in military applications;
L) Simulation in Military Application;
M) Computer Networks, Mobility and Pervasive Systems;
N) Military Marketing;
O) Military Physical Training;
P) Assistive Devices and Wearable Technology;
Q) Naval and Military Engineering;
R) Weapons and Combat Systems;
S) Operational Oceanography.
SUBMISSION AND DECISION
Submitted papers (until 6-page limit) must comply with the format standard (http://www.micrads.org/images/Templates.rar), be written in English, Spanish or Portuguese, 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 Scientific Committee. This information should only be included in the camera-ready version, saved in Word format and uploaded at the conference management system.
All papers will be subjected to a ?double-blind review? by at least two members of the Scientific Committee.
Based on Scientific Committee evaluation, a paper can be rejected or accepted by the Conference Chairs. In the later case, it can be accepted as paper or poster.
The authors of papers accepted as posters must 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 7 minute limit per poster.
The authors of accepted papers will have 15 minutes to present their work in a Conference Work Session; approximately 5 minutes of discussion will follow each presentation.
PUBLICATION AND INDEXING
To ensure that an accepted paper is published, at least one of the authors must be fully registered by the 3 of February 2018, and the paper must comply with the suggested layout and page-limit (until 10 pages). 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.
Papers can be written in English, Spanish or Portuguese. Accepted and registered papers will be published in Proceedings by Springer, in a book of its SIST series, and will be submitted for indexing by ISI, SCOPUS, EI-Compendex, SpingerLink, and Google Scholar.
IMPORTANT DATES
Paper Submission: January 14, 2018
Notification of Acceptance: February 14, 2018
Payment of Registration, to ensure the inclusion of an accepted paper in the conference proceedings: February 18, 2018.
Camera-ready Submission: February 21, 2018
-
Website o MICRADS'18: http://www.micrads.org/
-------
---
This email has been checked for viruses by AVG.
http://www.avg.com
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-21 21:03 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, kvm, mst, qemu-devel, penguin-kernel, mawilcox,
linux-kernel, virtualization, linux-mm, akpm, mhocko
In-Reply-To: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com>
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.
diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h
new file mode 100644
index 000000000000..c008309a9494
--- /dev/null
+++ b/include/linux/xbitmap.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * eXtensible Bitmaps
+ * Copyright (c) 2017 Microsoft Corporation
+ * Author: Matthew Wilcox <mawilcox@microsoft.com>
+ *
+ * eXtensible Bitmaps provide an unlimited-size sparse bitmap facility.
+ * All bits are initially zero.
+ *
+ * Locking is to be provided by the user. No xb_ function is safe to
+ * call concurrently with any other xb_ function.
+ */
+
+#include <linux/idr.h>
+
+struct xb {
+ struct radix_tree_root xbrt;
+};
+
+#define XB_INIT { \
+ .xbrt = RADIX_TREE_INIT(IDR_RT_MARKER | GFP_NOWAIT), \
+}
+#define DEFINE_XB(name) struct xb name = XB_INIT
+
+static inline void xb_init(struct xb *xb)
+{
+ INIT_RADIX_TREE(&xb->xbrt, IDR_RT_MARKER | GFP_NOWAIT);
+}
+
+int xb_set_bit(struct xb *xb, unsigned long bit);
+bool xb_test_bit(const struct xb *xb, unsigned long bit);
+void xb_clear_bit(struct xb *xb, unsigned long bit);
+void xb_zero(struct xb *xb, unsigned long min, unsigned long max);
+void xb_fill(struct xb *xb, unsigned long min, unsigned long max);
+bool xb_find_set(const struct xb *xb, unsigned long max, unsigned long *bit);
+bool xb_find_zero(const struct xb *xb, unsigned long max, unsigned long *bit);
+
+static inline bool xb_empty(const struct xb *xb)
+{
+ return radix_tree_empty(&xb->xbrt);
+}
+
+int __must_check xb_preload(gfp_t);
+
+static inline void xb_preload_end(void)
+{
+ preempt_enable();
+}
diff --git a/lib/Makefile b/lib/Makefile
index d11c48ec8ffd..08a8183c390b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -19,7 +19,7 @@ KCOV_INSTRUMENT_dynamic_debug.o := n
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o dump_stack.o timerqueue.o\
- idr.o int_sqrt.o extable.o \
+ idr.o xbitmap.o int_sqrt.o extable.o \
sha1.o chacha20.o irq_regs.o argv_split.o \
flex_proportions.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index c8d55565fafa..d2bd8feb7b85 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -37,7 +37,7 @@
#include <linux/rcupdate.h>
#include <linux/slab.h>
#include <linux/string.h>
-
+#include <linux/xbitmap.h>
/* Number of nodes in fully populated tree of given height */
static unsigned long height_to_maxnodes[RADIX_TREE_MAX_PATH + 1] __read_mostly;
@@ -77,6 +77,11 @@ static struct kmem_cache *radix_tree_node_cachep;
RADIX_TREE_MAP_SHIFT))
#define IDA_PRELOAD_SIZE (IDA_MAX_PATH * 2 - 1)
+#define XB_INDEX_BITS (BITS_PER_LONG - ilog2(IDA_BITMAP_BITS))
+#define XB_MAX_PATH (DIV_ROUND_UP(XB_INDEX_BITS, \
+ RADIX_TREE_MAP_SHIFT))
+#define XB_PRELOAD_SIZE (XB_MAX_PATH * 2 - 1)
+
/*
* Per-cpu pool of preloaded nodes
*/
@@ -1781,7 +1786,7 @@ void __rcu **radix_tree_next_chunk(const struct radix_tree_root *root,
child = rcu_dereference_raw(node->slots[offset]);
}
- if (!child)
+ if (!child && !is_idr(root))
goto restart;
if (child == RADIX_TREE_RETRY)
break;
@@ -2135,6 +2140,35 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
+/**
+ * xb_preload - preload for xb_set_bit()
+ * @gfp_mask: allocation mask to use for preloading
+ *
+ * Preallocate memory to use for the next call to xb_set_bit(). On success,
+ * return zero, with preemption disabled. On error, return -ENOMEM with
+ * preemption not disabled.
+ */
+int xb_preload(gfp_t gfp)
+{
+ if (!this_cpu_read(ida_bitmap)) {
+ struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
+
+ if (!bitmap)
+ return -ENOMEM;
+ /*
+ * The per-CPU variable is updated with preemption enabled.
+ * If the calling task is unlucky to be scheduled to another
+ * CPU which has no ida_bitmap allocation, it will be detected
+ * when setting a bit (i.e. xb_set_bit()).
+ */
+ bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
+ kfree(bitmap);
+ }
+
+ return __radix_tree_preload(gfp, XB_PRELOAD_SIZE);
+}
+EXPORT_SYMBOL(xb_preload);
+
void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
new file mode 100644
index 000000000000..d596ba247b45
--- /dev/null
+++ b/lib/xbitmap.c
@@ -0,0 +1,396 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * XBitmap implementation
+ * Copyright (c) 2017 Microsoft Corporation
+ * Author: Matthew Wilcox <mawilcox@microsoft.com>
+ */
+
+#include <linux/bitmap.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/xbitmap.h>
+
+/**
+ * xb_set_bit() - Set a bit in the XBitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to set.
+ *
+ * This function is used to set a bit in the xbitmap.
+ *
+ * Return: 0 on success. -ENOMEM if memory could not be allocated.
+ */
+int xb_set_bit(struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+
+ 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 */
+ return -ENOMEM;
+ }
+ bitmap = rcu_dereference_raw(*slot);
+ if (!bitmap) {
+ bitmap = this_cpu_xchg(ida_bitmap, NULL);
+ if (!bitmap)
+ return -ENOMEM;
+ memset(bitmap, 0, sizeof(*bitmap));
+ radix_tree_iter_replace(root, &iter, slot, bitmap);
+ }
+
+ __set_bit(bit, bitmap->bitmap);
+ if (bitmap_full(bitmap->bitmap, IDA_BITMAP_BITS))
+ radix_tree_iter_tag_clear(root, &iter, IDR_FREE);
+ return 0;
+}
+EXPORT_SYMBOL(xb_set_bit);
+
+/**
+ * xb_clear_bit() - Clear a bit in the XBitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to clear.
+ *
+ * This function is used to clear a bit in the xbitmap.
+ */
+void xb_clear_bit(struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+
+ bit %= IDA_BITMAP_BITS;
+ slot = radix_tree_iter_lookup(root, &iter, index);
+ if (!slot)
+ return;
+ bitmap = radix_tree_deref_slot(slot);
+ if (!bitmap)
+ return;
+
+ radix_tree_iter_tag_set(root, &iter, IDR_FREE);
+ __clear_bit(bit, bitmap->bitmap);
+ if (bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS)) {
+ kfree(bitmap);
+ radix_tree_iter_delete(root, &iter, slot);
+ }
+}
+EXPORT_SYMBOL(xb_clear_bit);
+
+/**
+ * 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);
+ first = 0;
+ if (bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS))
+ goto delete;
+ continue;
+delete:
+ kfree(bitmap);
+ radix_tree_iter_delete(root, &iter, slot);
+ }
+}
+EXPORT_SYMBOL(xb_zero);
+
+/**
+ * xb_test_bit() - Test a bit in the xbitmap.
+ * @xb: The XBitmap.
+ * @bit: Index of the bit to test.
+ *
+ * This function is used to test a bit in the xbitmap.
+ *
+ * Return: %true if the bit is set.
+ */
+bool xb_test_bit(const struct xb *xb, unsigned long bit)
+{
+ unsigned long index = bit / IDA_BITMAP_BITS;
+ struct ida_bitmap *bitmap = radix_tree_lookup(&xb->xbrt, index);
+
+ bit %= IDA_BITMAP_BITS;
+
+ if (!bitmap)
+ return false;
+ return test_bit(bit, bitmap->bitmap);
+}
+EXPORT_SYMBOL(xb_test_bit);
+
+/**
+ * xb_find_set() - Find the next set bit in a range of bits.
+ * @xb: The XBitmap.
+ * @max: The maximum position to search.
+ * @bit: The first bit to examine, and on exit, the found bit.
+ *
+ * On entry, @bit points to the index of the first bit to search. On exit,
+ * if this function returns %true, @bit will be updated to the index of the
+ * first found bit. It will not be updated if this function returns %false.
+ *
+ * Return: %true if a set bit was found.
+ */
+bool xb_find_set(const struct xb *xb, unsigned long max, unsigned long *bit)
+{
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+ unsigned long index = *bit / IDA_BITMAP_BITS;
+ unsigned int first = *bit % IDA_BITMAP_BITS;
+ unsigned long maxindex = max / IDA_BITMAP_BITS;
+
+ radix_tree_for_each_slot(slot, &xb->xbrt, &iter, index) {
+ if (iter.index > maxindex)
+ break;
+ bitmap = radix_tree_deref_slot(slot);
+ if (bitmap) {
+ unsigned int nbits = IDA_BITMAP_BITS;
+ if (iter.index == maxindex)
+ nbits = max % IDA_BITMAP_BITS + 1;
+ first = find_next_bit(bitmap->bitmap, nbits, first);
+ if (first != nbits) {
+ *bit = first + iter.index * IDA_BITMAP_BITS;
+ return true;
+ }
+ }
+ first = 0;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(xb_find_set);
+
+/**
+ * xb_find_zero() - Find the next zero bit in a range of bits
+ * @xb: The XBitmap.
+ * @max: The maximum index to search.
+ * @bit: Pointer to an index.
+ *
+ * On entry, @bit points to the index of the first bit to search. On exit,
+ * if this function returns %true, @bit will be updated to the index of the
+ * first found bit. It will not be updated if this function returns %false.
+ *
+ * Return: %true if a clear bit was found.
+ */
+bool xb_find_zero(const struct xb *xb, unsigned long max, unsigned long *bit)
+{
+ void __rcu **slot;
+ struct radix_tree_iter iter;
+ struct ida_bitmap *bitmap;
+ unsigned long index = *bit / IDA_BITMAP_BITS;
+ unsigned long first = *bit % IDA_BITMAP_BITS;
+ unsigned long maxindex = max / IDA_BITMAP_BITS;
+
+ radix_tree_for_each_tagged(slot, &xb->xbrt, &iter, index, IDR_FREE) {
+ unsigned int nbits = IDA_BITMAP_BITS;
+ if (iter.index > maxindex)
+ return false;
+ bitmap = radix_tree_deref_slot(slot);
+ if (!bitmap)
+ break;
+ if (iter.index == maxindex)
+ nbits = max % IDA_BITMAP_BITS + 1;
+ first = find_next_zero_bit(bitmap->bitmap, nbits, first);
+ if (first != nbits)
+ break;
+ first = 0;
+ }
+
+ *bit = first + iter.index * IDA_BITMAP_BITS;
+ return true;
+}
+EXPORT_SYMBOL(xb_find_zero);
+
+#ifndef __KERNEL__
+
+static DEFINE_XB(xb1);
+
+static void xbitmap_check_bit(unsigned long bit)
+{
+ unsigned long nbit = 0;
+
+ xb_preload(GFP_KERNEL);
+ assert(!xb_test_bit(&xb1, bit));
+ assert(xb_set_bit(&xb1, bit) == 0);
+ assert(xb_test_bit(&xb1, bit));
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == bit);
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == bit);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == bit + 1);
+ xb_clear_bit(&xb1, bit);
+ assert(xb_empty(&xb1));
+ xb_clear_bit(&xb1, bit);
+ assert(xb_empty(&xb1));
+ nbit = 0;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+}
+
+static void xbitmap_check_bit_range(void)
+{
+ unsigned long nbit = 0;
+
+ /* Regular test1: node = NULL */
+ xb_preload(GFP_KERNEL);
+ xb_set_bit(&xb1, 700);
+ xb_preload_end();
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == 700);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == 701);
+ xb_zero(&xb1, 0, 1023);
+
+ /*
+ * 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));
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2048, &nbit) == true);
+ assert(nbit == 2000);
+ assert(xb_find_set(&xb1, 2002, &nbit) == true);
+ assert(nbit == 2000);
+ nbit = 2002;
+ assert(xb_find_set(&xb1, 2041, &nbit) == true);
+ assert(nbit == 2040);
+ nbit = 2002;
+ assert(xb_find_set(&xb1, 2040, &nbit) == true);
+ assert(nbit == 2040);
+ nbit = 2000;
+ assert(xb_find_zero(&xb1, 2048, &nbit) == true);
+ assert(nbit == 2002);
+ nbit = 2048;
+ assert(xb_find_zero(&xb1, 2060, &nbit) == true);
+ assert(nbit == 2048);
+ xb_zero(&xb1, 0, 2047);
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2048, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+
+ /*
+ * Overflow tests:
+ * Set bit 1 and ULONG_MAX - 4
+ * Next 1 in [ULONG_MAX - 4, ULONG_MAX) --> ULONG_MAX - 4
+ * Next 1 [ULONG_MAX - 3, ULONG_MAX + 4) --> none
+ * Next 0 [ULONG_MAX - 4, ULONG_MAX + 4) --> none
+ */
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, 1));
+ xb_preload_end();
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, ULONG_MAX - 4));
+ nbit = ULONG_MAX - 4;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == ULONG_MAX - 4);
+ nbit++;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == ULONG_MAX - 3);
+ nbit--;
+ assert(xb_find_zero(&xb1, ULONG_MAX, &nbit) == true);
+ assert(nbit == ULONG_MAX - 3);
+ xb_zero(&xb1, ULONG_MAX - 4, ULONG_MAX);
+ nbit = ULONG_MAX - 10;
+ assert(xb_find_set(&xb1, ULONG_MAX, &nbit) == false);
+ assert(nbit == ULONG_MAX - 10);
+ xb_zero(&xb1, 0, 1);
+ nbit = 0;
+ assert(xb_find_set(&xb1, 2, &nbit) == false);
+ assert(nbit == 0);
+ xb_preload_end();
+}
+
+/* Check that setting an already-full bitmap works */
+static void xbitmap_check_set(unsigned long base)
+{
+ unsigned long i;
+
+ assert(xb_empty(&xb1));
+
+ for (i = 0; i < 64 * 1024; i++) {
+ xb_preload(GFP_KERNEL);
+ assert(xb_set_bit(&xb1, base + i) == 0);
+ xb_preload_end();
+ }
+ for (i = 0; i < 64 * 1024; i++)
+ assert(xb_set_bit(&xb1, base + i) == 0);
+
+ for (i = 0; i < 64 * 1024; i++)
+ xb_clear_bit(&xb1, base + i);
+
+ assert(xb_empty(&xb1));
+}
+
+static void xbitmap_checks(void)
+{
+ xb_init(&xb1);
+ xbitmap_check_bit(0);
+ xbitmap_check_bit(30);
+ xbitmap_check_bit(31);
+ xbitmap_check_bit(62);
+ xbitmap_check_bit(63);
+ xbitmap_check_bit(64);
+ xbitmap_check_bit(700);
+ xbitmap_check_bit(1023);
+ xbitmap_check_bit(1024);
+ xbitmap_check_bit(1025);
+ xbitmap_check_bit((1UL << 63) | (1UL << 24));
+ xbitmap_check_bit((1UL << 63) | (1UL << 24) | 70);
+
+ xbitmap_check_bit_range();
+
+ xbitmap_check_set(0);
+ xbitmap_check_set(1024);
+ xbitmap_check_set(1024 * 64);
+}
+
+int __weak main(void)
+{
+ radix_tree_init();
+ xbitmap_checks();
+}
+#endif
diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h
index ca160270fdfa..6b559ee25def 100644
--- a/tools/include/linux/bitmap.h
+++ b/tools/include/linux/bitmap.h
@@ -37,6 +37,40 @@ static inline void bitmap_zero(unsigned long *dst, int nbits)
}
}
+static inline void __bitmap_clear(unsigned long *map, unsigned int start,
+ int len)
+{
+ unsigned long *p = map + BIT_WORD(start);
+ const unsigned int size = start + len;
+ int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+
+ while (len - bits_to_clear >= 0) {
+ *p &= ~mask_to_clear;
+ len -= bits_to_clear;
+ bits_to_clear = BITS_PER_LONG;
+ mask_to_clear = ~0UL;
+ p++;
+ }
+ if (len) {
+ mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+ *p &= ~mask_to_clear;
+ }
+}
+
+static inline __always_inline void bitmap_clear(unsigned long *map,
+ unsigned int start,
+ unsigned int nbits)
+{
+ if (__builtin_constant_p(nbits) && nbits == 1)
+ __clear_bit(start, map);
+ else if (__builtin_constant_p(start) && IS_ALIGNED(start, 8) &&
+ __builtin_constant_p(nbits) && IS_ALIGNED(nbits, 8))
+ memset((char *)map + start / 8, 0, nbits / 8);
+ else
+ __bitmap_clear(map, start, nbits);
+}
+
static inline void bitmap_fill(unsigned long *dst, unsigned int nbits)
{
unsigned int nlongs = BITS_TO_LONGS(nbits);
diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
index 0ad884452c5c..3c992ae15440 100644
--- a/tools/include/linux/kernel.h
+++ b/tools/include/linux/kernel.h
@@ -13,6 +13,8 @@
#define UINT_MAX (~0U)
#endif
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define PERF_ALIGN(x, a) __PERF_ALIGN_MASK(x, (typeof(x))(a)-1)
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
+
clean:
- $(RM) $(TARGETS) *.o radix-tree.c idr.c generated/map-shift.h
+ $(RM) $(TARGETS) *.o radix-tree.c idr.c xbitmap.c generated/map-shift.h
vpath %.c ../../lib
@@ -34,6 +38,7 @@ $(OFILES): Makefile *.h */*.h generated/map-shift.h \
../../include/linux/*.h \
../../include/asm/*.h \
../../../include/linux/radix-tree.h \
+ ../../../include/linux/xbitmap.h \
../../../include/linux/idr.h
radix-tree.c: ../../../lib/radix-tree.c
@@ -42,6 +47,9 @@ radix-tree.c: ../../../lib/radix-tree.c
idr.c: ../../../lib/idr.c
sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@
+xbitmap.c: ../../../lib/xbitmap.c
+ sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@
+
.PHONY: mapshift
mapshift:
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index c3bc3f364f68..426f32f28547 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -17,6 +17,4 @@
#define pr_debug printk
#define pr_cont printk
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
-
#endif /* _KERNEL_H */
diff --git a/tools/testing/radix-tree/linux/xbitmap.h b/tools/testing/radix-tree/linux/xbitmap.h
new file mode 100644
index 000000000000..61de214542e7
--- /dev/null
+++ b/tools/testing/radix-tree/linux/xbitmap.h
@@ -0,0 +1 @@
+#include "../../../../include/linux/xbitmap.h"
diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 257f3f8aacaa..d112363262ae 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -326,6 +326,10 @@ static void single_thread_tests(bool long_run)
rcu_barrier();
printv(2, "after idr_checks: %d allocated, preempt %d\n",
nr_allocated, preempt_count);
+ xbitmap_checks();
+ rcu_barrier();
+ printv(2, "after xbitmap_checks: %d allocated, preempt %d\n",
+ nr_allocated, preempt_count);
big_gang_check(long_run);
rcu_barrier();
printv(2, "after big_gang_check: %d allocated, preempt %d\n",
diff --git a/tools/testing/radix-tree/test.h b/tools/testing/radix-tree/test.h
index d9c031dbeb1a..8175d6b23b32 100644
--- a/tools/testing/radix-tree/test.h
+++ b/tools/testing/radix-tree/test.h
@@ -38,6 +38,7 @@ void benchmark(void);
void idr_checks(void);
void ida_checks(void);
void ida_thread_tests(void);
+void xbitmap_checks(void);
struct item *
item_tag_set(struct radix_tree_root *root, unsigned long index, int tag);
^ permalink raw reply related
* Question about eliminating virtio-scsi 30s timeout and hot-unplug
From: Jason Behmer via Virtualization @ 2017-12-21 19:29 UTC (permalink / raw)
To: pbonzini; +Cc: virtualization
[-- Attachment #1.1: Type: text/plain, Size: 418 bytes --]
Hi Paolo,
I had a question about this patch that eliminates virtio-scsi timeout on
IOs - https://patchwork.kernel.org/patch/9802047/. How does this work when
we have IOs outstanding to a device that is then hot-unplugged. I'm under
the impression that those IOs will never get returned with any status
(please correct me if I'm wrong), and we will then end up waiting on them
forever with this patch.
Thanks,
Jason
[-- Attachment #1.2: Type: text/html, Size: 556 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: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Vincent Legoll @ 2017-12-21 16:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: virtualization, Linux Kernel ML, mst
In-Reply-To: <87tvwkfj2f.fsf@concordia.ellerman.id.au>
Hello,
thanks for the help, and sorry for the poor patch,
On Thu, Dec 21, 2017 at 11:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> This breaks all existing .configs *and* defconfigs that use VIRTIO.
>
> Please don't do that.
>
> If you make it default y then everything will continue to work.
Do you want me to respin it with the added default ?
Tchuss
--
Vincent Legoll
^ permalink raw reply
* Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Matthew Wilcox @ 2017-12-21 14:18 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, kvm, mst, qemu-devel, penguin-kernel, mawilcox,
linux-kernel, virtualization, linux-mm, akpm, mhocko
In-Reply-To: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com>
First of all, the test-suite doesn't build, so I don't know whether you
ran it or not. Then I added the xb_find_set() call below, and it fails
the assert, so you should probably fix that.
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
index f03a0f9f9e29..b29af08a7597 100644
--- a/lib/xbitmap.c
+++ b/lib/xbitmap.c
@@ -249,11 +249,12 @@ void xbitmap_check_bit(unsigned long bit)
assert(!xb_test_bit(&xb1, bit));
assert(xb_set_bit(&xb1, bit) == 0);
assert(xb_test_bit(&xb1, bit));
- assert(xb_clear_bit(&xb1, bit) == 0);
+ xb_clear_bit(&xb1, bit);
assert(xb_empty(&xb1));
- assert(xb_clear_bit(&xb1, bit) == 0);
+ xb_clear_bit(&xb1, bit);
assert(xb_empty(&xb1));
xb_preload_end();
+ assert(xb_find_set(&xb1, ULONG_MAX, 0) == bit);
}
static void xbitmap_check_bit_range(void)
diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index 34ece7883629..adf36e34dd77 100644
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -1,9 +1,9 @@
# 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 \
On Thu, Dec 21, 2017 at 10:30:06AM +0800, Wei Wang wrote:
> v20 RESEND Changes:
> - fixed the !node path
> - added the test cases for the !node path
> - change __builtin_constant_p(start & 7) to __builtin_constant_p(start)
Why would you do such a thing? Just copy the kernel definitions.
> diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h
> index 108f929..ede1029 100644
> --- a/include/linux/xbitmap.h
> +++ b/include/linux/xbitmap.h
> @@ -35,6 +35,12 @@ static inline void xb_init(struct xb *xb)
> int xb_set_bit(struct xb *xb, unsigned long bit);
> bool xb_test_bit(const struct xb *xb, unsigned long bit);
> void xb_clear_bit(struct xb *xb, unsigned long bit);
> +void xb_clear_bit_range(struct xb *xb, unsigned long start,
> + unsigned long nbits);
This is xb_zero(). I thought we talked about this before?
> +unsigned long xb_find_set(struct xb *xb, unsigned long size,
> + unsigned long offset);
> +unsigned long xb_find_zero(struct xb *xb, unsigned long size,
> + unsigned long offset);
Since you're using xb_find_zero(), I think we need the tags from the IDR.
At that point, I'm not sure there's a point in keeping the xbitmap and the
IDR as separate data structures.
> +/**
> + * 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.
^ permalink raw reply related
* [PATCH v5 4/4] virtio_remoteproc: correct put_device virtio_device.dev
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, ohad, bjorn.andersson, mst, jasowang; +Cc: virtualization
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(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 2946348..b0633fd 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -327,7 +327,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
ret = register_virtio_device(vdev);
if (ret) {
- put_device(&rproc->dev);
+ put_device(&vdev->dev);
dev_err(dev, "failed to register vdev: %d\n", ret);
goto out;
}
--
2.9.4
^ permalink raw reply related
* [PATCH v5 3/4] virtio_vop: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
As mentioned at drivers/base/core.c:
/*
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
*/
so we don't free vdev until vdev->vdev.dev.release be called.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/misc/mic/vop/vop_main.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index a341938..3633202 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -452,10 +452,12 @@ static irqreturn_t vop_virtio_intr_handler(int irq, void *data)
static void vop_virtio_release_dev(struct device *_d)
{
- /*
- * No need for a release method similar to virtio PCI.
- * Provide an empty one to avoid getting a warning from core.
- */
+ struct virtio_device *vdev =
+ container_of(_d, struct virtio_device, dev);
+ struct _vop_vdev *vop_vdev =
+ container_of(vdev, struct _vop_vdev, vdev);
+
+ kfree(vop_vdev);
}
/*
@@ -466,7 +468,7 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
unsigned int offset, struct vop_device *vpdev,
int dnode)
{
- struct _vop_vdev *vdev;
+ struct _vop_vdev *vdev, *reg_dev = NULL;
int ret;
u8 type = ioread8(&d->type);
@@ -497,6 +499,7 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
vdev->c2h_vdev_db = ioread8(&vdev->dc->c2h_vdev_db);
ret = register_virtio_device(&vdev->vdev);
+ reg_dev = vdev;
if (ret) {
dev_err(_vop_dev(vdev),
"Failed to register vop device %u type %u\n",
@@ -512,7 +515,10 @@ static int _vop_add_device(struct mic_device_desc __iomem *d,
free_irq:
vpdev->hw_ops->free_irq(vpdev, vdev->virtio_cookie, vdev);
kfree:
- kfree(vdev);
+ if (reg_dev)
+ put_device(&vdev->vdev.dev);
+ else
+ kfree(vdev);
return ret;
}
@@ -568,7 +574,7 @@ static int _vop_remove_device(struct mic_device_desc __iomem *d,
iowrite8(-1, &dc->h2c_vdev_db);
if (status & VIRTIO_CONFIG_S_DRIVER_OK)
wait_for_completion(&vdev->reset_done);
- kfree(vdev);
+ put_device(&vdev->vdev.dev);
iowrite8(1, &dc->guest_ack);
dev_dbg(&vpdev->dev, "%s %d guest_ack %d\n",
__func__, __LINE__, ioread8(&dc->guest_ack));
--
2.9.4
^ permalink raw reply related
* [PATCH v5 2/4] virtio_pci: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 12:40 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
As mentioned at drivers/base/core.c:
/*
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
*/
so we don't free vp_dev until vp_dev->vdev.dev.release be called.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 1c4797e..48d4d1c 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -513,7 +513,7 @@ static void virtio_pci_release_dev(struct device *_d)
static int virtio_pci_probe(struct pci_dev *pci_dev,
const struct pci_device_id *id)
{
- struct virtio_pci_device *vp_dev;
+ struct virtio_pci_device *vp_dev, *reg_dev = NULL;
int rc;
/* allocate our structure and fill it out */
@@ -551,6 +551,7 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
pci_set_master(pci_dev);
rc = register_virtio_device(&vp_dev->vdev);
+ reg_dev = vp_dev;
if (rc)
goto err_register;
@@ -564,7 +565,10 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
err_probe:
pci_disable_device(pci_dev);
err_enable_device:
- kfree(vp_dev);
+ if (reg_dev)
+ put_device(&vp_dev->vdev.dev);
+ else
+ kfree(vp_dev);
return rc;
}
--
2.9.4
^ permalink raw reply related
* [PATCH v5 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 12:39 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
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(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index bf7ff39..59e36ef 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -303,11 +303,21 @@ void unregister_virtio_driver(struct virtio_driver *driver)
}
EXPORT_SYMBOL_GPL(unregister_virtio_driver);
+/**
+ * register_virtio_device - register virtio device
+ * @dev : virtio device to be registered
+ *
+ * On error, the caller must call put_device on &@dev->dev (and not kfree),
+ * as another code path may have obtained a reference to @dev.
+ *
+ * Returns: 0 on suceess, -error on failure
+ */
int register_virtio_device(struct virtio_device *dev)
{
int err;
dev->dev.bus = &virtio_bus;
+ device_initialize(&dev->dev);
/* Assign a unique device index and hence name. */
err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL);
@@ -330,9 +340,11 @@ int register_virtio_device(struct virtio_device *dev)
INIT_LIST_HEAD(&dev->vqs);
- /* device_register() causes the bus infrastructure to look for a
- * matching driver. */
- err = device_register(&dev->dev);
+ /*
+ * device_add() causes the bus infrastructure to look for a matching
+ * driver.
+ */
+ err = device_add(&dev->dev);
if (err)
ida_simple_remove(&virtio_index_ida, dev->index);
out:
--
2.9.4
^ permalink raw reply related
* [PATCH v5 0/4] use put_device to cleanup resource
From: weiping zhang @ 2017-12-21 12:38 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
Hi,
The main change is split device_register into 2 sperate calls:
device_initalize() and device_add, and then the caller can use
put_device safety when fail to register_virtio_device.
v4->v5:
* virtio: correct some comments
* virtio_remoteproc: use put_device directly, not use temp reg_dev
v3->v4:
* split device_register into device_initialize and devicea_add that
the caller can always use put_device when fail to register virtio
device.
v2->v3:
* virtio: add new helper do get device's status then determine use
put_device or kfree.
v1->v2:
* virtio_pci: add comments in commit message for why using put_device
* virtio_vop: also use put_device int _vop_remove_device
weiping zhang (4):
virtio: split device_register into device_initialize and device_add
virtio_pci: don't kfree device on register failure
virtio_vop: don't kfree device on register failure
virtio_remoteproc: correct put_device virtio_device.dev
drivers/misc/mic/vop/vop_main.c | 20 +++++++++++++-------
drivers/remoteproc/remoteproc_virtio.c | 2 +-
drivers/virtio/virtio.c | 18 +++++++++++++++---
drivers/virtio/virtio_pci_common.c | 8 ++++++--
4 files changed, 35 insertions(+), 13 deletions(-)
--
2.9.4
^ permalink raw reply
* Re: [PATCH v20 0/7] Virtio-balloon Enhancement
From: Matthew Wilcox @ 2017-12-21 12:14 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz@gmail.com, kvm@vger.kernel.org, mst@redhat.com,
Tetsuo Handa, liliang.opensource@gmail.com, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
mawilcox@microsoft.com, nilal@redhat.com, riel@redhat.com,
cornelia.huck@de.ibm.com, mhocko@kernel.org, quan.xu0@gmail.com,
linux-kernel@vger.kernel.org, amit.sh
In-Reply-To: <5A3B2148.8050306@intel.com>
On Thu, Dec 21, 2017 at 10:49:44AM +0800, Wei Wang wrote:
> On 12/21/2017 01:10 AM, Matthew Wilcox wrote:
> One more question is about the return value, why would it be ambiguous? I
> think it is the same as find_next_bit() which returns the found bit or size
> if not found.
Because find_next_bit doesn't reasonably support a bitmap which is
ULONG_MAX in size. The point of XBitmap is to support a bitmap which
is ULONG_MAX in size, so every possible return value is a legitimate
"we found a bit here". There's no value which can possibly be used for
"no bit was found".
^ permalink raw reply
* Re: [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Michael Ellerman @ 2017-12-21 10:53 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: Vincent Legoll
In-Reply-To: <20171209152657.4762-1-vincent.legoll@gmail.com>
Vincent Legoll <vincent.legoll@gmail.com> writes:
> No need to get into the submenu to disable all VIRTIO-related
> config entries.
>
> This makes it easier to disable all VIRTIO config options
> without entering the submenu. It will also enable one
> to see that en/dis-abled state from the outside menu.
>
> This is only intended to change menuconfig UI, not change
> the config dependencies.
>
> Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
> ---
> drivers/virtio/Kconfig | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cff773f15b7e..d485a63a8233 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,7 +5,10 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
> or CONFIG_S390_GUEST.
>
> -menu "Virtio drivers"
> +menuconfig VIRTIO_MENU
> + bool "Virtio drivers"
> +
> +if VIRTIO_MENU
This breaks all existing .configs *and* defconfigs that use VIRTIO.
Please don't do that.
If you make it default y then everything will continue to work.
cheers
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index d485a63a8233..35897649c24f 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -7,6 +7,7 @@ config VIRTIO
menuconfig VIRTIO_MENU
bool "Virtio drivers"
+ default y
if VIRTIO_MENU
^ permalink raw reply related
* Re: virtio: make VIRTIO a menuconfig to ease disabling it all
From: Andrei Vagin @ 2017-12-21 6:44 UTC (permalink / raw)
To: Vincent Legoll; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <20171209152657.4762-1-vincent.legoll@gmail.com>
On Sat, Dec 09, 2017 at 04:26:57PM +0100, Vincent Legoll wrote:
> No need to get into the submenu to disable all VIRTIO-related
> config entries.
>
> This makes it easier to disable all VIRTIO config options
> without entering the submenu. It will also enable one
> to see that en/dis-abled state from the outside menu.
>
> This is only intended to change menuconfig UI, not change
> the config dependencies.
>
> Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
> ---
> drivers/virtio/Kconfig | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cff773f15b7e..d485a63a8233 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -5,7 +5,10 @@ config VIRTIO
> bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
> or CONFIG_S390_GUEST.
>
> -menu "Virtio drivers"
> +menuconfig VIRTIO_MENU
> + bool "Virtio drivers"
Hi Vincent,
make localyesconfig and make localmodconfig doesn't work with this
patch.
My scenario looks like this:
* Create a virtual machine with Ubuntu 14.04 in GCE
* git clone git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
* cd linux-next
* curl -o .config https://raw.githubusercontent.com/avagin/criu/linux-next/scripts/linux-next-config
* make localyesconfig
Without this patch:
$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
CONFIG_VIRTIO_BLK=y
# CONFIG_VIRTIO_BLK_SCSI is not set
CONFIG_SCSI_VIRTIO=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_CONSOLE=y
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_PCI_LEGACY=y
CONFIG_VIRTIO_BALLOON=y
# CONFIG_VIRTIO_INPUT is not set
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set
With this patch:
$ cat .config | grep VIRTIO
CONFIG_BLK_MQ_VIRTIO=y
# CONFIG_VIRTIO_BLK is not set
CONFIG_SCSI_VIRTIO=y
# CONFIG_VIRTIO_NET is not set
CONFIG_CAIF_VIRTIO=y
# CONFIG_VIRTIO_CONSOLE is not set
# CONFIG_HW_RANDOM_VIRTIO is not set
CONFIG_VIRTIO=y
# CONFIG_VIRTIO_MENU is not set
# CONFIG_RPMSG_VIRTIO is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set
You can see that with this patch CONFIG_VIRTIO_BLK is not set.
It is wrong, and the kernel with this config will not be able to boot.
We can add "default y" to fix this problem.
https://travis-ci.org/avagin/linux/jobs/313348334
https://travis-ci.org/avagin/linux/jobs/318491188
Thanks,
Andrei
> +
> +if VIRTIO_MENU
>
> config VIRTIO_PCI
> tristate "PCI driver for virtio devices"
> @@ -79,4 +82,4 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
>
> If unsure, say 'N'.
>
> -endmenu
> +endif # VIRTIO_MENU
^ permalink raw reply
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Jakub Kicinski @ 2017-12-21 4:52 UTC (permalink / raw)
To: Siwei Liu
Cc: mst, sridhar.samudrala, alexander.duyck, virtualization, netdev,
David Miller
In-Reply-To: <CADGSJ22mDyC0Sw2EPW96ZWvMPLnzif+Y9MMouhM-6T03ZaKA0Q@mail.gmail.com>
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.
^ permalink raw reply
* Re: [PATCH v4 4/4] virtio_remoteproc: don't kfree device on register failure
From: weiping zhang @ 2017-12-21 3:08 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <20171220171201.6aa2b68f.cohuck@redhat.com>
2017-12-21 0:12 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
> On Wed, 20 Dec 2017 12:27:33 +0800
> weiping zhang <zwp10758@gmail.com> wrote:
>
>> rproc_virtio_dev_release will be called iff virtio_device.dev's
>> refer count became to 0. Here we should check if we call device_register
>
> "reference count drops to 0"
>
> s/call/called/
rproc_add_virtio_dev is not like other virtio driver, it always call
register_virtio_device,
so commit message would be:
virtio_remoteproc: correct put_device virtio_device.dev
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.
>> or not, if called, put vdev.dev, and then rproc->dev's cleanup will be
>> done in rproc_virtio_dev_release, otherwise we do cleanup directly.
>>
>> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
>> ---
>> drivers/remoteproc/remoteproc_virtio.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 2946348..1073ea3 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -304,7 +304,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> {
>> struct rproc *rproc = rvdev->rproc;
>> struct device *dev = &rproc->dev;
>> - struct virtio_device *vdev = &rvdev->vdev;
>> + struct virtio_device *vdev = &rvdev->vdev, *reg_dev = NULL;
>> int ret;
>>
>> vdev->id.device = id,
>> @@ -326,15 +326,24 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> kref_get(&rvdev->refcount);
>>
>> ret = register_virtio_device(vdev);
>> + reg_dev = vdev;
>> if (ret) {
>> - put_device(&rproc->dev);
>> dev_err(dev, "failed to register vdev: %d\n", ret);
>> goto out;
>> }
>>
>> dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
>>
>> + return 0;
>> +
>> out:
>> + if (reg_dev)
>> + put_device(&vdev->dev);
>> + else {
>> + kref_put(&rvdev->refcount, rproc_vdev_release);
>> + put_device(&rproc->dev);
>> + }
>> +
>> return ret;
>> }
>>
>
> I think in this case using the marker makes a straightforward cleanup
> way too complicated. There's a single way we can get to the out label,
> and that's when register_virtio_device() failed. Switching
> put_device(&rproc->dev) to put_device(@vdev->dev) (what your first
> patch did) seems like the way to go.
OK, put it directly at v5.
> (It also may be good to cc: the maintainers for this driver.)
OK, thanks
^ permalink raw reply
* Re: [PATCH v20 0/7] Virtio-balloon Enhancement
From: Wei Wang @ 2017-12-21 2:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: yang.zhang.wz@gmail.com, kvm@vger.kernel.org, mst@redhat.com,
Tetsuo Handa, liliang.opensource@gmail.com, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
mawilcox@microsoft.com, nilal@redhat.com, riel@redhat.com,
cornelia.huck@de.ibm.com, mhocko@kernel.org, quan.xu0@gmail.com,
linux-kernel@vger.kernel.org, amit.sh
In-Reply-To: <20171220171019.GA12236@bombadil.infradead.org>
On 12/21/2017 01:10 AM, Matthew Wilcox wrote:
> On Wed, Dec 20, 2017 at 04:13:16PM +0000, Wang, Wei W wrote:
>> On Wednesday, December 20, 2017 8:26 PM, Matthew Wilcox wrote:
>>> unsigned long bit;
>>> xb_preload(GFP_KERNEL);
>>> xb_set_bit(xb, 700);
>>> xb_preload_end();
>>> bit = xb_find_set(xb, ULONG_MAX, 0);
>>> assert(bit == 700);
>> This above test will result in "!node with bitmap !=NULL", and it goes to the regular "if (bitmap)" path, which finds 700.
>>
>> A better test would be
>> ...
>> xb_set_bit(xb, 700);
>> assert(xb_find_set(xb, ULONG_MAX, 800) == ULONG_MAX);
>> ...
> I decided to write a test case to show you what I meant, then I discovered
> the test suite didn't build, then the test I wrote took forever to run, so
> I rewrote xb_find_set() using the radix tree iterators. So I have no idea
> what bugs may be in your implementation, but at least this function passes
> the current test suite. Of course, there may be gaps in the test suite.
> And since I changed the API to not have the ambiguous return value, I
> also changed the test suite, and maybe I introduced a bug.
Thanks for the effort. That's actually caused by the previous "!node"
path, which incorrectly changed "index = (index | RADIX_TREE_MAP_MASK) +
1". With the change below, it will run pretty well with the test cases.
if (!node && !bitmap)
return size;
Would you mind to have a try with the v20 RESEND patch that was just
shared? It makes the above change and added the test case you suggested?
One more question is about the return value, why would it be ambiguous?
I think it is the same as find_next_bit() which returns the found bit or
size if not found.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v4 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 2:43 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <CAA70yB4e6Qp_2D+6-RCSVLKvSVxJ_vSeBi8Day7b1pCOzvfD_g@mail.gmail.com>
2017-12-21 10:37 GMT+08:00 weiping zhang <zwp10758@gmail.com>:
> 2017-12-20 23:53 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
>> On Wed, 20 Dec 2017 12:26:25 +0800
>> weiping zhang <zwp10758@gmail.com> wrote:
>>
>> [you used a different mail address in your From: than in your s-o-b:;
>> same for the other patches]
a wrong setting of my email client, correct it next time, thanks.
>>> In order to make caller do a simple cleanup, we split device_register
>>> into device_initialize and device_add. device_initialize always sucess,
>>
>> s/success/succeeds/
>>
>>> the caller can always use put_device when fail to register virtio_device
>>
>> "so the caller can always use put_device when register_virtio_device
>> failed,"
>>
>>> no matter fail at ida_simple_get or at device_add.
>>
>> "no matter whether it failed..."
>>
>>> + *
>>> + * If an error occurs, the caller must use put_device, instead of kfree, because
>>> + * device_initialize and device_add will increase @dev->dev's reference count.
>>
>> That's not correct: It's not because of device_add increasing the
>> reference count (it releases it again on failure), but because another
>> code path may have obtained a reference.
>>
>> What about:
>>
>> "On error, the caller must call put_device on &@dev->dev (and not
>> kfree), as another code path may have obtained a reference to @dev."
>>
> It's good to understand, further more dev->name may has a bit mem leak.
> anyway, I'll correct all comments at V5. Thanks very much.
>>> + *
>>> + * Returns: 0 on suceess, -error on failure
>>> + */
>>> int register_virtio_device(struct virtio_device *dev)
^ permalink raw reply
* Re: [PATCH v4 1/4] virtio: split device_register into device_initialize and device_add
From: weiping zhang @ 2017-12-21 2:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtualization, Michael S . Tsirkin
In-Reply-To: <20171220165316.5545b24c.cohuck@redhat.com>
2017-12-20 23:53 GMT+08:00 Cornelia Huck <cohuck@redhat.com>:
> On Wed, 20 Dec 2017 12:26:25 +0800
> weiping zhang <zwp10758@gmail.com> wrote:
>
> [you used a different mail address in your From: than in your s-o-b:;
> same for the other patches]
>
>> In order to make caller do a simple cleanup, we split device_register
>> into device_initialize and device_add. device_initialize always sucess,
>
> s/success/succeeds/
>
>> the caller can always use put_device when fail to register virtio_device
>
> "so the caller can always use put_device when register_virtio_device
> failed,"
>
>> no matter fail at ida_simple_get or at device_add.
>
> "no matter whether it failed..."
>
>> + *
>> + * If an error occurs, the caller must use put_device, instead of kfree, because
>> + * device_initialize and device_add will increase @dev->dev's reference count.
>
> That's not correct: It's not because of device_add increasing the
> reference count (it releases it again on failure), but because another
> code path may have obtained a reference.
>
> What about:
>
> "On error, the caller must call put_device on &@dev->dev (and not
> kfree), as another code path may have obtained a reference to @dev."
>
It's good to understand, further more dev->name may has a bit mem leak.
anyway, I'll correct all comments at V5. Thanks very much.
>> + *
>> + * Returns: 0 on suceess, -error on failure
>> + */
>> int register_virtio_device(struct virtio_device *dev)
^ permalink raw reply
* [PATCH v20 3/7 RESEND] xbitmap: add more operations
From: Wei Wang @ 2017-12-21 2:30 UTC (permalink / raw)
To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
linux-mm, mst, mhocko, akpm, mawilcox, penguin-kernel
This patch adds support to find next 1 or 0 bit in a xbmitmap range and
clear a range of bits.
More possible optimizations to add in the future:
1) xb_set_bit_range: set a range of bits.
2) when searching a bit, if the bit is not found in the slot, move on to
the next slot directly.
3) add tags to help searching.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Suggested-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/xbitmap.h | 6 ++
lib/xbitmap.c | 205 +++++++++++++++++++++++++++++++++++++++++++
tools/include/linux/bitmap.h | 34 +++++++
tools/include/linux/kernel.h | 2 +
4 files changed, 247 insertions(+)
v20 RESEND Changes:
- fixed the !node path
- added the test cases for the !node path
- change __builtin_constant_p(start & 7) to __builtin_constant_p(start)
diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h
index 108f929..ede1029 100644
--- a/include/linux/xbitmap.h
+++ b/include/linux/xbitmap.h
@@ -35,6 +35,12 @@ static inline void xb_init(struct xb *xb)
int xb_set_bit(struct xb *xb, unsigned long bit);
bool xb_test_bit(const struct xb *xb, unsigned long bit);
void xb_clear_bit(struct xb *xb, unsigned long bit);
+void xb_clear_bit_range(struct xb *xb, unsigned long start,
+ unsigned long nbits);
+unsigned long xb_find_set(struct xb *xb, unsigned long size,
+ unsigned long offset);
+unsigned long xb_find_zero(struct xb *xb, unsigned long size,
+ unsigned long offset);
static inline bool xb_empty(const struct xb *xb)
{
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
index a1c0abd..1c99586 100644
--- a/lib/xbitmap.c
+++ b/lib/xbitmap.c
@@ -3,6 +3,16 @@
#include <linux/bitmap.h>
#include <linux/slab.h>
+/*
+ * Developer notes:
+ * - locks are required to gurantee there is no concurrent
+ * calls of xb_set_bit, xb_clear_bit, xb_clear_bit_range, xb_test_bit,
+ * xb_find_set, or xb_find_clear to operate on the same ida bitmap.
+ * - The current implementation of xb_clear_bit_range, xb_find_set and
+ * xb_find_clear may cause long latency when the bit range to operate
+ * on is super large (e.g. [0, ULONG_MAX]).
+ */
+
/**
* xb_set_bit - set a bit in the xbitmap
* @xb: the xbitmap tree used to record the bit
@@ -72,6 +82,49 @@ void xb_clear_bit(struct xb *xb, unsigned long bit)
EXPORT_SYMBOL(xb_clear_bit);
/**
+ * xb_clear_bit_range - clear a range of bits in the xbitmap
+ * @start: the start of the bit range, inclusive
+ * @nbits: number of bits to clear
+ *
+ * This function is used to clear a range of bits in the xbitmap. If all the
+ * bits in the bitmap are 0, the bitmap will be freed.
+ */
+void xb_clear_bit_range(struct xb *xb, unsigned long start,
+ unsigned long nbits)
+{
+ struct radix_tree_root *root = &xb->xbrt;
+ struct radix_tree_node *node;
+ void __rcu **slot;
+ struct ida_bitmap *bitmap;
+ unsigned long index = start / IDA_BITMAP_BITS;
+ unsigned long bit = start % IDA_BITMAP_BITS;
+
+ if (nbits > ULONG_MAX - start)
+ nbits = ULONG_MAX - start;
+
+ while (nbits) {
+ unsigned int __nbits = min(nbits,
+ (unsigned long)IDA_BITMAP_BITS - bit);
+
+ bitmap = __radix_tree_lookup(root, index, &node, &slot);
+ if (bitmap) {
+ if (__nbits != IDA_BITMAP_BITS)
+ bitmap_clear(bitmap->bitmap, bit, __nbits);
+
+ if (__nbits == IDA_BITMAP_BITS ||
+ bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS)) {
+ kfree(bitmap);
+ __radix_tree_delete(root, node, slot);
+ }
+ }
+ bit = 0;
+ index++;
+ nbits -= __nbits;
+ }
+}
+EXPORT_SYMBOL(xb_clear_bit_range);
+
+/**
* xb_test_bit - test a bit in the xbitmap
* @xb: the xbitmap tree used to record the bit
* @bit: index of the bit to test
@@ -94,6 +147,98 @@ bool xb_test_bit(const struct xb *xb, unsigned long bit)
}
EXPORT_SYMBOL(xb_test_bit);
+/**
+ * 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);
+
+/**
+ * xb_find_zero - find the next zero 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 zero bit is found.
+ */
+unsigned long xb_find_zero(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 (bitmap) {
+ if (nbits > IDA_BITMAP_BITS)
+ nbits = IDA_BITMAP_BITS;
+
+ ret = find_next_zero_bit(bitmap->bitmap, nbits, bit);
+ if (ret != nbits)
+ return ret + index * IDA_BITMAP_BITS;
+ } else {
+ return bit + index * IDA_BITMAP_BITS;
+ }
+ bit = 0;
+ index++;
+ }
+
+ return size;
+}
+EXPORT_SYMBOL(xb_find_zero);
+
#ifndef __KERNEL__
static DEFINE_XB(xb1);
@@ -111,6 +256,64 @@ void xbitmap_check_bit(unsigned long bit)
xb_preload_end();
}
+static void xbitmap_check_bit_range(void)
+{
+ /* Regular test1: node = NULL */
+ xb_preload(GFP_KERNEL);
+ xb_set_bit(&xb1, 700);
+ xb_preload_end();
+ assert(xb_find_set(&xb1, ULONG_MAX, 0) == 700);
+ assert(xb_find_set(&xb1, ULONG_MAX, 800) == ULONG_MAX);
+ xb_clear_bit_range(&xb1, 0, 1024);
+
+ /*
+ * 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));
+ assert(xb_find_set(&xb1, 2048, 0) == 2000);
+ assert(xb_find_set(&xb1, 2002, 2000) == 2000);
+ assert(xb_find_set(&xb1, 2041, 2002) == 2040);
+ assert(xb_find_set(&xb1, 2040, 2002) == 2040);
+ assert(xb_find_zero(&xb1, 2048, 2000) == 2002);
+ assert(xb_find_zero(&xb1, 2060, 2048) == 2048);
+ xb_clear_bit_range(&xb1, 0, 2048);
+ assert(xb_find_set(&xb1, 2048, 0) == 2048);
+ xb_preload_end();
+
+ /*
+ * Overflow tests:
+ * Set bit 1 and ULONG_MAX - 4
+ * Next 1 in [ULONG_MAX - 4, ULONG_MAX) --> ULONG_MAX - 4
+ * Next 1 [ULONG_MAX - 3, ULONG_MAX + 4) --> none
+ * Next 0 [ULONG_MAX - 4, ULONG_MAX + 4) --> none
+ */
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, 1));
+ xb_preload_end();
+ xb_preload(GFP_KERNEL);
+ assert(!xb_set_bit(&xb1, ULONG_MAX - 4));
+ assert(xb_find_set(&xb1, ULONG_MAX, ULONG_MAX - 4) == ULONG_MAX - 4);
+ assert(xb_find_set(&xb1, ULONG_MAX + 4, ULONG_MAX - 3) ==
+ ULONG_MAX + 4);
+ assert(xb_find_zero(&xb1, ULONG_MAX + 4, ULONG_MAX - 4) ==
+ ULONG_MAX + 4);
+ xb_clear_bit_range(&xb1, ULONG_MAX - 4, 4);
+ assert(xb_find_set(&xb1, ULONG_MAX, ULONG_MAX - 10) == ULONG_MAX);
+ xb_clear_bit_range(&xb1, 0, 2);
+ assert(xb_find_set(&xb1, 2, 0) == 2);
+ xb_preload_end();
+}
+
void xbitmap_checks(void)
{
xb_init(&xb1);
@@ -122,6 +325,8 @@ void xbitmap_checks(void)
xbitmap_check_bit(1025);
xbitmap_check_bit((1UL << 63) | (1UL << 24));
xbitmap_check_bit((1UL << 63) | (1UL << 24) | 70);
+
+ xbitmap_check_bit_range();
}
int __weak main(void)
diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h
index ca16027..6b559ee 100644
--- a/tools/include/linux/bitmap.h
+++ b/tools/include/linux/bitmap.h
@@ -37,6 +37,40 @@ static inline void bitmap_zero(unsigned long *dst, int nbits)
}
}
+static inline void __bitmap_clear(unsigned long *map, unsigned int start,
+ int len)
+{
+ unsigned long *p = map + BIT_WORD(start);
+ const unsigned int size = start + len;
+ int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+ unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+
+ while (len - bits_to_clear >= 0) {
+ *p &= ~mask_to_clear;
+ len -= bits_to_clear;
+ bits_to_clear = BITS_PER_LONG;
+ mask_to_clear = ~0UL;
+ p++;
+ }
+ if (len) {
+ mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+ *p &= ~mask_to_clear;
+ }
+}
+
+static inline __always_inline void bitmap_clear(unsigned long *map,
+ unsigned int start,
+ unsigned int nbits)
+{
+ if (__builtin_constant_p(nbits) && nbits == 1)
+ __clear_bit(start, map);
+ else if (__builtin_constant_p(start) && IS_ALIGNED(start, 8) &&
+ __builtin_constant_p(nbits) && IS_ALIGNED(nbits, 8))
+ memset((char *)map + start / 8, 0, nbits / 8);
+ else
+ __bitmap_clear(map, start, nbits);
+}
+
static inline void bitmap_fill(unsigned long *dst, unsigned int nbits)
{
unsigned int nlongs = BITS_TO_LONGS(nbits);
diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
index 0ad8844..3c992ae 100644
--- a/tools/include/linux/kernel.h
+++ b/tools/include/linux/kernel.h
@@ -13,6 +13,8 @@
#define UINT_MAX (~0U)
#endif
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define PERF_ALIGN(x, a) __PERF_ALIGN_MASK(x, (typeof(x))(a)-1)
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Siwei Liu @ 2017-12-21 2:16 UTC (permalink / raw)
To: Stephen Hemminger
Cc: mst, sridhar.samudrala, alexander.duyck, virtualization, netdev,
David Miller
In-Reply-To: <20171219104159.30d9caaf@xeon-e3>
On Tue, Dec 19, 2017 at 10:41 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Stephen Hemminger <stephen@networkplumber.org>
>> Date: Tue, 19 Dec 2017 09:55:48 -0800
>>
>> > could be 10ms, just enough to let udev do its renaming
>>
>> Please, move to some kind of notification or event based handling of
>> this problem.
>>
>> No delay is safe, what if userspace gets swapped out or whatever
>> else might make userspace stall unexpectedly?
>>
>
> 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.
Regards,
-Siwei
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: Add ethtool stats
From: Toshiaki Makita @ 2017-12-21 2:12 UTC (permalink / raw)
To: Jason Wang, David S . Miller, Michael S . Tsirkin; +Cc: netdev, virtualization
In-Reply-To: <ff4eb1b4-e49f-2c5d-8e99-810093643c5c@redhat.com>
On 2017/12/20 17:13, Jason Wang wrote:
> On 2017年12月20日 12:40, Toshiaki Makita wrote:
>> The main purpose of this patch is adding a way of checking per-queue
>> stats.
>> It's useful to debug performance problems on multiqueue environment.
>>
>> $ ethtool -S ens10
>> NIC statistics:
>> rx_packets: 4172939
>> tx_packets: 5855538
>> rx_bytes: 6317757408
>> tx_bytes: 8865151846
>> rx_dropped: 0
>> rx_length_errors: 0
>> rx_frame_errors: 0
>> tx_dropped: 0
>> tx_fifo_errors: 0
>> rx_queue_0_packets: 2090408
>> rx_queue_0_bytes: 3164825094
>> rx_queue_1_packets: 2082531
>> rx_queue_1_bytes: 3152932314
>> tx_queue_0_packets: 2770841
>> tx_queue_0_bytes: 4194955474
>> tx_queue_1_packets: 3084697
>> tx_queue_1_bytes: 4670196372
>>
>> Signed-off-by: Toshiaki Makita<makita.toshiaki@lab.ntt.co.jp>
>
> Thanks for the patch. This is not the first patch that wants to do this.
> Maybe you can go through the previous comments (E.g there's one in
> https://patchwork.ozlabs.org/patch/244413/).
Thanks for pointing it. I was not aware of it.
I checked the previous comments but not sure if there is anything to do.
The main logic is coincidentally the same - splitting percpu stats data
structure into rx and tx ones in receive_queue and send_queue, which has
been acked.
Comments from Ben Hutchings is about stats ordering but it seems no
consensus has been formed since then.
Feedback from Michael is kind of nit-picking ones and does not apply to
mine. He also requested other stats items like vm-exit, but not in the
same patch.
You requested performance numbers. Would you like to have it for this
patch as well?
> For me, I'd expect to add some XDP counters like other device did.
Nice to have it. I guess it would be another patch.
--
Toshiaki Makita
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Siwei Liu @ 2017-12-21 1:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: mst, sridhar.samudrala, alexander.duyck, virtualization, netdev,
David Miller
In-Reply-To: <20171219104159.30d9caaf@xeon-e3>
[-- Attachment #1.1: Type: text/plain, Size: 1504 bytes --]
On Tue, Dec 19, 2017 at 10:41 AM, Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Tue, 19 Dec 2017 13:21:17 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Date: Tue, 19 Dec 2017 09:55:48 -0800
> >
> > > could be 10ms, just enough to let udev do its renaming
> >
> > Please, move to some kind of notification or event based handling of
> > this problem.
> >
> > No delay is safe, what if userspace gets swapped out or whatever
> > else might make userspace stall unexpectedly?
> >
>
> 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.
Regards,
-Siwei
[-- Attachment #1.2: Type: text/html, Size: 2322 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: [RFC PATCH] virtio_net: Extend virtio to use VF datapath when available
From: Jakub Kicinski @ 2017-12-21 0:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Sridhar Samudrala, virtualization, alexander.duyck, netdev
In-Reply-To: <20171221021450-mutt-send-email-mst@kernel.org>
On Thu, 21 Dec 2017 02:15:31 +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 20, 2017 at 02:33:34PM -0800, Jakub Kicinski wrote:
> > On Mon, 18 Dec 2017 16:40:36 -0800, Sridhar Samudrala wrote:
> > > +static int virtio_netdev_event(struct notifier_block *this,
> > > + unsigned long event, void *ptr)
> > > +{
> > > + struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
> > > +
> > > + /* Skip our own events */
> > > + if (event_dev->netdev_ops == &virtnet_netdev)
> > > + return NOTIFY_DONE;
> >
> > I wonder how does this work WRT loop prevention. What if I have two
> > virtio devices with the same MAC, what is preventing them from claiming
> > each other? Is it only the check above (not sure what is "own" in
> > comment referring to)?
>
> I expect we will add a feature bit (VIRTIO_NET_F_MASTER) and it will
> be host's responsibility not to add more than 1 such device.
Do you mean a virtio_net feature bit? That won't stop the loop with
a hyperv device. Unless we want every paravirt device to have such
bit and enforce there can be only one magic bond enslavement active in
the system.
Making the bonding information separate from the slaves seems so much
cleaner... No limitation on device types, or how many bonds user
chooses to create. No MAC matching...
> > I'm worried the check above will not stop virtio from enslaving hyperv
> > interfaces and vice versa, potentially leading to a loop, no? There is
> > also the fact that it would be preferable to share the code between
> > paravirt drivers, to avoid duplicated bugs.
> >
> > My suggestion during the previous discussion was to create a paravirt
> > bond device, which will explicitly tell the OS which interfaces to
> > bond, regardless of which driver they're using. Could be some form of
> > ACPI/FW driver too, I don't know enough about x86 FW to suggest
> > something fully fleshed :(
^ 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