Messages in this thread | | | Date | Fri, 2 Sep 2005 18:08:54 -0600 | From | Grant Grundler <> | Subject | Re: [PATCH 1/2] pci: Block config access during BIST (resend) |
| |
On Sat, Sep 03, 2005 at 09:11:30AM +1000, Paul Mackerras wrote: > Think about it. Taking the lock ensures that we don't do the > assignment (dev->block_ucfg_access = 1) while any other cpu has the > pci_lock. In other words, the reason for taking the lock is so that > we wait until nobody else is doing an access, not to make others wait.
The block_ucfg_access field is only used when making the choice to use saved state or call the PCI bus cfg accessor. I don't what problem waiting solves here since any CPU already accessing real cfg space will finish what they are doing anyway. ie they already made the choice to access real cfg space. We just need to make sure successive accesses to cfg space for this device only access the saved state. For that, a memory barrier around all uses of block_ucfg_access should be sufficient. Or do you think I'm still drinking the wrong color cool-aid?
> > If you had: > > spin_lock_irqsave(&pci_lock, flags); > > pci_save_state(dev); > > dev->block_ucfg_access = 1; > > spin_unlock_irqrestore(&pci_lock, flags); > > > > Then I could buy your arguement since the flag now implies > > we need to atomically save state and set the flag. > > That's probably a good thing to do to.
One needs to verify pci_lock isn't acquired in pci_save_state() (or some other obvious dead lock).
It would make sense to block pci cfg *writes* to that device while we save the state.
grant - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |