I had run the checkpatch.pl, but I did not see these issues. May be it was to do with the way I generated the patch. I will address the ones that make sense.
Do we have to compile for 32b with gcc? From: Bill Fischofer [mailto:[email protected]] Sent: Tuesday, March 28, 2017 10:01 PM To: Brian Brooks Cc: lng-odp-forward; Ola Liljedahl; Kevin Wang; Honnappa Nagarahalli Subject: Re: [lng-odp] [API-NEXT 4/4] A scalable software scheduler 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. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
