Quoting Or Gerlitz [EMAIL PROTECTED]:
Subject: Re: [PATCH for-2.6.22] IB/cm: improve local id allocation
Michael S. Tsirkin wrote:
IB/cm uses idr for local id allocations, with a running counter
as start_id. This fails to generate distinct ids
static int cm_alloc_id(struct cm_id_private
IB/cm uses idr for local id allocations, with a running counter
as start_id. This fails to generate distinct ids in the scenario where
1. An id is constantly created and destroyed
2. A chunk of ids just beyond the current next_id value is occupied
This in turn leads to an increased
Quoting Roland Dreier [EMAIL PROTECTED]:
Subject: Re: [PATCH for-2.6.22] IB/cm: improve local id allocation
IB/cm uses idr for local id allocations, with a running counter
as start_id. This fails to generate distinct ids in the scenario where
1. An id is constantly created and
+ next_id = (unsigned)id + 1;
what happens when this wraps and becomes negative?
in fact the idr stuff all works with plain signed ints -- could
idr_get_new() ever give a negative id? (too lazy too look at the
source right now)
A quick looks makes it look like idr
Quoting Roland Dreier [EMAIL PROTECTED]:
Subject: Re: [PATCH for-2.6.22] IB/cm: improve local id allocation
lib/idr.c says it returns positive IDs always (actually the comments
say in the range 0 ... 0x7fff). So I guess we would want
something like:
if (!ret)
A quick looks makes it look like idr stuff is *really* not designed to
get a negative input: and note that old code has the wrap-around problem,
too.
So, I think the following would be a better fix:
Yes, that's basically what I just proposed (although see below). It
all looks pretty
True, except INT_MAX isn't defined in kernel headers I think,
so I just put 0x7fff there.
It doesn't really matter (see my other reply) but actually INT_MAX and
others are in linux/kernel.h
- R.
___
general mailing list
Quoting Roland Dreier [EMAIL PROTECTED]:
Subject: Re: [PATCH for-2.6.22] IB/cm: improve local id allocation
A quick looks makes it look like idr stuff is *really* not designed to
get a negative input: and note that old code has the wrap-around problem,
too.
So, I think the following
Yes, that's basically what I just proposed (although see below). It
all looks pretty safe to me... Sean, what do you think about this for
2.6.22?
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index eff591d..5e77b01 100644
--- a/drivers/infiniband/core/cm.c
And since it's a *mask*, we can do it this way if you like:
+ if (!ret)
+ next_id = ((unsigned)id + 1) MAX_ID_MASK;
which might generate a bit less code.
Good point. In fact it is 8 bytes smaller for x86-64 at least, so
this is what I just
10 matches
Mail list logo
