Re: [PATCH 1/2] pci: Block config access during BIST (resend)

[Paul Mackerras <paulus@samba.org>]

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 optimization
> > that we do on ppc64 where a cpu that is waiting for a lock can tell
> > give its time slice to the cpu that is holding the lock (on systems
> > where the hypervisor time-slices multiple virtual cpus on one physical
> > cpu).
> 
> relax_cpu() doesn't do that?

No, how can it, when it doesn't get told which virtual cpu to yield
to?  spin_lock knows which virtual cpu to yield to because we store
that in the lock variable.

> > What's wrong with just doing spin_lock/spin_unlock?
> 
> it's not wrong - just misleading IMHO. There is no
> "critical section" in that particular chunk of code.

Other code is using the lock to ensure the atomicity of a compound
action, which involves the testing the flag and taking some action
based on the value of the flag.  We take the lock to preserve that
atomicity.  Locks are often used to make a set of compound actions
atomic with respect to each other, which is what we're doing here.

> If relax_cpu doesn't allow time-slice donation, then I guess
> spinlock/unlock with only a comment inside it explain why
> would be ok too.

Preferable, in fact. :)

Paul.
-
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/