Re: [PATCH v5 1/6] libnvdimm: nd_region flush callback support
On Thu, Apr 18, 2019 at 9:18 AM Christoph Hellwig <hch(a)infradead.org> wrote:
On Thu, Apr 18, 2019 at 09:05:05AM -0700, Dan Williams wrote:
> > > I'd either add a comment about avoiding retpoline overhead here or
just
> > > make ->flush == NULL mean generic_nvdimm_flush(). Just so that people
don't
> > > get confused by the code.
> >
> > Isn't this premature optimization? I really don't like adding things
> > like this without some numbers to show it's worth it.
>
> I don't think it's premature given this optimization technique is
> already being deployed elsewhere, see:
>
> https://lwn.net/Articles/774347/
For one this one was backed by numbers, and second after feedback
from Linux we switched to the NULL pointer check instead.
Ok I should have noticed the switch to NULL pointer check. However,
the question still stands do we want everyone to run numbers to
justify this optimization, or make it a new common kernel coding
practice to do:
if (!object->op)
generic_op(object);
else
object->op(object);
...in hot paths? I agree with not doing premature optimization in
principle, but this hack is minimally intrusive from a readability
perspective similar to likely()/unlikely() usage which also don't come
with numbers on a per patch basis.