Update on Chapter Deferred Processing

PATCH In review Junchang Wang 2 participants 6 messages Jun 10, 2017

lore

Share & tools

lore

Junchang Wang

author

2017-06-10 14:28 UTC #0 lore raw

cover Update on Chapter Deferred Processing

Hi Paul,

I'm reading Chapter Deferred Processing, and following are some patches. Please take a look.

--Jason

Junchang Wang (4):
Figure Storage Hazard-Pointer Storage and Erasure: Switch from
ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
route_hazptr: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
route_seqlock: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

 CodeSamples/defer/route_hazptr.c  | 10 +++++-----
 CodeSamples/defer/route_seqlock.c |  6 +++---
 CodeSamples/defer/seqlock.h       |  4 ++--
 defer/hazptr.tex                  | 16 ++++++++--------
 defer/seqlock.tex                 | 12 ++++++------
 5 files changed, 26 insertions(+), 26 deletions(-)

Signature · 1 line

2.7.4

Junchang Wang

author

2017-06-10 14:28 UTC #1 lore raw

PATCH 1/4 Figure Storage Hazard-Pointer Storage and Erasure: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

Signed-off-by Junchang Wang <[email protected]>

---
 defer/hazptr.tex | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

defer/hazptr.tex +5 -5

index cdde323..10ae7a1 100644
@@ -26,12 +26,12 @@ may safely be freed.
  2 {
  3   void *tmp;
  4 
- 5   tmp = ACCESS_ONCE(*p);
- 6   ACCESS_ONCE(*hp) = tmp;
+ 5   tmp = READ_ONCE(*p);
+ 6   WRITE_ONCE(*hp, tmp);
  7   smp_mb();
- 8   if (tmp != ACCESS_ONCE(*p) ||
+ 8   if (tmp != READ_ONCE(*p) ||
  9       tmp == HAZPTR_POISON) {
-10     ACCESS_ONCE(*hp) = NULL;
+10     WRITE_ONCE(*hp, NULL);
 11     return 0;
 12   }
 13   return 1;

@@ -40,7 +40,7 @@ may safely be freed.
 16 void hp_erase(void **hp)
 17 {
 18   smp_mb();
-19   ACCESS_ONCE(*hp) = NULL;
+19   WRITE_ONCE(*hp, NULL);
 20   hp_free(hp);
 21 }
 \end{verbbox}

Signature · 1 line

2.7.4

Junchang Wang

author

2017-06-10 14:28 UTC #2 lore raw

PATCH 2/4 route_hazptr: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

In reply to Junchang Wang

Signed-off-by Junchang Wang <[email protected]>

---
 CodeSamples/defer/route_hazptr.c | 10 +++++-----
 defer/hazptr.tex                 |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

CodeSamples/defer/route_hazptr.c +5 -5

index 590ba4d..49e6e4a 100644
@@ -49,7 +49,7 @@ unsigned long route_lookup(unsigned long addr)
 retry:
 	repp = &route_list.re_next;
 	do {
-		rep = ACCESS_ONCE(*repp);
+		rep = READ_ONCE(*repp);
 		if (rep == NULL)
 			return ULONG_MAX;
 		if (rep == (struct route_entry *)HAZPTR_POISON)

@@ -61,13 +61,13 @@ retry:
 		smp_mb(); /* Force pointer loads in order. */
 
 		/* Recheck the hazard pointer against the original. */
-		if (ACCESS_ONCE(*repp) != rep)
+		if (READ_ONCE(*repp) != rep)
 			goto retry;
 
 		/* Advance to next. */
 		repp = &rep->re_next;
 	} while (rep->addr != addr);
-	if (ACCESS_ONCE(rep->re_freed))
+	if (READ_ONCE(rep->re_freed))
 		abort();
 	return rep->iface;
 }

@@ -129,7 +129,7 @@ void route_clear(void)
 
 	spin_lock(&routelock);
 	rep = route_list.re_next;
-	ACCESS_ONCE(route_list.re_next) = NULL;
+	WRITE_ONCE(route_list.re_next, NULL);
 	while (rep != NULL) {
 		rep1 = rep->re_next;
 		rep->re_next = (struct route_entry *)HAZPTR_POISON;

@@ -157,7 +157,7 @@ void hazptr_free(void *p)
 {
 	struct route_entry *rep = p;
 
-	ACCESS_ONCE(rep->re_freed) = 1;
+	WRITE_ONCE(rep->re_freed, 1);
 	free(p);
 }

defer/hazptr.tex +3 -3

index 10ae7a1..b45f5b6 100644
@@ -201,7 +201,7 @@ and in other publications~\cite{ThomasEHart2007a,McKenney:2013:SDS:2483852.24838
 18 retry:
 19   repp = &route_list.re_next;
 20   do {
-21     rep = ACCESS_ONCE(*repp);
+21     rep = READ_ONCE(*repp);
 22     if (rep == NULL)
 23       return ULONG_MAX;
 24     if (rep == (struct route_entry *)HAZPTR_POISON)

@@ -209,11 +209,11 @@ and in other publications~\cite{ThomasEHart2007a,McKenney:2013:SDS:2483852.24838
 26     my_hazptr[offset].p = &rep->hh;
 27     offset = !offset;
 28     smp_mb();
-29     if (ACCESS_ONCE(*repp) != rep)
+29     if (READ_ONCE(*repp) != rep)
 30       goto retry;
 31     repp = &rep->re_next;
 32   } while (rep->addr != addr);
-33   if (ACCESS_ONCE(rep->re_freed))
+33   if (READ_ONCE(rep->re_freed))
 34     abort();
 35   return rep->iface;
 36 }

Signature · 1 line

2.7.4

Junchang Wang

author

2017-06-10 14:28 UTC #3 lore raw

PATCH 3/4 route_seqlock: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

In reply to Junchang Wang

Signed-off-by Junchang Wang <[email protected]>

---
 CodeSamples/defer/route_seqlock.c |  6 +++---
 CodeSamples/defer/seqlock.h       |  4 ++--
 defer/seqlock.tex                 | 12 ++++++------
 3 files changed, 11 insertions(+), 11 deletions(-)

CodeSamples/defer/route_seqlock.c +3 -3

index 633cf05..8224682 100644
@@ -47,7 +47,7 @@ retry:
 	s = read_seqbegin(&sl);
 	repp = &route_list.re_next;
 	do {
-		rep = ACCESS_ONCE(*repp);
+		rep = READ_ONCE(*repp);
 		if (rep == NULL) {
 			if (read_seqretry(&sl, s))
 				goto retry;

@@ -57,7 +57,7 @@ retry:
 		/* Advance to next. */
 		repp = &rep->re_next;
 	} while (rep->addr != addr);
-	if (ACCESS_ONCE(rep->re_freed))
+	if (READ_ONCE(rep->re_freed))
 		abort();
 	ret = rep->iface;
 	if (read_seqretry(&sl, s))

@@ -123,7 +123,7 @@ void route_clear(void)
 
 	write_seqlock(&sl);
 	rep = route_list.re_next;
-	ACCESS_ONCE(route_list.re_next) = NULL;
+	WRITE_ONCE(route_list.re_next, NULL);
 	while (rep != NULL) {
 		rep1 = rep->re_next;
 		free(rep);

CodeSamples/defer/seqlock.h +2 -2

index b210944..c994285 100644
@@ -38,7 +38,7 @@ static inline unsigned long read_seqbegin(seqlock_t *slp)
 {
 	unsigned long s;
 
-	s = ACCESS_ONCE(slp->seq);
+	s = READ_ONCE(slp->seq);
 	smp_mb();
 	return s & ~0x1UL;
 }

@@ -48,7 +48,7 @@ static inline int read_seqretry(seqlock_t *slp, unsigned long oldseq)
 	unsigned long s;
 
 	smp_mb();
-	s = ACCESS_ONCE(slp->seq);
+	s = READ_ONCE(slp->seq);
 	return s != oldseq;
 }

defer/seqlock.tex +6 -6

index ba8abfe..cb5e4e6 100644
@@ -116,7 +116,7 @@ It is also used in pathname traversal to detect concurrent rename operations.
 13  {
 14    unsigned long s;
 15
-16    s = ACCESS_ONCE(slp->seq);
+16    s = READ_ONCE(slp->seq);
 17    smp_mb();
 18    return s & ~0x1UL;
 19  }

@@ -127,7 +127,7 @@ It is also used in pathname traversal to detect concurrent rename operations.
 24    unsigned long s;
 25
 26    smp_mb();
-27    s = ACCESS_ONCE(slp->seq);
+27    s = READ_ONCE(slp->seq);
 28    return s != oldseq;
 29  }
 30

@@ -211,11 +211,11 @@ in other words, that there has been no writer, and returns true if so.
 	In older versions of the Linux kernel, no.
 
 	In very new versions of the Linux kernel, line~16 could use
-	\co{smp_load_acquire()} instead of \co{ACCESS_ONCE()}, which
+	\co{smp_load_acquire()} instead of \co{READ_ONCE()}, which
 	in turn would allow the \co{smp_mb()} on line~17 to be dropped.
 	Similarly, line~41 could use an \co{smp_store_release()}, for
 	example, as follows: \\
-	\co{smp_store_release(&slp->seq, ACCESS_ONCE(slp->seq) + 1);} \\
+	\co{smp_store_release(&slp->seq, READ_ONCE(slp->seq) + 1);} \\
 	This would allow the \co{smp_mb()} on line~40 to be dropped.
 } \QuickQuizEnd

@@ -310,7 +310,7 @@ increment of the sequence number on line~44, then releases the lock.
 18   s = read_seqbegin(&sl);
 19   repp = &route_list.re_next;
 20   do {
-21     rep = ACCESS_ONCE(*repp);
+21     rep = READ_ONCE(*repp);
 22     if (rep == NULL) {
 23       if (read_seqretry(&sl, s))
 24         goto retry;

@@ -318,7 +318,7 @@ increment of the sequence number on line~44, then releases the lock.
 26     }
 27     repp = &rep->re_next;
 28   } while (rep->addr != addr);
-29   if (ACCESS_ONCE(rep->re_freed))
+29   if (READ_ONCE(rep->re_freed))
 30     abort();
 31   ret = rep->iface;
 32   if (read_seqretry(&sl, s))

Signature · 1 line

2.7.4

Junchang Wang

author

2017-06-10 14:35 UTC #4 lore raw

In reply to Junchang Wang

Hi Paul,

The fourth patch is a false alarm. So I won't send it out. Please review and apply patches 1 - 3. Thanks.

--Jason

Quoted 23 lines · Junchang Wang

Hi Paul,
I'm reading Chapter Deferred Processing, and following are some patches. Please take a look.
--Jason
Junchang Wang (4):
Figure Storage Hazard-Pointer Storage and Erasure: Switch from
ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
route_hazptr: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
route_seqlock: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
 CodeSamples/defer/route_hazptr.c  | 10 +++++-----
 CodeSamples/defer/route_seqlock.c |  6 +++---
 CodeSamples/defer/seqlock.h       |  4 ++--
 defer/hazptr.tex                  | 16 ++++++++--------
 defer/seqlock.tex                 | 12 ++++++------
 5 files changed, 26 insertions(+), 26 deletions(-)
--
2.7.4

Paul E. McKenney

2017-06-10 17:08 UTC #5 lore raw

Quoted 4 lines · Junchang Wang

Hi Paul,
The fourth patch is a false alarm. So I won't send it out. Please review and apply patches 1 - 3. Thanks.

Applied and pushed, thank you!

Please see note below.

Quoted 13 lines

--Jason
Quoted 10 lines · Junchang Wang
Hi Paul,
I'm reading Chapter Deferred Processing, and following are some patches. Please take a look.
--Jason
Junchang Wang (4):
Figure Storage Hazard-Pointer Storage and Erasure: Switch from
ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

When changing text, it suffices to give the chapter's top-level directory name, so I changed this to:

defer: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

Quoted 2 lines

Quoted 2 lines
route_hazptr: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()
route_seqlock: Switch from ACCESS_ONCE() to READ_ONCE()/WRITE_ONCE()

These are fine, giving the name of the executable for changes to a given code sample is quite good, thank you!

Thanx, Paul

Quoted 11 lines

Quoted 10 lines

 CodeSamples/defer/route_hazptr.c  | 10 +++++-----
 CodeSamples/defer/route_seqlock.c |  6 +++---
 CodeSamples/defer/seqlock.h       |  4 ++--
 defer/hazptr.tex                  | 16 ++++++++--------
 defer/seqlock.tex                 | 12 ++++++------
 5 files changed, 26 insertions(+), 26 deletions(-)

--
2.7.4