lkml.org 
[lkml]   [2014]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH part1 v5 5/7] PCI: Add pci_dummy_ops to isolate pci device temporarily
From
Date
On Mon, 2014-02-10 at 15:59 +0800, Yijing Wang wrote:
> Hi Oliver,
> Thanks for your review and comments!
>
> >> +static DEFINE_SPINLOCK(pci_freeze_lock);
> >
> > The lock is used only here.
>
> Also be used in pci_bus_unfreeze_device();

Sorry, I meant only in this patch.

>
> >
> >> +/**
> >> + * pci_bus_freeze_device - freeze pci bus to access pci device
> >> + * @bus: the pci bus to freeze
> >> + *
> >> + * Replace pci bus ops by pci_dummy_ops, protect system from
> >> + * accessing pci devices.
> >> + */
> >> +void pci_bus_freeze_device(struct pci_bus *bus)
> >> +{
> >> + struct pci_ops *ops;
> >> + unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&pci_freeze_lock, flags);
> >> + ops = pci_bus_set_ops(bus, &pci_dummy_ops);
> >> + bus->save_ops = ops;
> >> + spin_unlock_irqrestore(&pci_freeze_lock, flags);
> >
> > Against what exactly are you locking here?
>
> I want to use this spin lock to serialize freeze device and unfreeze device.

Yes, but against what? I am sorry I should have been more explicit.
You are using these functions only in pci_scan_single_device()


CPU A CPU B
pci_bus_freeze_device() wait
bus->save_ops = ops {valid} wait
... pci_bus_freeze_device()
wait bus->save_ops = ops
{pci_dummy_ops !}
pci_bus_unfreeze_device() wait
pci_bus_set_ops(bus, bus->save_ops)

You see the problem?

If this function ever races with itself, the locking is useless.
If it doesn't race with itself, the locking is not needed.
If this function can really race with itself, you need a refcount
for freezing.

Regards
Oliver




\
 
 \ /
  Last update: 2014-02-11 02:41    [W:0.142 / U:3.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site