Hi
Werner Almesberger wrote:
Michael Trimarchi wrote:
I hope that this one is little bit better :)
*Much* nicer ! :-) However, ...
+ init_rwsem(&ar->arHwAvail);
+ up_write(&ar->arHwAvail);
The rwsem is already unlocked after init_rwsem. So I'm not sure what
the up_write does. Probably nothing good.
Sorry it was removed during test, but I forget to commit.
Did you test if it works ?
+#define DECLARE_LOCKED_VERSION(_name) \
Nice use of macros ! In your original patch, I hadn't even noticed
just how many functions you had to change :)
One small nit: mixed-case names aren't common in the kernel. How
about lock_ar6000 or such ?
Ok
+ ret = _name(dev, info, p, data); \
+ ret = ar6000_hw_avail_unlock(ar); \
+ return ret; \
Ignoring return values ? Tsk, tsk :-) Since it cannot fail, you
probably don't want ar6000_hw_avail_unlock to return a status at all.
This is wrong.
In fact, you could trim this down to simply
AR_SOFTC_T *ar = (AR_SOFTC_T *) netdev_priv(dev); \
int ret = -EAGAIN; \
\
if (down_read_trylock(&ar->arHwAvail)) \
ret = _name(dev, info, p, data); \
up_read(&ar->arHwAvail); \
return ret; \
}
This compiles into even less code than your version (after fixing
it) and you can see all that's happening at one glance.
I test it and the problem disappear, ok I will fix. I see the other patches,
I'm not sure if you just reduce the window or your patch is ok, I must
take a look
to the net part to undestand if it is correct.
Thanks,
- Werner
Michael
