| Message ID | 1473668175-3088-1-git-send-email-dongli.zhang@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 12.09.16 at 10:16, <dongli.zhang@oracle.com> wrote: > This patch cleaned up the code by replacing complicated tlbflush check with > an inline function. We should use this inline function to avoid the long > and complicated to read tlbflush check when implementing TODOs left in > commit a902c12ee45fc9389eb8fe54eeddaf267a555c58. > --- a/xen/include/xen/mm.h > +++ b/xen/include/xen/mm.h > @@ -567,4 +567,15 @@ int prepare_ring_for_helper(struct domain *d, unsigned long gmfn, > struct page_info **_page, void **_va); > void destroy_ring_for_helper(void **_va, struct page_info *page); > > +static inline int page_needs_tlbflush(struct page_info *page, bool and const please. > + bool_t need_tlbflush, bool But really passing this into a function with this name is kind of awkward. Perhaps a better function name would be e.g. accumulate_tlbflush(), and then this parameter would maybe better be the first one. > + uint32_t tlbflush_timestamp, > + uint32_t tlbflush_current_time) I don't think you should pass this into the function ... > +{ > + return page->u.free.need_tlbflush && > + page->tlbflush_timestamp <= tlbflush_current_time && ... and use tlbflush_current_time() here instead. Jan
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 18ff6cf..5b93a01 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -827,10 +827,9 @@ static struct page_info *alloc_heap_pages( BUG_ON(pg[i].count_info != PGC_state_free); pg[i].count_info = PGC_state_inuse; - if ( pg[i].u.free.need_tlbflush && - (pg[i].tlbflush_timestamp <= tlbflush_current_time()) && - (!need_tlbflush || - (pg[i].tlbflush_timestamp > tlbflush_timestamp)) ) + if ( page_needs_tlbflush(&pg[i], need_tlbflush, + tlbflush_timestamp, + tlbflush_current_time()) ) { need_tlbflush = 1; tlbflush_timestamp = pg[i].tlbflush_timestamp; diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index 58bc0b8..766559d 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -567,4 +567,15 @@ int prepare_ring_for_helper(struct domain *d, unsigned long gmfn, struct page_info **_page, void **_va); void destroy_ring_for_helper(void **_va, struct page_info *page); +static inline int page_needs_tlbflush(struct page_info *page, + bool_t need_tlbflush, + uint32_t tlbflush_timestamp, + uint32_t tlbflush_current_time) +{ + return page->u.free.need_tlbflush && + page->tlbflush_timestamp <= tlbflush_current_time && + (!need_tlbflush || + page->tlbflush_timestamp > tlbflush_timestamp); +} + #endif /* __XEN_MM_H__ */
This patch cleaned up the code by replacing complicated tlbflush check with an inline function. We should use this inline function to avoid the long and complicated to read tlbflush check when implementing TODOs left in commit a902c12ee45fc9389eb8fe54eeddaf267a555c58. Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> --- Changed since v3: * Wrap the complicated tlbflush condition check as inline function (suggested by Dario). --- xen/common/page_alloc.c | 7 +++---- xen/include/xen/mm.h | 11 +++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-)