Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 11 Jul 2013 08:26:44 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Garrett Cooper <yaneurabeya@gmail.com>
Cc:        svn-src-head <svn-src-head@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org>, shahark@mellanox.com, acornehl@gmail.com
Subject:   Re: svn commit: r253048 - in head/sys/ofed: drivers/net/mlx4 include/linux
Message-ID:  <201307110826.44287.jhb@freebsd.org>
In-Reply-To: <CAGHfRMAJs385b0ZbXq%2BnMEzqNJ8VK=6Y8Jfn%2BfTeSqsdpkr8LA@mail.gmail.com>
References:  <201307082125.r68LPDlY023493@svn.freebsd.org> <201307091120.22114.jhb@freebsd.org> <CAGHfRMAJs385b0ZbXq%2BnMEzqNJ8VK=6Y8Jfn%2BfTeSqsdpkr8LA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, July 10, 2013 7:58:30 pm Garrett Cooper wrote:
> On Tue, Jul 9, 2013 at 8:20 AM, John Baldwin <jhb@freebsd.org> wrote:
> 
> ...
> 
> > I hadn't seen it.  I had wondered if the '\n' issue was a generic sysfs thing.
> > It sounds like it is and I'd be happy to revert the mlx4 change and alter the
> > sysfs bits to manage the newline directly if that is more appropriate.
> 
> I'll doublecheck this, but basically I'm really adverse to diverging
> from Linux in this area -- hence that's why I did what I did in the PR
> I mentioned.

I checked the other sysfs attributes under sys/ofed and all the ones that deal
with strings expect the newline.  There might be some that are "blobs" that do
not expect newlines, but it wasn't clear.  I do agree that it would be best to
avoid diffs for this case if possible in all the sysfs handlers.

(It would be even nicer if ofed used a more abstract notion for device attributes
in its drivers that could map to sysctl on FreeBSD and sysfs device attributes on
Linux, but that's a much larger change, and one OFED would have to want to do.)

> > I'd also like this to use sysctl_handle_string() if at all possible.  Are you in
> > a position to test patches still?
> 
> Unfortunately I'm not right now :(. Anthony may or may not be able to
> test this out (I used his machine when we were hacking around on IB
> stuff).
> 
> > If so, maybe give this a whirl.  It's similar to yours except it uses
> > sysctl_handle_string() and strlcat() rather than continuing to do things
> > by hand.  It also outputs an empty string to userland if the attribute
> > doesn't have a show method (your version would never pass out an old
> > string in that case unlike the original code).
> 
> Can you please pass along a patch to Anthony and me so we can try and
> apply it to test it out (Gmail's going to taint the inline patch, as
> is the EMC Exchange server with the patch attachment)? If Anthony
> still has the machine setup, then I'd be more than happy to test out
> the patch :).

Ok, I've put the patch at a URL.  Note that it is relative to what was just
committed to HEAD, so to test on 9 you'd have to apply both patches.

http://www.freebsd.org/~jhb/patches/ofed_sysfs.patch

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307110826.44287.jhb>