Re: [PATCH v2 39/49] IB/hfi1: add sysfs routines

Or Gerlitz
2015-06-15T19:20:21Z

On Mon, Jun 15, 2015 at 10:14 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Jun 15, 2015 at 4:29 PM, Mike Marciniszyn
> <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> 58K LOC with a single non-empty change log line saying "This patch
> series adds the OPA gen1 driver" is surely a nice candidate for all
> times record of how not to submit upstream drivers. Why not sparing
>> +struct hfi1_sc2vl_attr {

what's SC and what's SC-to-VL? aren't you using SM to do such
mappings? if this and the below sysfs work-around/s for the maybe
current lack of SM to configure the port? why do the upstream kernel
has to carry ~hundreds of sysfs entries forever for such
non-documented things?

>> +HFI1_SL2SC_ATTR(0);

same comment on SL2SC

>> +HFI1_VL2MTU_ATTR(0);

same comment on VL2MTU, why in sysfs?!

>> +HFI1_DIAGC_ATTR(rc_resends);
>> +HFI1_DIAGC_ATTR_PCPU(rc_acks, C_SW_CPU_RC_ACKS, CNTR_INVALID_VL);
>> +HFI1_DIAGC_ATTR_PCPU(rc_qacks, C_SW_CPU_RC_QACKS, CNTR_INVALID_VL);
>> +HFI1_DIAGC_ATTR_PCPU(rc_delayed_comp, C_SW_CPU_RC_DELAYED_COMP,
>> + CNTR_INVALID_VL);
>> +HFI1_DIAGC_ATTR(seq_naks);
>> +HFI1_DIAGC_ATTR(rdma_seq);
>> +HFI1_DIAGC_ATTR(rnr_naks);
>> +HFI1_DIAGC_ATTR(other_naks);
>> +HFI1_DIAGC_ATTR(rc_timeouts);
>> +HFI1_DIAGC_ATTR(loop_pkts);
>> +HFI1_DIAGC_ATTR(pkt_drops);
>> +HFI1_DIAGC_ATTR(dmawait);
>> +HFI1_DIAGC_ATTR(unaligned);
>> +HFI1_DIAGC_ATTR(rc_dupreq);
>> +HFI1_DIAGC_ATTR(rc_seqnak);

this belongs to the IB core, in a similar manner for the iWARP
counters which are reported from there.