Re: [PATCH v3 04/11] PCI/MSI/pSeries: Make quota traversing andrequesting race-safe

[Alexander Gordeev]

On Tue, Dec 10, 2013 at 03:30:20PM -0700, Bjorn Helgaas wrote:
> Can you outline the race and the scenario that leads to incorrect results
> or a crash?  I looked through rtas_setup_msi_irqs() (briefly) and I didn't
> see the way that concurrent calls for different devices could interfere
> with each other.
> 
> I was looking for some place that modifies state, where concurrent calls
> might trample on each other, but it looks like msi_quota_for_device() is
> pretty safe: it traverses a tree, but everything it computes is on the
> stack and it doesn't seem to save results anywhere.  Maybe I'm barking up
> the wrong tree?

Hmm. I've assumed the number of MSIs for a device is cached, and therefore
concurrent calls to msi_quota_for_device() and rtas_change_msi() could race.
But it seems msi_quota_for_device() indeed computes a quota while reading
only device's properties and gains constant result (well, assuming the device
tree is not updated, but this is a different story). Which makes me confused
about this note from a earlier thread:

[quote]
On Sat, 2013-10-05 at 16:20 +0200, Alexander Gordeev wrote:
> So my point is - drivers should first obtain a number of MSIs they *can*
> get, then *derive* a number of MSIs the device is fine with and only then
> request that number. Not terribly different from memory or any other type
> of resource allocation ;)

What if the limit is for a group of devices ? Your interface is racy in
that case, another driver could have eaten into the limit in between the
calls.

Ben.
[/quote]

Some comment from Ben would be nice, but I think the patch could be dropped
for now.

Thanks, Bjorn!

> Bjorn

-- 
Regards,
Alexander Gordeev
agordeev@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/