| From: Davidlohr Bueso <dave@stgolabs.ne> |
| Date: Fri, 30 Oct 2015 05:25:59 +0900 |
| Subject: blktrace: re-write setting q->blk_trace |
| |
| commit cdea01b2bf98affb7e9c44530108a4a28535eee8 upstream. |
| |
| This is really about simplifying the double xchg patterns into |
| a single cmpxchg, with the same logic. Other than the immediate |
| cleanup, there are some subtleties this change deals with: |
| |
| (i) While the load of the old bt is fully ordered wrt everything, |
| ie: |
| |
| old_bt = xchg(&q->blk_trace, bt); [barrier] |
| if (old_bt) |
| (void) xchg(&q->blk_trace, old_bt); [barrier] |
| |
| blk_trace could still be changed between the xchg and the old_bt |
| load. Note that this description is merely theoretical and afaict |
| very small, but doing everything in a single context with cmpxchg |
| closes this potential race. |
| |
| (ii) Ordering guarantees are obviously kept with cmpxchg. |
| |
| (iii) Gets rid of the hacky-by-nature (void)xchg pattern. |
| |
| Signed-off-by: Davidlohr Bueso <dbueso@suse.de> |
| eviewed-by: Jeff Moyer <jmoyer@redhat.com> |
| Signed-off-by: Jens Axboe <axboe@fb.com> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| kernel/trace/blktrace.c | 16 +++++----------- |
| 1 file changed, 5 insertions(+), 11 deletions(-) |
| |
| --- a/kernel/trace/blktrace.c |
| +++ b/kernel/trace/blktrace.c |
| @@ -448,7 +448,7 @@ int do_blk_trace_setup(struct request_qu |
| struct block_device *bdev, |
| struct blk_user_trace_setup *buts) |
| { |
| - struct blk_trace *old_bt, *bt = NULL; |
| + struct blk_trace *bt = NULL; |
| struct dentry *dir = NULL; |
| int ret, i; |
| |
| @@ -532,11 +532,8 @@ int do_blk_trace_setup(struct request_qu |
| bt->trace_state = Blktrace_setup; |
| |
| ret = -EBUSY; |
| - old_bt = xchg(&q->blk_trace, bt); |
| - if (old_bt) { |
| - (void) xchg(&q->blk_trace, old_bt); |
| + if (cmpxchg(&q->blk_trace, NULL, bt)) |
| goto err; |
| - } |
| |
| if (atomic_inc_return(&blk_probes_ref) == 1) |
| blk_register_tracepoints(); |
| @@ -1550,7 +1547,7 @@ static int blk_trace_remove_queue(struct |
| static int blk_trace_setup_queue(struct request_queue *q, |
| struct block_device *bdev) |
| { |
| - struct blk_trace *old_bt, *bt = NULL; |
| + struct blk_trace *bt = NULL; |
| int ret = -ENOMEM; |
| |
| bt = kzalloc(sizeof(*bt), GFP_KERNEL); |
| @@ -1566,12 +1563,9 @@ static int blk_trace_setup_queue(struct |
| |
| blk_trace_setup_lba(bt, bdev); |
| |
| - old_bt = xchg(&q->blk_trace, bt); |
| - if (old_bt != NULL) { |
| - (void)xchg(&q->blk_trace, old_bt); |
| - ret = -EBUSY; |
| + ret = -EBUSY; |
| + if (cmpxchg(&q->blk_trace, NULL, bt)) |
| goto free_bt; |
| - } |
| |
| if (atomic_inc_return(&blk_probes_ref) == 1) |
| blk_register_tracepoints(); |