Hi Herbert,
On Sun, 25 Nov 2007 15:29:43 +0100, Herbert Valerio Riedel wrote:
> On Sun, 2007-11-25 at 14:08 +0100, Jean Delvare wrote:
> > +static ssize_t show_fan_input(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int nr = to_sensor_dev_attr(attr)->index;
> > + struct gl518_data *data = gl518_update_device(dev);
> > + return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_in[nr],
> > + DIV_FROM_REG(data->fan_div[nr])));
> > +}
> > +
> > +static ssize_t show_fan_min(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int nr = to_sensor_dev_attr(attr)->index;
> > + struct gl518_data *data = gl518_update_device(dev);
> > + return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[nr],
> > + DIV_FROM_REG(data->fan_div[nr])));
> > +}
>
> one thing just sprang to my mind... is it safe to assume that the
> expressions data->fan_min[nr] and data->fan_div[nr] are evaluated
> atomically? or to put it differently, why don't we need a rw-lock here
> to protect against interleaved updates?
This is a very good question. We always assumed that reading from the
register cache did not require to hold the lock. However for this to be
true on all architectures, I guess that we should be declaring all the
register values with type atomic_t. So you're probably right, our code
is probably not safe on some architectures.
--
Jean Delvare
