| Message ID | 20200602013208.4325-2-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blktrace: fix sparse warnings | expand |
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.
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.
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/
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;
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(-)