"Stokes, Ian" <ian.sto...@intel.com> writes:
>> -----Original Message-----
>> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
>> Sent: Sunday, March 18, 2018 5:38 PM
>> To: Stokes, Ian <ian.sto...@intel.com>; d...@openvswitch.org
>> Cc: i.maxim...@samsung.com
>> Subject: RE: [ovs-dev] [PATCH v9 2/3] dpif-netdev: Detailed performance
>> stats for PMDs
>>
>> > Checkpatch reports the following:
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #560 FILE: lib/dpif-netdev-perf.h:113:
>> > uint64_t cycles; /* Number of TSC cycles spent in it/ms.
>> */
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #563 FILE: lib/dpif-netdev-perf.h:116:
>> > uint32_t pkts; /* Packets processed in iteration/ms. */
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #564 FILE: lib/dpif-netdev-perf.h:117:
>> > uint32_t upcalls; /* Number of upcalls in iteration/ms. */
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #565 FILE: lib/dpif-netdev-perf.h:118:
>> > uint32_t upcall_cycles; /* Cycles spent in upcalls in
>> iteration/ms. */
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #566 FILE: lib/dpif-netdev-perf.h:119:
>> > uint32_t batches; /* Number of rx batches in iteration/ms.
>> */
>> >
>> > WARNING: Line lacks whitespace around operator
>> > #567 FILE: lib/dpif-netdev-perf.h:120:
>> > uint32_t max_vhost_qfill; /* Maximum fill level encountered in
>> it/ms. */
>>
>> These warnings are pretty silly, given that they complain about comments.
>> Somebody should improve checkpatch.py to skip checking comments for coding
>> style! For now I have modified these comments to avoid the warnings.
Thanks for the report. I'll see if there's a quick enhancement I can
make.
>
> In this case I agree, it probably doesn't make sense.
>
>> > WARNING: Line lacks whitespace around operator
>> > #1218 FILE: lib/dpif-netdev.c:3396:
>> > int rem_qlen = 0, *qlen_p= NULL;
>>
>> OK, this one was real.
:-)
1/7 isn't bad, right :)
>>
>> > > + } else {
>> > > + ds_put_format(str,
>> > > + " Rx packets: %12"PRIu64"\n",
>> > > + 0UL);
>> >
>> > Will cause compilation error ovs OVS travis
>> >
>> > lib/dpif-netdev-perf.c:194:17: error: format '%llu' expects argument
>> > of type 'long long unsigned int', but argument 3 has type 'long unsigned
>> int' [-Werror=format=]
>> > 0UL);
>> >
>> > See link below for further info
>> >
>> > https://travis-ci.org/istokes/ovs/jobs/354225095
>> >
>> > > + }
>> > > + if (tx_packets > 0) {
>> > > + ds_put_format(str,
>> > > + " Tx packets: %12"PRIu64" (%.0f Kpps)\n"
>> > > + " Tx batches: %12"PRIu64" (%.2f pkts/batch)"
>> > > + "\n",
>> > > + tx_packets, (tx_packets / duration) / 1000,
>> > > + tx_batches, 1.0 * tx_packets / tx_batches);
>> > > + } else {
>> > > + ds_put_format(str,
>> > > + " Tx packets: %12"PRIu64"\n"
>> > > + "\n",
>> > > + 0UL);
>> >
>> > Same as above.
>>
>> Fixed the constants to 0ULL. Hope that does the trick.
>>
>> > > +/* This function clears the PMD performance counters from within
>> > > +the PMD
>> > > + * thread or from another thread when the PMD thread is not
>> > > +executing its
>> > > + * poll loop. */
>> > > void
>> > > -pmd_perf_stats_clear(struct pmd_perf_stats *s)
>> > > +pmd_perf_stats_clear_lock(struct pmd_perf_stats *s)
>> > > + OVS_REQUIRES(pmd->stats_mutex)
>> >
>> > Will cause compilation error for OVS Travis build
>> >
>> > lib/dpif-netdev-perf.c:365:18: error: use of undeclared identifier 'pmd'
>> > OVS_REQUIRES(pmd->stats_mutex)
>> >
>> > https://travis-ci.org/istokes/ovs/jobs/354225106
>>
>> Yes, that was a leftover that normal compilation with GCC didn't catch.
>> Should read OVS_REQUIRES(s->stats_mutex).
>>
>> Will include the fixes in v10.
>>
>> Thanks, Jan
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
