diff mbox

libibverbs init.c: remove stderr warnings if no userspace driver found

Message ID 1431109314-31662-1-git-send-email-jsquyres@cisco.com (mailing list archive)
State Superseded, archived
Delegated to: Doug Ledford
Headers show

Commit Message

Jeff Squyres May 8, 2015, 6:21 p.m. UTC
Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
---
 src/init.c | 14 --------------
 1 file changed, 14 deletions(-)

Comments

Yann Droneaud May 9, 2015, 2:04 p.m. UTC | #1
Hi,

Le vendredi 08 mai 2015 à 11:21 -0700, Jeff Squyres a écrit :
> Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

This is a little short for an explanation: what was the issue with the
error messages ?


> ---
>  src/init.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/src/init.c b/src/init.c
> index d0e4b1c..9c21768 100644
> --- a/src/init.c
> +++ b/src/init.c
> @@ -557,19 +557,5 @@ HIDDEN int ibverbs_init(struct ibv_device ***list)
>  	}
>  
>  out:
> -	for (sysfs_dev = sysfs_dev_list,
> -		     next_dev = sysfs_dev ? sysfs_dev->next : NULL;
> -	     sysfs_dev;
> -	     sysfs_dev = next_dev, next_dev = sysfs_dev ? sysfs_dev->next : NULL) {
> -		if (!sysfs_dev->have_driver) {
> -			fprintf(stderr, PFX "Warning: no userspace device-specific "
> -				"driver found for %s\n", sysfs_dev->sysfs_path);
> -			if (statically_linked)
> -				fprintf(stderr, "	When linking libibverbs statically, "
> -					"driver must be statically linked too.\n");
> -		}
> -		free(sysfs_dev);

I believe this free() was necessary to not leak some memory.

> -	}
> -
>  	return num_devices;
>  }


Regards.
Jeff Squyres May 11, 2015, 8:32 p.m. UTC | #2
T24gTWF5IDksIDIwMTUsIGF0IDg6MDQgQU0sIFlhbm4gRHJvbmVhdWQgPHlkcm9uZWF1ZEBvcHRl
eWEuY29tPiB3cm90ZToNCj4gDQo+IExlIHZlbmRyZWRpIDA4IG1haSAyMDE1IMOgIDExOjIxIC0w
NzAwLCBKZWZmIFNxdXlyZXMgYSDDqWNyaXQgOg0KPj4gU2lnbmVkLW9mZi1ieTogSmVmZiBTcXV5
cmVzIDxqc3F1eXJlc0BjaXNjby5jb20+DQo+IA0KPiBUaGlzIGlzIGEgbGl0dGxlIHNob3J0IGZv
ciBhbiBleHBsYW5hdGlvbjogd2hhdCB3YXMgdGhlIGlzc3VlIHdpdGggdGhlDQo+IGVycm9yIG1l
c3NhZ2VzID8NCg0KQ2lzY28gaGFzIHN0b3BwZWQgc2hpcHBpbmcgaXRzIGxpYmlidmVyYnMgdXNu
aWMgZHJpdmVyLCBhbHRob3VnaCB3ZSBhcmUgc3RpbGwgdXNpbmcgdGhlIGtlcm5lbCBkcml2ZXIg
aW4gdGhlIC9zeXMvY2xhc3MvaW5maW5pYmFuZCBzcGFjZSAoc2luY2UgaXQncyB0aGUgb25seSB3
YXkgdG8gYmUgdXBzdHJlYW0pLiAgU3BlY2lmaWNhbGx5OiBpbnN0ZWFkIG9mIHVzaW5nIGxpYmli
dmVyYnMgZm9yIHVzZXJzcGFjZSBhY2Nlc3MsIHdlIGFyZSBub3cgdXNpbmcgbGliZmFicmljLg0K
DQpUaGF0IGlzOiBpdCdzIG5vdCBhIHdhcm5pbmcgb3IgYW4gZXJyb3IgaWYgbGliaWJ2ZXJicyBj
YW5ub3QgZmluZCBhIHVzZXJzcGFjZSBkcml2ZXIgZm9yIGtlcm5lbCBkZXZpY2VzLiAgSW5kZWVk
LCByZXR1cm5pbmcgYSBudW1fZGV2aWNlcyBvZiAwIGlzIHN1ZmZpY2llbnQgLS0gdGhlIG1pZGRs
ZXdhcmUgc2hvdWxkbid0IGJlIHVuY29uZGl0aW9uYWxseSBwcmludGluZyBvdXQgc3RkZXJyIG1l
c3NhZ2U7IGxldCB0aGUgdXBwZXIgbGF5ZXIgYXBwbGljYXRpb24gZG8gdGhhdCAoaWYgaXQgd2Fu
dHMgdG8pLg0KDQpGV0lXLCBTZWFuIGp1c3QgcmVtb3ZlZCBhIHNpbWlsYXIgc2V0IG9mIHN0ZGVy
ciB3YXJuaW5ncyBmcm9tIGxpYnJkbWFjbToNCg0KICAgaHR0cDovL2dpdC5vcGVuZmFicmljcy5v
cmcvP3A9fnNoZWZ0eS9saWJyZG1hY20uZ2l0O2E9Y29tbWl0ZGlmZjtoPTJiMmFhZDgwOWFmYzU2
ZmEzMTU3ZjVjZjk5MDM2ZjkyYjljOTBmMTYNCg0KPj4gLQkJZnJlZShzeXNmc19kZXYpOw0KPiAN
Cj4gSSBiZWxpZXZlIHRoaXMgZnJlZSgpIHdhcyBuZWNlc3NhcnkgdG8gbm90IGxlYWsgc29tZSBt
ZW1vcnkuDQoNCkFoIC0tIEkgbWlzLXJlYWQgdGhlIGxvb3AuICBJJ2xsIHJlLXN1Ym1pdCB3aXRo
IHRoZSBsb29wIHN0aWxsIHRoZXJlLCBidXQganVzdCByZW1vdmluZyB0aGUgZnByaW50ZiBibG9j
ay4NCg0KLS0gDQpKZWZmIFNxdXlyZXMNCmpzcXV5cmVzQGNpc2NvLmNvbQ0KRm9yIGNvcnBvcmF0
ZSBsZWdhbCBpbmZvcm1hdGlvbiBnbyB0bzogaHR0cDovL3d3dy5jaXNjby5jb20vd2ViL2Fib3V0
L2RvaW5nX2J1c2luZXNzL2xlZ2FsL2NyaS8NCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yann Droneaud May 11, 2015, 9:32 p.m. UTC | #3
Hi,

