3.16.83-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Davidlohr Bueso dave@stgolabs.ne
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();