| Message ID | 1521380012-64924-2-git-send-email-markb@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Mar 18, 2018 at 01:33:32PM +0000, Mark Bloch wrote: > Currently access to hardware stats buffer isn't protected, this can > result in multiple writes and reads at the same time to the same > memory location. This can lead to providing an incorrect value to > the user. Add a mutex to protect against it. > > Signed-off-by: Mark Bloch <markb@mellanox.com> > --- > drivers/infiniband/core/sysfs.c | 10 ++++++++-- > include/rdma/ib_verbs.h | 1 + > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c > index 8ae1308e..b769d61 100644 > --- a/drivers/infiniband/core/sysfs.c > +++ b/drivers/infiniband/core/sysfs.c > @@ -810,10 +810,15 @@ static ssize_t show_hw_stats(struct kobject *kobj, struct attribute *attr, > dev = port->ibdev; > stats = port->hw_stats; > } > + mutex_lock(&stats->lock); Lock is not needed here, because every thread will get its own stats. > ret = update_hw_stats(dev, stats, hsa->port_num, hsa->index); > if (ret) > - return ret; > - return print_hw_stat(stats, hsa->index, buf); > + goto unlock; > + ret = print_hw_stat(stats, hsa->index, buf); > +unlock: > + mutex_unlock(&stats->lock); > + > + return ret; > } > > static ssize_t show_stats_lifespan(struct kobject *kobj, > @@ -951,6 +956,7 @@ static void setup_hw_stats(struct ib_device *device, struct ib_port *port, > sysfs_attr_init(hsag->attrs[i]); > } > > + mutex_init(&stats->lock); > /* treat an error here as non-fatal */ > hsag->attrs[i] = alloc_hsa_lifespan("lifespan", port_num); > if (hsag->attrs[i]) > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 73b2387..33fbccb 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -484,6 +484,7 @@ enum ib_port_speed { > * filled in by the drivers get_stats routine > */ > struct rdma_hw_stats { > + struct mutex lock; > unsigned long timestamp; > unsigned long lifespan; > const char * const *names; > -- > 1.8.4.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index 8ae1308e..b769d61 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -810,10 +810,15 @@ static ssize_t show_hw_stats(struct kobject *kobj, struct attribute *attr, dev = port->ibdev; stats = port->hw_stats; } + mutex_lock(&stats->lock); ret = update_hw_stats(dev, stats, hsa->port_num, hsa->index); if (ret) - return ret; - return print_hw_stat(stats, hsa->index, buf); + goto unlock; + ret = print_hw_stat(stats, hsa->index, buf); +unlock: + mutex_unlock(&stats->lock); + + return ret; } static ssize_t show_stats_lifespan(struct kobject *kobj, @@ -951,6 +956,7 @@ static void setup_hw_stats(struct ib_device *device, struct ib_port *port, sysfs_attr_init(hsag->attrs[i]); } + mutex_init(&stats->lock); /* treat an error here as non-fatal */ hsag->attrs[i] = alloc_hsa_lifespan("lifespan", port_num); if (hsag->attrs[i]) diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 73b2387..33fbccb 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -484,6 +484,7 @@ enum ib_port_speed { * filled in by the drivers get_stats routine */ struct rdma_hw_stats { + struct mutex lock; unsigned long timestamp; unsigned long lifespan; const char * const *names;
Currently access to hardware stats buffer isn't protected, this can result in multiple writes and reads at the same time to the same memory location. This can lead to providing an incorrect value to the user. Add a mutex to protect against it. Signed-off-by: Mark Bloch <markb@mellanox.com> --- drivers/infiniband/core/sysfs.c | 10 ++++++++-- include/rdma/ib_verbs.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-)