Skip to content

Commit

Permalink
blktrace: Avoid sparse warnings when assigning q->blk_trace
Browse files Browse the repository at this point in the history
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
c780e86 ("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>
  • Loading branch information
Jan Kara authored and Jens Axboe committed Jun 17, 2020
1 parent 1b0b283 commit c3dbe54
Showing 1 changed file with 8 additions and 11 deletions.
19 changes: 8 additions & 11 deletions kernel/trace/blktrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit c3dbe54

Please sign in to comment.