On 26/8/2014 7:08 PM, Jason Gunthorpe wrote:
On Tue, Aug 26, 2014 at 03:18:37PM +0300, Or Gerlitz wrote:
On 25/08/2014 22:33, Doug Ledford wrote:
On Aug 25, 2014, at 1:33 PM, Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
+ timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC);
+ if (-1 == timer_fd) {
+ print_err("Couldn't create timer\n");
+ return timer_fd;
+ }
The use of timerfd will impact the minimum OS version, have you
checked this is OK? Does RHEL5 still work?
It was added in linux v2.6.25. I think that an API that's more than
6.5 years old is valid.
RHEL5 is using 2.6.18 as their base kernel. You should at least
consult with the OFED people to determine if this is a problem for
them.
Please don't. This code should not be changed for something as ancient as rhel5.
Indeed. Telling people to avoid using constructs/mechanisms ~6-7
years after they were introduced isn't something we want nor need to
do.
I looked myself and it looks like OFED has dropped support for these
old distros so there isn't any problem.
However, I still think this use of timerfd is fairly gratuitous, and
looking closer, causes little bugs:
+ if (timerfd_settime(timer_fd, 0, &timer_time, NULL)) {
+ print_err("Couldn't set timer\n");
+ return -1;
^^^^^^^^^^^^^^
leaks timer_fd
Alos, I noticed:
+ /* wait for an incoming message on the netlink socket */
+ ret = select(nfds, &fdset, NULL, NULL, NULL);
+
+ if (ret) {
Fails to detect error return from select.
Jason
Thanks, I'll fix those issues too.
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
