Thread (7 messages) flat view 7 messages, 2 authors, 2016-10-13

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

From: Mintz, Yuval <hidden>
Date: 2016-10-13 10:44:43
Also in: lkml

quoted
quoted
quoted
While I don't mind, you could have argued is that we're not
removing enough, not too much.
I.e., perhaps the rdma_msix_* fields should also have been
ifdef-ed instead. [in which case this solution would not have
worked]
That would add even more #ifdefs though.
I agree. Although I'm never clear on the guidelines for the tradeoff -
How much memory/code is considered too much so that you'd have To
ifdef code out instead of 'wasting'?
[I obviously don't claim 64 bytes of memory hit that threshold]
I don't think code size should ever be a reason for an #ifdef in a .c
file: if the code is well-structured, you can always get the same object code
using if(IS_ENABLED()) checks within the code at better readability or better
compile-time coverage.

Between if(IS_ENABLED()) checks and inline helpers, it usually doesn't matter
much either way as long as the separation between the modules is clear enough.
In the example above, removing the structure fields however would require to
move the debugging output into another inline function though.
Still, the question remains - If we were to allocate X bytes of memory
per-something [in this case, per qed owned PCI function], and that memory
wouldn't be functional without a some CONFIG option enabled,
how big should X become before we'd decide the fields should also be
dependent on the option?
It bears no real relevance to this case, as the fields involved are insignificantly
small. But still - is there a rule of thumb here?
quoted
BTW, are you interested in doing a v2 for this? Or would you prefer if
we'd pick it up from here?
I think it's better if you do a v2, as you better understand the long-term plans. I'd
be happy to test your patch in my randconfig build setup if you like.
Sure.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help