Le lundi 11 mai 2015 à 20:32 +0000, Jeff Squyres (jsquyres) a écrit :
> On May 9, 2015, at 8:04 AM, Yann Droneaud <ydroneaud@opteya.com> wrote:
> > 
> > Le vendredi 08 mai 2015 à 11:21 -0700, Jeff Squyres a écrit :
> >> Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
> > 
> > This is a little short for an explanation: what was the issue with the
> > error messages ?
> 
> Cisco has stopped shipping its libibverbs usnic driver, although we
> are still using the kernel driver in the /sys/class/infiniband space
> (since it's the only way to be upstream).  Specifically: instead of
> using libibverbs for userspace access, we are now using libfabric.
> 

OK.

(I have to find some information about this libfabric which is already
released as 1.0.0 version).

> That is: it's not a warning or an error if libibverbs cannot find a
> userspace driver for kernel devices.  Indeed, returning a num_devices
> of 0 is sufficient -- the middleware shouldn't be unconditionally
> printing out stderr message; let the upper layer application do that
> (if it wants to).
> 

This paragraph should definitively be part of your commit message.

> FWIW, Sean just removed a similar set of stderr warnings from librdmacm:
> 
>    http://git.openfabrics.org/?p=~shefty/librdmacm.git;a=commitdiff;h=2b2aad809afc56fa3157f5cf99036f92b9c90f16
> 

I don't think this a good thing to remove all the error messages
unconditionally. It could have been made a build option + an environment
variable option to help mere mortals to debug their setups.

Regards.
Jeff Squyres May 12, 2015, 2:10 p.m. UTC | #4
On May 11, 2015, at 2:32 PM, Yann Droneaud <ydroneaud@opteya.com> wrote:
> 
> (I have to find some information about this libfabric which is already
> released as 1.0.0 version).

See http://ofiwg.github.io/libfabric/ (and the corresponding code https://github.com/ofiwg/libfabric).

>> That is: it's not a warning or an error if libibverbs cannot find a
>> userspace driver for kernel devices.  Indeed, returning a num_devices
>> of 0 is sufficient -- the middleware shouldn't be unconditionally
>> printing out stderr message; let the upper layer application do that
>> (if it wants to).
> 
> This paragraph should definitively be part of your commit message.

Ok.

>> FWIW, Sean just removed a similar set of stderr warnings from librdmacm:
>> 
>>   http://git.openfabrics.org/?p=~shefty/librdmacm.git;a=commitdiff;h=2b2aad809afc56fa3157f5cf99036f92b9c90f16
> 
> I don't think this a good thing to remove all the error messages
> unconditionally. It could have been made a build option + an environment
> variable option to help mere mortals to debug their setups.

Ok.  v3 coming shortly.
diff mbox

Patch

diff --git a/src/init.c b/src/init.c
index d0e4b1c..9c21768 100644
--- a/src/init.c
+++ b/src/init.c
@@ -557,19 +557,5 @@  HIDDEN int ibverbs_init(struct ibv_device ***list)
 	}
 
 out:
-	for (sysfs_dev = sysfs_dev_list,
-		     next_dev = sysfs_dev ? sysfs_dev->next : NULL;
-	     sysfs_dev;
-	     sysfs_dev = next_dev, next_dev = sysfs_dev ? sysfs_dev->next : NULL) {
-		if (!sysfs_dev->have_driver) {
-			fprintf(stderr, PFX "Warning: no userspace device-specific "
-				"driver found for %s\n", sysfs_dev->sysfs_path);
-			if (statically_linked)
-				fprintf(stderr, "	When linking libibverbs statically, "
-					"driver must be statically linked too.\n");
-		}
-		free(sysfs_dev);
-	}
-
 	return num_devices;
 }