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
--
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
