On 29/08/18 14:26, Jan Beulich wrote: >>>> On 29.08.18 at 15:16, <andrew.coop...@citrix.com> wrote: >> On 17/08/18 07:42, Jan Beulich wrote: >>> Restore symmetry between get_page_from_l<N>e(): pv_l1tf_check_l<N>e is >>> uniformly invoked from outside of them. >> I'm not sure what symmetry you are referring to. > Just look at the current state: get_page_from_l[234]e() call > pv_l1tf_check_l[234]e(), but get_page_from_l1e() in the > same situation. Instead alloc_l1_table() does.
I'm afraid that I can't even parse this, but I think I've got the point. IMO, it would be clearer for the commit message to read "is now now uniformly invoked". > >>> They're no longer getting called >>> for non-present PTEs. This way the slightly odd three-way return value >>> meaning of the higher level ones can also be got rid of. >>> >>> Introduce local variables holding the page table entries processed, and >>> use them throughout the loop bodies instead of re-reading them from the >>> page table several times. >>> >>> Signed-off-by: Jan Beulich <jbeul...@suse.com> >> With at least the above query resolved, Reviewed-by: Andrew Cooper >> <andrew.coop...@citrix.com>, but a style recommendation. > Thanks, but please let me know if the above clarifies this. > >>> @@ -1396,8 +1369,7 @@ static int alloc_l1_table(struct page_in >>> if ( ret ) >>> goto out; >>> } >>> - >>> - switch ( ret = get_page_from_l1e(pl1e[i], d, d) ) >>> + else switch ( ret = get_page_from_l1e(pl1e[i], d, d) ) >> else switch isn't a usual construct (here and in ptwr_emulated_update). >> I was expecting the misleading indentation warning to trigger, but GCC >> 8.2 does appear to be happy. > Presumably because then it would also trigger for "else if". The difference is that "else if" is a very common idiom. > >> Still, for code clarity, I'd suggest adding braces to the and indenting >> the switch statement. > At first I meant to do so, then decided against because of the extra > code churn in both places where I've chosen this approach (also > making looking at the patch itself less difficult). Would you be okay > with the re-indentation done in a separate follow-up patch? Yeah - thats fine. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
