On Mon, Mar 19, 2018 at 09:13:02AM -0600, Jason Gunthorpe wrote:
> On Mon, Mar 19, 2018 at 12:08:09PM +0200, Leon Romanovsky wrote:
> > > 794 static ssize_t show_hw_stats(struct kobject *kobj, struct attribute *attr,
> > > 795 char *buf)
> > > 796 {
> > > 797 struct ib_device *dev;
> > > 798 struct ib_port *port;
> > > 799 struct hw_stats_attribute *hsa;
> > > 800 struct rdma_hw_stats *stats;
> > > 801 int ret;
> > > 802
> > > 803 hsa = container_of(attr, struct hw_stats_attribute, attr);
> > > 804 if (!hsa->port_num) {
> > > 805 dev = container_of((struct device *)kobj,
> > > 806 struct ib_device, dev);
> > > 807 stats = dev->hw_stats;
> > > 808 } else {
> > > 809 port = container_of(kobj, struct ib_port, kobj);
> > > 810 dev = port->ibdev;
> > > 811 stats = port->hw_stats;
> >
> > Mark pointed to me that "stats" is actually pointer to shared memory.
>
> Yes it is.
>
> So either stats should be made thread local here, or we need the lock.
>
> You can't rely on ordinary writes to shared memory to do anything
> sane, only atomic writes have sensible properties.
>
> I can't think of any reason why adding this lock would be bad?
It is not bad, but I wanted to push Mark to think about thread safe
solution. I don't like the fact that all callers of get_hw_state will
need to put lock around it.
Thanks
>
> Jason
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
