| Message ID | 1415906662-4576-2-git-send-email-bobby.prani@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
On Thu, Nov 13, 2014 at 02:24:07PM -0500, Pranith Kumar wrote: > Recently lockless_dereference() was added which can be used in place of > hard-coding smp_read_barrier_depends(). The following PATCH makes the change. > > Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> > --- > drivers/crypto/caam/jr.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > index bae20d8..9b3ef1bc 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg) > for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) { > sw_idx = (tail + i) & (JOBR_DEPTH - 1); > > - smp_read_barrier_depends(); > - Did you mean to add a lockless_dereference() somewhere? I would guess that one is required on the load into sw_idx before the "for" loop, but I cannot claim to be familiar with this code. Thanx, Paul > if (jrp->outring[hw_idx].desc == > jrp->entinfo[sw_idx].desc_addr_dma) > break; /* found */ > @@ -218,7 +216,6 @@ static void caam_jr_dequeue(unsigned long devarg) > if (sw_idx == tail) { > do { > tail = (tail + 1) & (JOBR_DEPTH - 1); > - smp_read_barrier_depends(); > } while (CIRC_CNT(head, tail, JOBR_DEPTH) >= 1 && > jrp->entinfo[tail].desc_addr_dma == 0); > > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 13 Nov 2014 16:51:12 -0500 Pranith Kumar <bobby.prani@gmail.com> wrote: > On 11/13/2014 03:10 PM, Paul E. McKenney wrote: > > On Thu, Nov 13, 2014 at 02:24:07PM -0500, Pranith Kumar wrote: > >> Recently lockless_dereference() was added which can be used in place of > >> hard-coding smp_read_barrier_depends(). The following PATCH makes the change. > >> > >> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> > >> --- > >> drivers/crypto/caam/jr.c | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > >> index bae20d8..9b3ef1bc 100644 > >> --- a/drivers/crypto/caam/jr.c > >> +++ b/drivers/crypto/caam/jr.c > >> @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg) > >> for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) { > >> sw_idx = (tail + i) & (JOBR_DEPTH - 1); > >> > >> - smp_read_barrier_depends(); > >> - > > > > Did you mean to add a lockless_dereference() somewhere? I would guess > > that one is required on the load into sw_idx before the "for" loop, > > but I cannot claim to be familiar with this code. > > No, I could not understand why the barrier was here in the first place. The change log does not reflect the subject line. > > sw_idx is a local variable and is used to index the entinfo array. It does not seem like the barrier is needed. If not a comment saying why could be added I guess. I likely misinterpreted the "read index before reading contents at that index" instructions in early circular buffer documentation, and left it in for the benefit of the doubt. Now it looks to me it's not necessary, given both sw_idx and tail are just being computed within a lock, and removing both barriers doesn't affect the compiler output, so: Reviewed-by: Kim Phillips <kim.phillips@freescale.com> Thanks, Kim -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" 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/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index bae20d8..9b3ef1bc 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg) for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) { sw_idx = (tail + i) & (JOBR_DEPTH - 1); - smp_read_barrier_depends(); - if (jrp->outring[hw_idx].desc == jrp->entinfo[sw_idx].desc_addr_dma) break; /* found */ @@ -218,7 +216,6 @@ static void caam_jr_dequeue(unsigned long devarg) if (sw_idx == tail) { do { tail = (tail + 1) & (JOBR_DEPTH - 1); - smp_read_barrier_depends(); } while (CIRC_CNT(head, tail, JOBR_DEPTH) >= 1 && jrp->entinfo[tail].desc_addr_dma == 0);
Recently lockless_dereference() was added which can be used in place of hard-coding smp_read_barrier_depends(). The following PATCH makes the change. Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> --- drivers/crypto/caam/jr.c | 3 --- 1 file changed, 3 deletions(-)