Re: [RFC/RFT] rtl8187: Protect the config callback from mac80211 with a mutex

[Herton Ronaldo Krzesinski <herton@mandriva.com.br>]

Em Wednesday 30 July 2008 12:02:30 Michael Buesch escreveu:
> On Wednesday 30 July 2008 16:53:13 Herton Ronaldo Krzesinski wrote:
> > Em Wednesday 30 July 2008 10:27:26 Michael Buesch escreveu:
> > > On Wednesday 30 July 2008 15:24:40 Michael Buesch wrote:
<snip>
> > > and the mutex name, of course.
> > >
> > > 	/* Mutex to protect the device configuration data,
> > > 	 * which is foobar and bizzbaz */
> > > 	struct mutex conf_mutex;
> >
> > Yes, it's better this way. About the lock, the problem here is you can't
> > set the channel while transmitting data on 8187 (the card stops working
> > util reset like the comment on the code), so we must enable tx loopback
> > while setting channels, but you can't run rtl8187_config concurrently
> > because one instance may be disabling tx loopback while other is still
> > setting channel, or like the code is today there is a possibility that
> > you set tx loopback forever. The lock could be only in that section.
> >
> > The comment could be:
> > /* Mutex to protect the device configuration data,
> >  * we can't set channels concurrently */
>
> I think you probably want to protect the tx-loopback (enabled or disabled)
> state. That's what you're actually doing implicitely. The problem is not
> any channel concurrency or something like that, but that the
> tx-loopback-enable/disable is not recursive is the real thing we want to
> lock here, probably.

No, the issue is that I must enter in tx-loopback mode to set the channels 
inside rtl8187_config, to avoid transmission of packets, so the hardware 
doesn't halt. So that's why the lock, it must cover the entire section 
of "enable loopback"->"set_chan"->"disable loopback", not only the state of 
TX_CONF register.

>
> Note that I didn't read the code.
> But in general you get the idea. Don't lock code, but lock state data.
> (like the loopback-state data).

--
[]'s
Herton