[PATCH] Keep a shadow copy of HFA384X_INTEN_OFF

[Pavel Roskin]

On Tue, 20 May 2003, Denis Vlasenko wrote:

> Hmm... this is a small bit of a bigger problem. If we are to guard
> against interrupt / schedule / other CPU hitting us between these
> two lines of code:
> 
>         HFA384X_OUTW(HFA384X_EVENT_MASK, HFA384X_INTEN_OFF);
>         ((local_info_t*)dev->priv)->shadow_hfa384x_inten = HFA384X_EVENT_MASK;

First of all, please make it an inline function or another macro - that's 
the best way to keep data in-sync.

> we are almost inevitably want to protect some surrounding code too,
> because setting IntEn is usually only one stage of some larger
> hw manipulation.

A lot of races can be avoided by enabling interrupts after such
manipulation and disabling them before doing anything drastic (e.g.  
reset), provided that the hardware behaves properly and obeys those
registers.

> In short: if we are worried about this race, we already have
> locking problem (lack of locking) to fix.

I'm not sure that your understanding of Jouni's words is correct.  I don't 
know what race he is worried about.  Please give a realistic scenario.

> I won't pretend that I have experience in writing good SMP/IRQ/preempt
> safe code, but I think I see rather incomplete locking in hostap.

It may be, but hostap has 4 locks, which is quite impressive.

> hostap_ioctl -> prism2_ioctl_siwrate -> hostap_set_rate,
> no locks/semaphores held, we arrive here:

The orinoco driver holds a lock when processing ioctl.  You may want to 
check that code.  David Gibson is religious about locking.

> ret = (hostap_set_word(dev, HFA384X_RID_TXRATECONTROL,
>                        local->tx_rate_control) ||
>        hostap_set_word(dev, HFA384X_RID_CNFSUPPORTEDRATES,
>                        local->tx_rate_control) ||
>        local->func->reset_port(dev));   <--- (see below)
> 
> What protects us between two set_port() calls or second set_port()
> and reset_port()?

The question is what is can happen and how can it affect correctness of
the operation.  I cannot think of anything bad in this case.  RID settings
become affective on reset, i.e. atomically.

-- 
Regards,
Pavel Roskin