On 04/16/2015 07:05 PM, Weiny, Ira wrote:
>>
>> On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
>>
>>> We can give client->add() callback a return value and make
>>> ib_register_device() return -ENOMEM when it failed, just wondering why
>>> we don't do this at first, any special
Hi, Roland
Thanks for the comment :-)
On 04/16/2015 07:02 PM, Roland Dreier wrote:
> On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
> wrote:
>>> We can give client->add() callback a return value and make
>>> ib_register_device() return -ENOMEM when it failed, just wondering
>>> why we don't
On 04/16/2015 07:05 PM, Weiny, Ira wrote:
On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just wondering why
we don't do this at first, any special reason?
No idea,
Hi, Roland
Thanks for the comment :-)
On 04/16/2015 07:02 PM, Roland Dreier wrote:
On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
jguntho...@obsidianresearch.com wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just
On Thu, Apr 16, 2015 at 10:02:46AM -0700, Roland Dreier wrote:
> On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
> wrote:
> >> We can give client->add() callback a return value and make
> >> ib_register_device() return -ENOMEM when it failed, just wondering
> >> why we don't do this at first,
> > No idea, but having ib_register_device fail and unwind if a client
> > fails to attach makes sense to me.
>
> It seems a bit unfriendly to fail an entire device if one ULP has a
> problem. Let's say you have a system whose main network connection is
> IPoIB. Would you want that connection
On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
wrote:
>> We can give client->add() callback a return value and make
>> ib_register_device() return -ENOMEM when it failed, just wondering
>> why we don't do this at first, any special reason?
> No idea, but having ib_register_device fail and
>
> On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
>
> > We can give client->add() callback a return value and make
> > ib_register_device() return -ENOMEM when it failed, just wondering why
> > we don't do this at first, any special reason?
>
> No idea, but having
On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
> We can give client->add() callback a return value and make
> ib_register_device() return -ENOMEM when it failed, just wondering
> why we don't do this at first, any special reason?
No idea, but having ib_register_device fail and
On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just wondering why
we don't do this at first, any special reason?
No idea, but having ib_register_device fail
On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
jguntho...@obsidianresearch.com wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just wondering
why we don't do this at first, any special reason?
No idea, but having
On Wed, Apr 15, 2015 at 09:58:18AM +0200, Michael Wang wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just wondering
why we don't do this at first, any special reason?
No idea, but having ib_register_device fail and unwind
No idea, but having ib_register_device fail and unwind if a client
fails to attach makes sense to me.
It seems a bit unfriendly to fail an entire device if one ULP has a
problem. Let's say you have a system whose main network connection is
IPoIB. Would you want that connection to come
On Thu, Apr 16, 2015 at 10:02:46AM -0700, Roland Dreier wrote:
On Thu, Apr 16, 2015 at 9:44 AM, Jason Gunthorpe
jguntho...@obsidianresearch.com wrote:
We can give client-add() callback a return value and make
ib_register_device() return -ENOMEM when it failed, just wondering
why we don't
On 04/14/2015 08:21 PM, Jason Gunthorpe wrote:
> On Tue, Apr 14, 2015 at 06:02:47PM +, Hefty, Sean wrote:
>>> Yes, that is the only reasonable thing that could happen.
>>>
>>> init failure should only be possible under exceptional cases (OOM).
>>>
>>> The only system response is to call
On 04/14/2015 08:21 PM, Jason Gunthorpe wrote:
On Tue, Apr 14, 2015 at 06:02:47PM +, Hefty, Sean wrote:
Yes, that is the only reasonable thing that could happen.
init failure should only be possible under exceptional cases (OOM).
The only system response is to call ib_umad_add_one
On Tue, Apr 14, 2015 at 06:02:47PM +, Hefty, Sean wrote:
> > Yes, that is the only reasonable thing that could happen.
> >
> > init failure should only be possible under exceptional cases (OOM).
> >
> > The only system response is to call ib_umad_add_one again - so of
> > course the first
> Yes, that is the only reasonable thing that could happen.
>
> init failure should only be possible under exceptional cases (OOM).
>
> The only system response is to call ib_umad_add_one again - so of
> course the first call had to completely clean up everything it did.
A reasonable follow up
On Tue, Apr 14, 2015 at 01:43:11PM -0400, ira.weiny wrote:
> A failure to init port 2 ends up ends up "killing" port 1 and releasing the
> device associated resources.
Yes, that is the only reasonable thing that could happen.
init failure should only be possible under exceptional cases (OOM).
On Tue, Apr 14, 2015 at 11:25:15AM -0600, Jason Gunthorpe wrote:
> On Tue, Apr 14, 2015 at 10:18:07AM -0400, ira.weiny wrote:
>
> > After more thought and reading other opinions, I must agree we should not
> > have cap_foo_dev.
>
> I looked at it a bit, and I think Sean has also basically said,
On Tue, Apr 14, 2015 at 10:18:07AM -0400, ira.weiny wrote:
> After more thought and reading other opinions, I must agree we should not
> have cap_foo_dev.
I looked at it a bit, and I think Sean has also basically said, CM
does not support certain mixed port combinations. iWarp and IB simply
> But that moves us in the wrong direction. If we later support port 2
> without
> port 1 that code will be broken.
I agree that the code will be broken, but supporting that model requires a lot
more work in how the ib_cm listens across devices.
--
To unsubscribe from this list: send the line
On 04/14/2015 05:40 PM, ira.weiny wrote:
> On Tue, Apr 14, 2015 at 04:32:57PM +0200, Michael Wang wrote:
>>
>>
>> On 04/14/2015 04:18 PM, ira.weiny wrote:
>> [snip]
>>>
>>> /**
>>> - * cap_ib_cm_dev - Check if any port of device has the capability
>>> Infiniband
>>> - * Communication
On Tue, Apr 14, 2015 at 04:32:57PM +0200, Michael Wang wrote:
>
>
> On 04/14/2015 04:18 PM, ira.weiny wrote:
> [snip]
> >
> > /**
> > - * cap_ib_cm_dev - Check if any port of device has the capability
> > Infiniband
> > - * Communication Manager.
> > + * cap_ib_cm_any_port - Check if any
On 04/14/2015 04:18 PM, ira.weiny wrote:
[snip]
>
> /**
> - * cap_ib_cm_dev - Check if any port of device has the capability Infiniband
> - * Communication Manager.
> + * cap_ib_cm_any_port - Check if any port of the device has Infiniband
> + * Communication Manager (CM) support.
> *
> *
On Mon, Apr 13, 2015 at 02:01:38PM -0600, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2015 at 03:46:03PM -0400, ira.weiny wrote:
>
> > > This doesn't quite look right, it should be 'goto error1'
> >
> > Looks like you replied to the wrong patch. ?? I don't see error1 in
> > ipoib_add_one.
>
> >
On 04/13/2015 09:27 PM, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
>> dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
>> if (!dev_list)
>> @@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
>> }
>>
>>
On 04/13/2015 09:27 PM, Jason Gunthorpe wrote:
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
if (!dev_list)
@@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
}
for (p = s; p =
But that moves us in the wrong direction. If we later support port 2
without
port 1 that code will be broken.
I agree that the code will be broken, but supporting that model requires a lot
more work in how the ib_cm listens across devices.
--
To unsubscribe from this list: send the line
On Tue, Apr 14, 2015 at 10:18:07AM -0400, ira.weiny wrote:
After more thought and reading other opinions, I must agree we should not
have cap_foo_dev.
I looked at it a bit, and I think Sean has also basically said, CM
does not support certain mixed port combinations. iWarp and IB simply
cannot
On Tue, Apr 14, 2015 at 04:32:57PM +0200, Michael Wang wrote:
On 04/14/2015 04:18 PM, ira.weiny wrote:
[snip]
/**
- * cap_ib_cm_dev - Check if any port of device has the capability
Infiniband
- * Communication Manager.
+ * cap_ib_cm_any_port - Check if any port of the device
On Tue, Apr 14, 2015 at 01:43:11PM -0400, ira.weiny wrote:
A failure to init port 2 ends up ends up killing port 1 and releasing the
device associated resources.
Yes, that is the only reasonable thing that could happen.
init failure should only be possible under exceptional cases (OOM).
The
On Tue, Apr 14, 2015 at 11:25:15AM -0600, Jason Gunthorpe wrote:
On Tue, Apr 14, 2015 at 10:18:07AM -0400, ira.weiny wrote:
After more thought and reading other opinions, I must agree we should not
have cap_foo_dev.
I looked at it a bit, and I think Sean has also basically said, CM
does
Yes, that is the only reasonable thing that could happen.
init failure should only be possible under exceptional cases (OOM).
The only system response is to call ib_umad_add_one again - so of
course the first call had to completely clean up everything it did.
A reasonable follow up change
On Tue, Apr 14, 2015 at 06:02:47PM +, Hefty, Sean wrote:
Yes, that is the only reasonable thing that could happen.
init failure should only be possible under exceptional cases (OOM).
The only system response is to call ib_umad_add_one again - so of
course the first call had to
On Mon, Apr 13, 2015 at 02:01:38PM -0600, Jason Gunthorpe wrote:
On Mon, Apr 13, 2015 at 03:46:03PM -0400, ira.weiny wrote:
This doesn't quite look right, it should be 'goto error1'
Looks like you replied to the wrong patch. ?? I don't see error1 in
ipoib_add_one.
For the ib_cm
On 04/14/2015 05:40 PM, ira.weiny wrote:
On Tue, Apr 14, 2015 at 04:32:57PM +0200, Michael Wang wrote:
On 04/14/2015 04:18 PM, ira.weiny wrote:
[snip]
/**
- * cap_ib_cm_dev - Check if any port of device has the capability
Infiniband
- * Communication Manager.
+ *
On 04/14/2015 04:18 PM, ira.weiny wrote:
[snip]
/**
- * cap_ib_cm_dev - Check if any port of device has the capability Infiniband
- * Communication Manager.
+ * cap_ib_cm_any_port - Check if any port of the device has Infiniband
+ * Communication Manager (CM) support.
*
* @device:
On Mon, Apr 13, 2015 at 03:46:03PM -0400, ira.weiny wrote:
> > This doesn't quite look right, it should be 'goto error1'
>
> Looks like you replied to the wrong patch. ?? I don't see error1 in
> ipoib_add_one.
> For the ib_cm module...
Right, sorry.
> Yes I think it should go to "error1".
On Mon, Apr 13, 2015 at 01:27:01PM -0600, Jason Gunthorpe wrote:
> On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
> > dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
> > if (!dev_list)
> > @@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
> >
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
> dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
> if (!dev_list)
> @@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
> }
>
> for (p = s; p <= e; ++p) {
> - if
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
>
> Use raw management helpers to reform IB-ulp ipoib.
>
> Cc: Steve Wise
> Cc: Tom Talpey
> Cc: Jason Gunthorpe
> Cc: Doug Ledford
> Cc: Ira Weiny
> Cc: Sean Hefty
> Signed-off-by: Michael Wang
> ---
>
Use raw management helpers to reform IB-ulp ipoib.
Cc: Steve Wise
Cc: Tom Talpey
Cc: Jason Gunthorpe
Cc: Doug Ledford
Cc: Ira Weiny
Cc: Sean Hefty
Signed-off-by: Michael Wang
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 15 ---
1 file changed, 8 insertions(+), 7
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
Use raw management helpers to reform IB-ulp ipoib.
Cc: Steve Wise sw...@opengridcomputing.com
Cc: Tom Talpey t...@talpey.com
Cc: Jason Gunthorpe jguntho...@obsidianresearch.com
Cc: Doug Ledford dledf...@redhat.com
Cc: Ira
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
if (!dev_list)
@@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
}
for (p = s; p = e; ++p) {
- if
On Mon, Apr 13, 2015 at 01:27:01PM -0600, Jason Gunthorpe wrote:
On Mon, Apr 13, 2015 at 02:25:16PM +0200, Michael Wang wrote:
dev_list = kmalloc(sizeof *dev_list, GFP_KERNEL);
if (!dev_list)
@@ -1673,13 +1671,19 @@ static void ipoib_add_one(struct ib_device *device)
}
On Mon, Apr 13, 2015 at 03:46:03PM -0400, ira.weiny wrote:
This doesn't quite look right, it should be 'goto error1'
Looks like you replied to the wrong patch. ?? I don't see error1 in
ipoib_add_one.
For the ib_cm module...
Right, sorry.
Yes I think it should go to error1.
Use raw management helpers to reform IB-ulp ipoib.
Cc: Steve Wise sw...@opengridcomputing.com
Cc: Tom Talpey t...@talpey.com
Cc: Jason Gunthorpe jguntho...@obsidianresearch.com
Cc: Doug Ledford dledf...@redhat.com
Cc: Ira Weiny ira.we...@intel.com
Cc: Sean Hefty sean.he...@intel.com
48 matches
Mail list logo
