On Wed, Sep 07, 2005 at 10:05:30PM -0500, Brian King wrote:
> I reverted the patch to use a spinlock and added a comment.
> How does this look?
Fine with me. Ball is in akpm/Paul's court.
grant
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [
Grant Grundler wrote:
On Thu, Sep 08, 2005 at 08:39:15AM +1000, Paul Mackerras wrote:
Grant Grundler writes:
I would argue it more obvious. People looking at the code
are immediately going to realize it was a deliberate choice to
not use a spinlock.
It achieves exactly the same effect as s
On Thu, Sep 08, 2005 at 08:39:15AM +1000, Paul Mackerras wrote:
> Grant Grundler writes:
>
> > I would argue it more obvious. People looking at the code
> > are immediately going to realize it was a deliberate choice to
> > not use a spinlock.
>
> It achieves exactly the same effect as spin_lock/
Grant Grundler writes:
> I would argue it more obvious. People looking at the code
> are immediately going to realize it was a deliberate choice to
> not use a spinlock.
It achieves exactly the same effect as spin_lock/spin_unlock, just
more verbosely. :)
> > and it precludes the sorts of optimi
On Wed, Sep 07, 2005 at 03:49:37PM +1000, Paul Mackerras wrote:
> Maybe, but it seems like a bad idea to me. It's longer, it's less
> obvious what's happening,
I would argue it more obvious. People looking at the code
are immediately going to realize it was a deliberate choice to
not use a spinlo
Grant Grundler writes:
> Ok this is good example - I see what the problem is.
> You could use the following sequence too then:
> pci_block_user_cfg_access
> pci_save_state
> block_ucfg_access = 1
> mb()
> while (spin_is_locked(&pci_lock
Grant Grundler wrote:
> On Mon, Sep 05, 2005 at 01:31:20PM -0500, Brian King wrote:
>>+void pci_block_user_cfg_access(struct pci_dev *dev)
>>+{
>>+ pci_save_state(dev);
>>+ dev->block_ucfg_access = 1;
>>+ mb();
>>+ while (spin_is_locked(&pci_lock))
>>+ cpu_relax();
>>+}
On Mon, Sep 05, 2005 at 01:31:20PM -0500, Brian King wrote:
> That should work also. Here is an updated patch.
...
The code looks good...but it got me thinking.
...
> +void pci_block_user_cfg_access(struct pci_dev *dev)
> +{
> + pci_save_state(dev);
> + dev->block_ucfg_access = 1;
> +
Grant Grundler wrote:
On Sat, Sep 03, 2005 at 06:37:52PM -0500, Brian King wrote:
...
Without the locking, we introduce a race condition.
CPU 0 CPU 1
pci_block_user_cfg_access
On Sat, Sep 03, 2005 at 06:37:52PM -0500, Brian King wrote:
...
> Without the locking, we introduce a race condition.
>
> CPU 0 CPU 1
>
> pci_block_user_cfg_access
> pci_s
Grant Grundler wrote:
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 un
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
Grant Grundler writes:
> On Fri, Sep 02, 2005 at 06:56:35PM -0500, Brian King wrote:
> > Andrew Morton wrote:
> > >Brian King <[EMAIL PROTECTED]> wrote:
> > >
> > >>+void pci_block_user_cfg_access(struct pci_dev *dev)
> > >>+{
> > >>+ unsigned long flags;
> > >>+
> > >>+ pci_save_state(dev);
> > >
On Fri, Sep 02, 2005 at 06:56:35PM -0500, Brian King wrote:
> Andrew Morton wrote:
> >Brian King <[EMAIL PROTECTED]> wrote:
> >
> >>+void pci_block_user_cfg_access(struct pci_dev *dev)
> >>+{
> >>+ unsigned long flags;
> >>+
> >>+ pci_save_state(dev);
> >>+ spin_lock_irqsave(&pci_lock, flags)
Andrew Morton wrote:
Brian King <[EMAIL PROTECTED]> wrote:
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(&pci_lock, flags);
+ dev->block_ucfg_access = 1;
+ spin_unlock_irqrestore(&pci_loc
Brian King <[EMAIL PROTECTED]> wrote:
>
> +void pci_block_user_cfg_access(struct pci_dev *dev)
> +{
> + unsigned long flags;
> +
> + pci_save_state(dev);
> + spin_lock_irqsave(&pci_lock, flags);
> + dev->block_ucfg_access = 1;
> + spin_unlock_irqrestore(&pci_lock, flags);
Are y
16 matches
Mail list logo
