linux-kernel - RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 4 Apr 2018 11:54:22 +0000
From:   "Kalderon, Michal" <Michal.Kalderon@...ium.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
CC:     Sinan Kaya <okaya@...eaurora.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "timur@...eaurora.org" <timur@...eaurora.org>,
        "sulrich@...eaurora.org" <sulrich@...eaurora.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "Elior, Ariel" <Ariel.Elior@...ium.com>,
        Doug Ledford <dledford@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on
 weakly-ordered archs #2

> From: Jason Gunthorpe [mailto:jgg@...pe.ca]
> Sent: Tuesday, April 03, 2018 11:04 PM
> 
> On Tue, Apr 03, 2018 at 07:42:28AM +0000, Kalderon, Michal wrote:
> > > From: Sinan Kaya [mailto:okaya@...eaurora.org]
> > > Sent: Tuesday, April 03, 2018 5:30 AM
> > > To: linux-rdma@...r.kernel.org; timur@...eaurora.org;
> > > sulrich@...eaurora.org
> > > Cc: linux-arm-msm@...r.kernel.org;
> > > linux-arm-kernel@...ts.infradead.org;
> > > Kalderon, Michal <Michal.Kalderon@...ium.com>; Elior, Ariel
> > > <Ariel.Elior@...ium.com>; Doug Ledford <dledford@...hat.com>; Jason
> > > Gunthorpe <jgg@...pe.ca>; linux-kernel@...r.kernel.org
> > > Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers
> > > on weakly-ordered archs #2
> > >
> > > On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > > > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq,
> > > > u32
> > > cons, u8 flags)
> > > >  	wmb();
> > > >  	cq->db.data.agg_flags = flags;
> > > >  	cq->db.data.value = cpu_to_le32(cons);
> > > > -	writeq(cq->db.raw, cq->db_addr);
> > > > +	writeq_relaxed(cq->db.raw, cq->db_addr);
> > >
> > > Given the direction to get rid of wmb() in front of writeX()
> > > functions, I have been reviewing this code. Under normal
> > > circumstances, I can get rid of all
> > > wmb() as follows.
> > >
> > > However, I started having my doubts now. Are these wmb() used as a
> > > SMP barrier too?
> > > I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.
> >
> > Your doubts are in place. You initial patch series modified writel to
> > writel_relaxed Simply removing the wmb is dangerous. The wmb before
> > writel are used to make sure the HW observes the changes in memory
> > before we trigger the doorbell. Smp barriers here wouldn't suffice, as
> > on a single processor. we still need to make sure memory is updated and
> not remained in cache when HW accesses it.
> > Reviewing the qedr barriers, I can find places where this may have not
> > been necessary, But definitely you can't simply remove this wmb barriers.
> 
> As Sinan said, the consensus is that wmb();writel(); is redundant if the only
> purpose of the wmb is to order DMA and system memory.
> 
> So can you review these patches on that basis please? Is the WMB doing
> something else, eg SMP related? If yes, please send a patch adding
> appropriate comments.

Thanks Sinan and Jason for the references and explanations, I've reviewed the wmb
usages in qedr and am about to send a patch that replaces two of them with smp_wmb
and completely removes two of them that given your explanation, turned out to be redundant,
thanks.

> 
> Thanks,
> Jason