On 03-02-16, 13:42, Rafael J. Wysocki wrote: > > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, > > + char *buf) > > +{ > > + struct dbs_data *dbs_data = to_dbs_data(kobj); > > + struct governor_attr *gattr = to_gov_attr(attr); > > + int ret = -EIO; > > + > > + down_read(&dbs_data->rwsem); > > + > > + if (gattr->show) > > + ret = gattr->show(dbs_data, buf); > > + > > + up_read(&dbs_data->rwsem); > > Do we need the lock here too? > > show() is only going to read the value, isn't it? And everything u32 > or smaller is read atomically anyway.
Okay, will drop it for now. > Apart from this it looks good to me. > > When you're ready, please resend the whole series without patch [5/5] > which is premature IMO. I have changed that patch a bit, and am dropping just the lock now and not governor_enable thing. That should be sane enough I believe. Anyway I will be posting 7 patches now, pick only first 4 if you aren't confident about the rest. -- viresh