On 01/02/2022 17:14, Oleksandr Andrushchenko wrote:
On 01.02.22 19:05, Julien Grall wrote:
On 01/02/2022 16:25, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
This helper destroys all the ranges of the rangeset given.
Please note, that it uses rangeset_remove_range which returns an error
code on failure. This error code can be ignored as while destroying all
the ranges no memory allocation is expected, so in this case it must not
fail.
To make sure this remains valid use BUG_ON if that changes in the future.
Suggested-by: Roger Pau Monné <roger....@citrix.com>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
---
xen/common/rangeset.c | 6 ++++++
xen/include/xen/rangeset.h | 3 +++
2 files changed, 9 insertions(+)
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index ea27d651723b..9ca2b06cff22 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -525,6 +525,12 @@ void rangeset_swap(struct rangeset *a, struct rangeset *b)
write_unlock(&b->lock);
}
+void rangeset_reset(struct rangeset *r)
+{
+ /* This doesn't allocate anything and must not fail. */
+ BUG_ON(rangeset_remove_range(r, 0, ~0ULL));
I vaguely recall some discussion in the past (not related to this series) that
we wanted to avoid calling function have side-effect in a BUG_ON(). So if we
decide to remove at compile-time BUG_ON(), there would be no issue.
But then I am not sure about the use of BUG_ON(). Can you outline why crashing
the hypervisor is better than continuing (e.g. use WARN_ON() or ASSERT())?
Non-zero value will indicate we were not able to complete the operation
which must not fail because of the concrete use-case: we remove all the
ranges and it is not expected that this may fail.
Just to make sure this behavior does not change I use BUG_ON here which
if triggered clearly indicates that the behavior has changed and there is
a need in code change.
Right, but that change of behavior may not be noticed during
development. So I think we want to avoid BUG_ON() when this is possible.
I can turn this into WARN_ON instead, but this may lead to memory leaks
or some other errors not handled.
IMHO, this is a bit better but not by much. Looking a
rangeset_destroy(), you should be able to do it without any of the
issues you described here. Something like:
if ( r == NULL )
return;
while ( (x = first_range(r)) != NULL )
destroy_range(r, x);
--
Julien Grall
