[1/4] blktrace: use rcu calls to access q->blk_trace
diff mbox series

Message ID 20200602013208.4325-2-chaitanya.kulkarni@wdc.com
State New
Headers show
Series
  • blktrace: fix sparse warnings
Related show

Commit Message

Chaitanya Kulkarni June 2, 2020, 1:32 a.m. UTC
Since request queue's blk_trace member is been markds as __rcu,
replace xchg() and cmdxchg() calls with appropriate rcu API.
This removes the sparse warnings.

The xchg() call is replaced with rcu_replace_pointer() since all the
calls for xchg are protected are q->blk_trace_mutex. On replacing
the pointer rcu_replace_pointer returns the old value if that value is
NULL then existing code returns an error.

In setup functions cmpxchg() call is replaced with calls to
rcu_dereference_pointer() and rcu_replace_pointer(). The first
dereference pointer call returns the existing value. If the value is
not NULL existing code returns an error, otherwise the second call to
replace pointer sets the new value.        

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 kernel/trace/blktrace.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Bart Van Assche June 2, 2020, 2:27 a.m. UTC | #1
On 2020-06-01 18:32, Chaitanya Kulkarni wrote:
> Since request queue's blk_trace member is been markds as __rcu,
> replace xchg() and cmdxchg() calls with appropriate rcu API.
> This removes the sparse warnings.

This patch looks like a subset of a patch that was posted a few days ago
by Jan Kara? See also
https://lore.kernel.org/linux-block/20200528092910.11118-1-jack@suse.cz/.

Bart.
Chaitanya Kulkarni June 2, 2020, 3:10 a.m. UTC | #2
On 6/1/20 7:27 PM, Bart Van Assche wrote:
> On 2020-06-01 18:32, Chaitanya Kulkarni wrote:
>> Since request queue's blk_trace member is been markds as __rcu,
>> replace xchg() and cmdxchg() calls with appropriate rcu API.
>> This removes the sparse warnings.
> This patch looks like a subset of a patch that was posted a few days ago
> by Jan Kara? See also
> https://lore.kernel.org/linux-block/20200528092910.11118-1-jack@suse.cz/.
> 
> Bart.
> 

Thanks for pointing out. I think my patch maintains the goto labels and
keeps the original code flow with two calls as mentioned in the patch
description. Maybe we can merge both patches keeping Jan as an Original 
author since his patch was sent first ?

At the end of the I just want to fix these warnings before I re-spin
blktrace extension series.
Jan Kara June 2, 2020, 6:52 a.m. UTC | #3
On Tue 02-06-20 03:10:42, Chaitanya Kulkarni wrote:
> On 6/1/20 7:27 PM, Bart Van Assche wrote:
> > On 2020-06-01 18:32, Chaitanya Kulkarni wrote:
> >> Since request queue's blk_trace member is been markds as __rcu,
> >> replace xchg() and cmdxchg() calls with appropriate rcu API.
> >> This removes the sparse warnings.
> > This patch looks like a subset of a patch that was posted a few days ago
> > by Jan Kara? See also
> > https://lore.kernel.org/linux-block/20200528092910.11118-1-jack@suse.cz/.
> > 
> > Bart.
> > 
> 
> Thanks for pointing out. I think my patch maintains the goto labels and
> keeps the original code flow with two calls as mentioned in the patch
> description. Maybe we can merge both patches keeping Jan as an Original 
> author since his patch was sent first ?

Yes, my patch also removes the unnecessary atomic operations besides fixing
the sparse warnings. I'll rebase it on top of Luis' patches [1] which conflict
with it and resend it.

								Honza

[1] https://lore.kernel.org/linux-block/20200516031956.2605-1-mcgrof@kernel.org/

Patch
diff mbox series

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index ea47f2084087..27c19db01ba4 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -344,7 +344,8 @@  static int __blk_trace_remove(struct request_queue *q)
 {
 	struct blk_trace *bt;
 
-	bt = xchg(&q->blk_trace, NULL);
+	bt = rcu_replace_pointer(q->blk_trace, NULL,
+				 lockdep_is_held(&q->blk_trace_mutex));
 	if (!bt)
 		return -EINVAL;
 
@@ -544,9 +545,13 @@  static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 	bt->trace_state = Blktrace_setup;
 
 	ret = -EBUSY;
-	if (cmpxchg(&q->blk_trace, NULL, bt))
+	if (rcu_dereference_protected(q->blk_trace,
+				      lockdep_is_held(&q->blk_trace_mutex)))
 		goto err;
 
+	rcu_replace_pointer(q->blk_trace, bt,
+			    lockdep_is_held(&q->blk_trace_mutex));
+
 	get_probe_ref();
 
 	ret = 0;
@@ -1637,7 +1642,8 @@  static int blk_trace_remove_queue(struct request_queue *q)
 {
 	struct blk_trace *bt;
 
-	bt = xchg(&q->blk_trace, NULL);
+	bt = rcu_replace_pointer(q->blk_trace, NULL,
+				 lockdep_is_held(&q->blk_trace_mutex));
 	if (bt == NULL)
 		return -EINVAL;
 
@@ -1670,9 +1676,13 @@  static int blk_trace_setup_queue(struct request_queue *q,
 	blk_trace_setup_lba(bt, bdev);
 
 	ret = -EBUSY;
-	if (cmpxchg(&q->blk_trace, NULL, bt))
+	if (rcu_dereference_protected(q->blk_trace,
+				      lockdep_is_held(&q->blk_trace_mutex)))
 		goto free_bt;
 
+	rcu_replace_pointer(q->blk_trace, bt,
+			    lockdep_is_held(&q->blk_trace_mutex));
+
 	get_probe_ref();
 	return 0;