On Monday, July 08, 2013 6:13:22 pm Garrett Cooper wrote:
> On Jul 8, 2013, at 2:25 PM, John Baldwin wrote:
>
> > Author: jhb
> > Date: Mon Jul 8 21:25:12 2013
> > New Revision: 253048
> > URL: http://svnweb.freebsd.org/changeset/base/253048
> >
> > Log:
> > Allow mlx4 devices to switch from Ethernet to Infiniband (and vice
versa):
> > - Fix sysctl wrapper for sysfs attributes to properly handle new string
> > values similar to sysctl_handle_string() (only copyin the user's
> > supplied length and nul-terminate the string).
> > - Don't check for a trailing newline when evaluating the desired
operating
> > mode of a mlx4 device.
> >
> > PR: kern/179999
> > Submitted by: Shahar Klein <shah...@mellanox.com>
> > MFC after: 1 week
>
> Was there an issue with the patch I submitted via
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/174213 (basically solving the
same problem, but only with the sysfs <-> sysctl(9) handler)? I was of the
impression that sysfs on Linux always added on trailing newlines (but I could
be wrong because I haven't used Linux at a dev level for ages). Thanks!
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'd
also like this to use sysctl_handle_string() if at all possible. Are you in
a position to test patches still?
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).
Index: drivers/net/mlx4/main.c
===================================================================
--- drivers/net/mlx4/main.c (revision 253096)
+++ drivers/net/mlx4/main.c (working copy)
@@ -479,11 +479,11 @@
int i;
int err = 0;
- if (!strcmp(buf, "ib"))
+ if (!strcmp(buf, "ib\n"))
info->tmp_type = MLX4_PORT_TYPE_IB;
- else if (!strcmp(buf, "eth"))
+ else if (!strcmp(buf, "eth\n"))
info->tmp_type = MLX4_PORT_TYPE_ETH;
- else if (!strcmp(buf, "auto"))
+ else if (!strcmp(buf, "auto\n"))
info->tmp_type = MLX4_PORT_TYPE_AUTO;
else {
mlx4_err(mdev, "%s is not supported port type\n", buf);
Index: include/linux/sysfs.h
===================================================================
--- include/linux/sysfs.h (revision 253096)
+++ include/linux/sysfs.h (working copy)
@@ -81,37 +81,35 @@
kobj = arg1;
attr = (struct attribute *)arg2;
- buf = (void *)get_zeroed_page(GFP_KERNEL);
- len = 1; /* Copy out a NULL byte at least. */
if (kobj->ktype == NULL || kobj->ktype->sysfs_ops == NULL)
return (ENODEV);
- ops = kobj->ktype->sysfs_ops;
+ buf = (void *)get_zeroed_page(GFP_KERNEL);
if (buf == NULL)
return (ENOMEM);
+ ops = kobj->ktype->sysfs_ops;
if (ops->show) {
len = ops->show(kobj, attr, buf);
/*
- * It's valid not to have a 'show' so we just return 1 byte
- * of NULL.
+ * It's valid to not have a 'show' so just return an
+ * empty string.
*/
if (len < 0) {
error = -len;
- len = 1;
if (error != EIO)
goto out;
}
+
+ /* Trim trailing newline. */
+ len--;
+ buf[len] = '\0';
}
- error = SYSCTL_OUT(req, buf, len);
- if (error || !req->newptr || ops->store == NULL)
+
+ /* Leave one trailing byte to append a newline. */
+ error = sysctl_handle_string(oidp, buf, PAGE_SIZE - 1, req);
+ if (error != 0 || req->newptr == NULL || ops->store == NULL)
goto out;
- len = req->newlen - req->newidx;
- if (len >= PAGE_SIZE)
- error = EINVAL;
- else
- error = SYSCTL_IN(req, buf, len);
- if (error)
- goto out;
- ((char *)buf)[len] = '\0';
+ len = strlcat(buf, "\n", PAGE_SIZE);
+ KASSERT(len < PAGE_SIZE, ("new attribute truncated"));
len = ops->store(kobj, attr, buf, len);
if (len < 0)
error = -len;
--
John Baldwin
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
