Re: [PATCH] PCI: fix pci_bus_alloc_resource() hang

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hmm.. This resource comparison logic doesn't really appear to make any
sense if the resources can overlap.

And the fact that the resources can be identical implies that they can
overlap...

On Thu, Nov 4, 2010 at 7:36 PM, Bjorn Helgaas <bjorn.helgaas@xxxxxx> wrote:
>
> +static int resource_compare(struct resource *res1, struct resource *res2)
> +{
> +       if (res1->end < res2->end)
> +               return -1;
> +
> +       if (res1->end > res2->end)
> +               return 1;
> +
> +       if (res1->start < res2->start)
> +               return -1;
> +
> +       if (res1->start > res2->start)
> +               return 1;

So think about the (non-exact) overlap case, and imagine that resource
A has the same end as resource B. IOW, B is completely contained
within A. What happens?

Now, we return -1 if A starts before B.

But now imagine having a slightly larger A, so that the end of A is
larger than the end of B. It's a very similar situation to above: B is
completely contained within A, and A clearly starts before B does (and
and doesn't end before). But we return the _opposite_ comparison
result: we would return 1.

That makes no sense to me. In both cases, A completely contained B,
and A was clearly "before" B in the resource hierarchy. And we
returned different values.

So what does the return value of "resource_compare()" actually _mean_
from a semantic standpoint? It doesn't sound like it has a really
valid meaning.

Maybe the function is fine and everything works, but I'd really like
for it to have some kind of semantic meaning, or explanation for why
it compares things the way it does.

I realize that with ranges, there may not _be_ any full ordering, but
I suspect we have some constraints (like perhaps "there is no
_partial_ overlap of ranges") that makes it possible to give it a
well-defined semantic meaning.

So maybe just a comment on why it does what it does.

(But to make me happy, you could also say: if the end is the same,
then an earlier start means that we consider the end to be _later_,
because it means that the resource completely overlaps the other, and
thus even though they have the same end, the end of the resource with
the earlier start is "after" the resource with a later start. Do you
see what I'm incoherently trying to say? So switching the -1/1 returns
for the 'start' compare around would satisfy my "that makes no sense"
comment)

                        Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux