> +static int mlx4_change_port_types(struct mlx4_dev *dev,
> + enum mlx4_port_type *port_types)
> +{
> + int i;
> + int err = 0;
> + int change = 0;
> + int port;
> +
> + for (i = 0; i < MLX4_MAX_PORTS; i++) {
> + if (port_types[i] != dev->caps.port_type[i + 1]) {
> + change = 1;
> + dev->caps.port_type[i + 1] = port_types[i];
> + }
> + }
> + if (change) {
> + mlx4_unregister_device(dev);
> + for (port = 1; port <= dev->caps.num_ports; port++) {
> + mlx4_CLOSE_PORT(dev, port);
> + err = mlx4_SET_PORT(dev, port);
> + if (err) {
> + mlx4_err(dev, "Failed to set port %d, "
> + "aborting\n", port);
> + return err;
> + }
> + }
> + err = mlx4_register_device(dev);
> + }
> + return err;
> +}
If I read the code correctly, there is no locking around this, so
multiple processes could race and cause all sorts of problems. And also
you do the assignment
> + dev->caps.port_type[i + 1] = port_types[i];
before unregistering the device -- so there is a window where
caps.port_type has the wrong data -- not sure if this is a real issue.
> +static ssize_t show_port_type(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct mlx4_dev *mdev = pci_get_drvdata(pdev);
> + int i;
> +
> + sprintf(buf, "Current port types:\n");
> + for (i = 1; i <= MLX4_MAX_PORTS; i++) {
> + sprintf(buf, "%sPort%d: %s\n", buf, i,
> + (mdev->caps.port_type[i] == MLX4_PORT_TYPE_IB)?
> + "ib": "eth");
> + }
> + return strlen(buf);
> +}
This violates the one-value-per-file rule for sysfs if I am reading the
code correctly.
- R.
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
