First off, I'm not sure why you're seeing HTML since Gmail is set to plain
text mode for me.

On Tue, Mar 28, 2017 at 9:18 PM, Brian Brooks <[email protected]> wrote:

> On 03/28 18:50:32, Bill Fischofer wrote:
> > <html><head><meta http-equiv="Content-Type" content="text/html;
> charset=utf-8">
> > <meta name="Generator" content="Microsoft Exchange Server">
> > <!-- converted from text -->
> > <style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt;
> border-left: #800000 2px solid; } --></style></head>
> > <body>
> > <font size="2"><span style="font-size:10pt;"><div class="PlainText">This
> part generates numerous checkpatch warnings and errors. Please<br>
> > run checkpatch and correct for v2.<br>
>
> We ran checkpatch.pl and corrected the issues that made sense. We all
> know that
> checkpatch.pl is not perfect. Please point out the checkpatch.pl issues
> that
> are generated from this patch which are not false positives or not
> destructive
> to programmer readability.
>

Checkpatch does issue a number of spurious warnings relating to the use of
volatile, etc. but the following seem legit:

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars
per line)
#11:
respected, and (W)RR scheduling may be used within queues of the same
priority.

WARNING: 'absense' may be misspelled - perhaps 'absence'?
#16:
concurrent queues. LL/SC and CAS variants exist in cases where absense of

CHECK: No space is necessary after a cast
#171: FILE: platform/linux-generic/include/odp/api/plat/schedule_types.h:65:
+#define ODP_SCHED_GROUP_INVALID ((odp_schedule_group_t) -1)

CHECK: Alignment should match open parenthesis
#225: FILE: platform/linux-generic/include/odp_atomic16.h:45:
+static inline bool __atomic_compare_exchange_16(register __int128 *var,
+ __int128 *exp, register __int128 neu,

CHECK: Alignment should match open parenthesis
#271: FILE: platform/linux-generic/include/odp_atomic16.h:91:
+static inline bool __atomic_compare_exchange_16_frail(register __int128
*var,
+ __int128 *exp, register __int128 neu, bool weak,

CHECK: Alignment should match open parenthesis
#393: FILE: platform/linux-generic/include/odp_atomic16.h:213:
+static inline __int128 __atomic_fetch_or_16(__int128 *var,
+ __int128 mask,

CHECK: multiple assignments should be avoided
#856: FILE: platform/linux-generic/include/odp_llqueue.h:123:
+ llq->u.st.head = llq->u.st.tail = node;

CHECK: multiple assignments should be avoided
#965: FILE: platform/linux-generic/include/odp_llqueue.h:232:
+ llq->u.st.head = llq->u.st.tail = NULL;

CHECK: multiple assignments should be avoided
#1064: FILE: platform/linux-generic/include/odp_llqueue.h:331:
+ llq->u.st.head = llq->u.st.tail = NULL;


CHECK: Please use a blank line after function/struct/union/enum declarations
#1189: FILE: platform/linux-generic/include/odp_llsc.h:64:
+}
+#define ll32(a, b) ll((a), (b))


CHECK: Please use a blank line after function/struct/union/enum declarations
#1205: FILE: platform/linux-generic/include/odp_llsc.h:80:
+}
+#define sc32(a, b, c) sc((a), (b), (c))


CHECK: Please use a blank line after function/struct/union/enum declarations
#1220: FILE: platform/linux-generic/include/odp_llsc.h:95:
+}
+#define ll64(a, b) lld((a), (b))


CHECK: Please use a blank line after function/struct/union/enum declarations
#1236: FILE: platform/linux-generic/include/odp_llsc.h:111:
+}
+#define sc64(a, b, c) scd((a), (b), (c))


CHECK: Please use a blank line after function/struct/union/enum declarations
#1336: FILE: platform/linux-generic/include/odp_llsc.h:211:
+}
+#define ll64(a, b) ll((a), (b))


CHECK: Please use a blank line after function/struct/union/enum declarations
#1357: FILE: platform/linux-generic/include/odp_llsc.h:232:
+}
+#define sc64(a, b, c) sc((a), (b), (c))


CHECK: Alignment should match open parenthesis
#1981: FILE:
platform/linux-generic/include/odp_schedule_ordered_internal.h:131:
+void olock_unlock(const reorder_context_t *rctx, reorder_window_t *rwin,
+ uint32_t lock_index);


