From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matan Barak Subject: Re: [PATCH libibverbs V5 2/2] Use neighbour lookup for RoCE UD QPs Eth L2 resolution Date: Thu, 28 Aug 2014 19:27:37 +0300 Message-ID: <53FF5879.4040600@mellanox.com> References: <1408517381-17523-1-git-send-email-matanb@mellanox.com> <1408517381-17523-3-git-send-email-matanb@mellanox.com> <20140820170142.GC12605@obsidianresearch.com> <53F9EDCD.9060105@mellanox.com> <20140825183325.GC1298@obsidianresearch.com> <5C3BB7B1-07E6-431F-98E5-13543AAA897D@redhat.com> <53FC7B1D.6050501@mellanox.com> <20140826160853.GA31127@obsidianresearch.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140826160853.GA31127-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jason Gunthorpe , Or Gerlitz Cc: Doug Ledford , Roland Dreier , Yishai Hadas , "linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-rdma@vger.kernel.org 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 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html