[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v8]xen: sched: convert RTDS from time to event driven model
On 03/12/2016 05:36 PM, Andrew Cooper wrote:
On 12/03/2016 22:21, Chen, Tianyang wrote:
On 03/11/2016 11:54 PM, Meng Xu wrote:
I'm focusing on the style and the logic in the replenish handler:
/*
@@ -160,6 +180,7 @@ struct rt_private {
*/
struct rt_vcpu {
struct list_head q_elem; /* on the runq/depletedq list */
+ struct list_head replq_elem;/* on the repl event list */
missing space before /*
I wasn't sure if all the comments should be lined up or not. Maybe
there should be one more space for all the fields so they nicely line up?
At the very least, there should be a space between the ; and /
If you were introducing the entire structure at once then aligned
comments would be better, or if you were submitting a larger series with
some cleanup patches, then modifying the existing layout would be
acceptable.
In this case however, I would avoid aligning all the comments, as it
detracts from the clarity of the patch (whose purpose is a functional
change).
Right.
+static int
+deadline_queue_insert(struct rt_vcpu * (*_get_q_elem)(struct
list_head *elem),
+ struct rt_vcpu *svc, struct list_head *elem, struct list_head
*queue)
+{
+ struct list_head *iter;
+ int pos = 0;
+
+ list_for_each(iter, queue)
space after ( and before )
If not sure about the space, we can refer to the sched_credit.c for
the style as well, beside the CODING_STYLE file. :-)
Ok. But in __runq_pick() there is no space. Also there is no space in
the definition of this macro:
#define list_for_each(pos, head) \
Which one should I follow?
Apologies for that. The code is not in a particularly good state, but
we do request that all new code adheres to the guidelines, in a hope
that eventually it will be consistent.
The macro definition won't have spaces because CPP syntax requires that
the ( be immediately following the macro name. The Xen coding
guidelines require that control structures including if, for, while, and
these macro structures (being an extension of a for loop) have spaces
between the control name, and immediately inside the control brackets.
Therefore, it should end up as
...
list_for_each ( iter, queue )
{
...
If you wish, you are more than welcome to submit an additional patch
whose purpose is to specifically fix the pre-existing style issues, but
you are under no obligation to.
~Andrew
Thanks for the clarification.
Tianyang Chen
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
