Am Dienstag 15 November 2005 17:41 schrieb Krzysztof Halasa:
> > * The @dev_base list is protected by @dev_base_lock and the rtln
> > * semaphore.
> > *
> > * Pure readers hold dev_base_lock for reading.
> > *
> > * Writers must hold the rtnl semaphore while they loop through the
> > * dev_base list, and hold dev_base_lock for writing when they do the
> > * actual updates. This allows pure readers to access the list even
> > * while a writer is preparing to update it.
>
> You are _not_ protected with dev_base_lock, only the line:
> dev->operstate_useroverride = OPERU_DORMANT;
> is protected. Why don't you do atomic write instead?
Please reread what is written about locking in net/core/dev.c. It has looked a
bit strange to me, too, when I needed to use it for the first time, but my
code does the right thing here. Remember, linkwatch_fire_event() is _NOT_ a
pure reader.
> What does calling from process context change here? That would be true
> with BKL and, say, Linux 2.0 but it's no longer the case. rtnl lock
> protects you from userland, though.
Wrong.
> CPU1: CPU2:
> netif_set_kernel_operstate(newstate) {
> oldstate = dev->operstate_kernel;
> netif_set_kernel_operstate(newstate) {
> oldstate = dev->operstate_kernel;
> if (oldstate != newstate) {
> [1] dev->operstate_kernel = newstate;
> if (oldstate != newstate) {
> [2] dev->operstate_kernel =
> newstate;
>
> smp_wmb();
> linkwatch_fire_event(dev);
> smp_wmb();
> linkwatch_fire_event(dev);
See, I've clearly stated in my patch that I expect the driver to synchronize
calls if netif_set_kernel_operstate(), as does currently expect
netif_carrier_on/off() (documented in netdevice.h). The question for you is
whether such an expectation is totally unacceptable. Looking at your driver,
I see no reason why.
Stefan
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
