[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v6 1/8] mm: Place unscrubbed pages at the end of pagelist
On 07/08/17 17:57, Boris Ostrovsky wrote:
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index ef84b72..d26b232 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -44,7 +44,16 @@ struct page_info
/* Page is on a free list: ((count_info & PGC_count_mask) ==
0). */
struct {
/* Do TLBs need flushing for safety before next page
use? */
- bool_t need_tlbflush;
+ unsigned long need_tlbflush:1;
You've turned need_tlbflush from bool to unsigned long : 1. But some
of the users use true/false or may rely on the boolean property. So
it sounds like to me you want to use bool bitfields here (and in the
x86 part).
This is what we have (with this series applied):
root@ovs104> git grep need_tlbflush .
common/memory.c: bool need_tlbflush = false;
common/memory.c:
accumulate_tlbflush(&need_tlbflush, &page[j],
common/memory.c: if ( need_tlbflush )
common/page_alloc.c: bool need_tlbflush = false;
common/page_alloc.c: accumulate_tlbflush(&need_tlbflush,
&pg[i],
common/page_alloc.c: if ( need_tlbflush )
* common/page_alloc.c: pg[i].u.free.need_tlbflush =
(page_get_owner(&pg[i]) != NULL);
common/page_alloc.c: if ( pg[i].u.free.need_tlbflush )
include/asm-arm/mm.h: unsigned long need_tlbflush:1;
include/asm-x86/mm.h: unsigned long need_tlbflush:1;
include/xen/mm.h:static inline void accumulate_tlbflush(bool
*need_tlbflush,
include/xen/mm.h: if ( page->u.free.need_tlbflush &&
include/xen/mm.h: (!*need_tlbflush ||
include/xen/mm.h: *need_tlbflush = true;
root@ovs104>
The only possibly boolean-style use is '*' and event that I think is
allowed.
Stand-alone need_tlbflush variables above have nothing to do with this
change.
If you look at it, all of them use bool semantic. Now you mix bool and
int. We are trying to remove that in the new code, so please don't
introduce new one.
I am not sure I see how we are mixing the semantics here. <datatype>:1
is really a tightly-packed bool.
It is not a bool. It is an unsigned int. So if I am not mistaken, if you
assign 0x2, the variable will be 0 unless you do !!(...).
Whilst with bool you would get 1.
Switching to bitfields was, btw, suggested by Jan at some point so if
the two of you agree on how to proceed I can go either way (but by
preference is to keep it as a single-bit bitfield).
If you use a single-bit bitfield of bool (i.e bool need_flush : 1) you
would address both Jan's comment and mine.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
