Messages in this thread | | | Date | Wed, 26 Jun 2019 09:46:33 +0800 | From | Greg KH <> | Subject | Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs |
| |
On Tue, Jun 25, 2019 at 06:33:08PM -0700, Tri Vo wrote: > On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > +static ssize_t wakeup_source_count_show(struct wakeup_source *ws, > > > + struct wakeup_source_attribute *attr, > > > + char *buf) > > > +{ > > > + unsigned long flags; > > > + unsigned long var; > > > + > > > + spin_lock_irqsave(&ws->lock, flags); > > > + if (strcmp(attr->attr.name, "active_count") == 0) > > > + var = ws->active_count; > > > + else if (strcmp(attr->attr.name, "event_count") == 0) > > > + var = ws->event_count; > > > + else if (strcmp(attr->attr.name, "wakeup_count") == 0) > > > + var = ws->wakeup_count; > > > + else > > > + var = ws->expire_count; > > > + spin_unlock_irqrestore(&ws->lock, flags); > > > + > > > + return sprintf(buf, "%lu\n", var); > > > +} > > > > Why is this lock always needed to be grabbed? You are just reading a > > value, who cares if it changes inbetween reading it and returning the > > buffer string as it can change at that point in time anyway? > > Right, we don't care if the value changes in between us reading and > printing it. However, IIUC not grabbing this lock results in a data > race, which is undefined behavior.
A data race where? Writing to the value? How can that happen? All you are doing is incrementing this variable elsewhere, what is the worst that can happen?
thanks,
greg k-h
| |