blktrace: Avoid sparse warnings when assigning q->blk_trace
authorJan Kara <jack@suse.cz>
Fri, 5 Jun 2020 14:58:37 +0000 (16:58 +0200)
committerJens Axboe <axboe@kernel.dk>
Wed, 17 Jun 2020 15:07:11 +0000 (09:07 -0600)
Mostly for historical reasons, q->blk_trace is assigned through xchg()
and cmpxchg() atomic operations. Although this is correct, sparse
complains about this because it violates rcu annotations since commit
c780e86dd48e ("blktrace: Protect q->blk_trace with RCU") which started
to use rcu for accessing q->blk_trace. Furthermore there's no real need
for atomic operations anymore since all changes to q->blk_trace happen
under q->blk_trace_mutex and since it also makes more sense to check if
q->blk_trace is set with the mutex held earlier.

So let's just replace xchg() with rcu_replace_pointer() and cmpxchg()
with explicit check and rcu_assign_pointer(). This makes the code more
efficient and sparse happy.

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
kernel/trace/blktrace.c

index 8fd36e8..5ef0484 100644 (file)
@@ -347,7 +347,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;
 
@@ -501,7 +502,8 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
         * bdev can be NULL, as with scsi-generic, this is a helpful as
         * we can be.
         */
-       if (q->blk_trace) {
+       if (rcu_dereference_protected(q->blk_trace,
+                                     lockdep_is_held(&q->blk_trace_mutex))) {
                pr_warn("Concurrent blktraces are not allowed on %s\n",
                        buts->name);
                return -EBUSY;
@@ -556,10 +558,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
        bt->pid = buts->pid;
        bt->trace_state = Blktrace_setup;
 
-       ret = -EBUSY;
-       if (cmpxchg(&q->blk_trace, NULL, bt))
-               goto err;
-
+       rcu_assign_pointer(q->blk_trace, bt);
        get_probe_ref();
 
        ret = 0;
@@ -1642,7 +1641,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;
 
@@ -1674,10 +1674,7 @@ 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))
-               goto free_bt;
-
+       rcu_assign_pointer(q->blk_trace, bt);
        get_probe_ref();
        return 0;