On Wed, Feb 27, 2019 at 10:41:03AM +0200, Leon Romanovsky wrote:
> On Wed, Feb 27, 2019 at 09:58:10AM +0200, Gal Pressman wrote:
> > On 26-Feb-19 20:03, Steve Wise wrote:
> > >> +#define efa_stat_inc(dev, stat) \
> > >> + do { \
> > >> + typeof(dev) _dev = dev; \
> > >> + unsigned long flags; \
> > >> + \
> > >> + spin_lock_irqsave(&_dev->stats_lock, flags); \
> > >> + (stat)++; \
> > >> + spin_unlock_irqrestore(&_dev->stats_lock, flags); \
> > >> + } while (0)
> > >> +
> > >
> > >
> > > Would this be more safe as a static inline function where you explicitly
> > > type the parameters?
> >
> > The typeof looks like an overkill.
> > I prefer to keep it as a define, but I will thin it down a little.
You should always avoid define functions - do it only if you need to
meta-program something..
> Generally speaking, we are not super excited to see abstractions over
> basic kernel primitives, especially if such abstractions don't do a lot,
> but hiding locks.
I'm confused what this anyhow.. This looks a lot like an atomic64_t incr.
The atomic version is surely faster than the spinlock version, so this
should probably change..
Jason