WARNING: 'noone' may be misspelled - perhaps 'no one'?
#2402: FILE: platform/linux-generic/odp_queue_scalable.c:95:
+ /* All remaining elements claimed, noone else can enqueue */

CHECK: spaces preferred around that '<<' (ctx:VxV)
#2417: FILE: platform/linux-generic/odp_queue_scalable.c:110:
+ return 1<<i;
         ^

CHECK: Alignment should match open parenthesis
#2438: FILE: platform/linux-generic/odp_queue_scalable.c:131:
+ shm = odp_shm_reserve(name,
+ ring_size * sizeof(odp_buffer_hdr_t *),

CHECK: Alignment should match open parenthesis
#2698: FILE: platform/linux-generic/odp_queue_scalable.c:391:
+ while (__atomic_load_n(&q->qschst.numevts, __ATOMIC_RELAXED) != 0 ||
+ __atomic_load_n(&q->qschst.cur_ticket, __ATOMIC_RELAXED) !=


WARNING: macros should not use a trailing semicolon
#3366: FILE: platform/linux-generic/odp_schedule_scalable.c:55:
+#define __atomic_fetch_max(var, v, mo) do { \
+ /* Evalulate 'v' once */ \
+ __typeof__(v) tmp_v = (v); \
+ __typeof__(*var) old_var = \
+ __atomic_load_n((var), __ATOMIC_RELAXED); \
+ while (tmp_v > old_var) { \
+ /* Attempt to store 'v' in '*var' */ \
+ if (__atomic_compare_exchange_n((var), &old_var, \
+ tmp_v, true, (mo), \
+ (mo))) \
+ break; \
+ /* Else failure, try again (with updated value of \

WARNING: Avoid unnecessary line continuations
#3379: FILE: platform/linux-generic/odp_schedule_scalable.c:68:
+ */ \

CHECK: Alignment should match open parenthesis
#3581: FILE: platform/linux-generic/odp_schedule_scalable.c:270:
+       LDXR8(&q->qschst.cur_ticket,
+      __ATOMIC_ACQUIRE) != ticket)

CHECK: Alignment should match open parenthesis
#3659: FILE: platform/linux-generic/odp_schedule_scalable.c:348:
+static void sched_update_deq(sched_elem_t *q,
+ uint32_t actual, bool atomic) __attribute__((always_inline));

CHECK: Alignment should match open parenthesis
#3661: FILE: platform/linux-generic/odp_schedule_scalable.c:350:
+static inline void sched_update_deq(sched_elem_t *q,
+ uint32_t actual, bool atomic)

CHECK: Alignment should match open parenthesis
#3745: FILE: platform/linux-generic/odp_schedule_scalable.c:434:
+       LDXR8(&q->qschst.cur_ticket,
+      __ATOMIC_ACQUIRE) != ticket)

CHECK: Alignment should match open parenthesis
#3791: FILE: platform/linux-generic/odp_schedule_scalable.c:480:
+static void sched_update_deq_sc(sched_elem_t *q,
+ uint32_t actual, bool atomic) __attribute__((always_inline));

CHECK: Alignment should match open parenthesis
#3793: FILE: platform/linux-generic/odp_schedule_scalable.c:482:
+static inline void sched_update_deq_sc(sched_elem_t *q,
+ uint32_t actual, bool atomic)

CHECK: Alignment should match open parenthesis
#3977: FILE: platform/linux-generic/odp_schedule_scalable.c:666:
+ sg_wanted = __atomic_load_n(&ts->sg_wanted[p],
+ __ATOMIC_ACQUIRE);

CHECK: Alignment should match open parenthesis
#3989: FILE: platform/linux-generic/odp_schedule_scalable.c:678:
+ insert_schedq_in_list(ts,
+ &sg->schedq[p * sg->xfactor +

CHECK: Alignment should match open parenthesis
#4002: FILE: platform/linux-generic/odp_schedule_scalable.c:691:
+ remove_schedq_from_list(ts,
+    &sg->schedq[p * sg->xfactor + x]);

CHECK: multiple assignments should be avoided
#4171: FILE: platform/linux-generic/odp_schedule_scalable.c:860:
+ ts->atomq = atomq = elem;

CHECK: Alignment should match open parenthesis
#4256: FILE: platform/linux-generic/odp_schedule_scalable.c:945:
+ if (odp_unlikely(__atomic_load_n(&rwin->turn,
+ __ATOMIC_ACQUIRE) != sn)) {


CHECK: Alignment should match open parenthesis
#4584: FILE: platform/linux-generic/odp_schedule_scalable.c:1273:
+ __atomic_store_n(&thread_state[thr].sg_sem,
+ 1,

CHECK: Alignment should match open parenthesis
#4656: FILE: platform/linux-generic/odp_schedule_scalable.c:1345:
+ shm = odp_shm_reserve(name ? name : "",
+     (sizeof(sched_group_t) +

CHECK: No space is necessary after a cast
#4664: FILE: platform/linux-generic/odp_schedule_scalable.c:1353:
+ sg = (sched_group_t *) odp_shm_addr(shm);

CHECK: No space is necessary after a cast
#4684: FILE: platform/linux-generic/odp_schedule_scalable.c:1373:
+ return (odp_schedule_group_t) (sgi);

CHECK: Alignment should match open parenthesis
#5054: FILE: platform/linux-generic/odp_schedule_scalable.c:1743:
+ if (odp_schedule_group_join(ODP_SCHED_GROUP_CONTROL,
+ &mask) != 0) {

CHECK: Alignment should match open parenthesis
#5060: FILE: platform/linux-generic/odp_schedule_scalable.c:1749:
+ if (odp_schedule_group_join(ODP_SCHED_GROUP_WORKER,
+ &mask) != 0) {

CHECK: Alignment should match open parenthesis
#5096: FILE: platform/linux-generic/odp_schedule_scalable.c:1785:
+ if (odp_schedule_group_leave(ODP_SCHED_GROUP_CONTROL,
+ &mask) != 0)

CHECK: Alignment should match open parenthesis
#5100: FILE: platform/linux-generic/odp_schedule_scalable.c:1789:
+ if (odp_schedule_group_leave(ODP_SCHED_GROUP_WORKER,
+ &mask) != 0)

CHECK: Please use a blank line after function/struct/union/enum declarations
#5185: FILE: platform/linux-generic/odp_schedule_scalable.c:1874:
+}
+static int thr_rem(odp_schedule_group_t group, int thr)

CHECK: Please use a blank line after function/struct/union/enum declarations
#5192: FILE: platform/linux-generic/odp_schedule_scalable.c:1881:
+}
+static int init_queue(uint32_t queue_index,

CHECK: Please use a blank line after function/struct/union/enum declarations
#5200: FILE: platform/linux-generic/odp_schedule_scalable.c:1889:
+}
+static void destroy_queue(uint32_t queue_index)

CHECK: Please use a blank line after function/struct/union/enum declarations
#5205: FILE: platform/linux-generic/odp_schedule_scalable.c:1894:
+}
+static int sched_queue(uint32_t queue_index)

CHECK: Please use a blank line after function/struct/union/enum declarations
#5211: FILE: platform/linux-generic/odp_schedule_scalable.c:1900:
+}
+static int ord_enq_multi(uint32_t queue_index, void *p_buf_hdr[],


CHECK: No space is necessary after a cast
#5314: FILE: platform/linux-generic/odp_schedule_scalable_ordered.c:38:
+ rwin = (reorder_window_t *) odp_shm_addr(*shm);

CHECK: Alignment should match open parenthesis
#5408: FILE: platform/linux-generic/odp_schedule_scalable_ordered.c:132:
+ if (__atomic_compare_exchange(&rwin->hc,
+ &old, /* Updated on failure */

CHECK: Please don't use multiple blank lines
#5757: FILE: test/common_plat/performance/odp_sched_latency.c:824:

+

total: 0 errors, 54 warnings, 44 checks, 5742 lines checked

NOTE: Ignored message types: BIT_MACRO COMPARISON_TO_NULL
DEPRECATED_VARIABLE NEW_TYPEDEFS SPLIT_STRING SSCANF_TO_KSTRTO

0004-A-scalable-software-scheduler.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


> > <br>
> > Also, this part introduces a number of errors that result in failure<br>
> > to compile using clang.
>
> This is likely true. Is compilation with Clang a blocker?
>

Yes, it is. We require ODP modules to be compilable with either GCC or
clang.


>
>

Reply via email to