| Message ID | 20200110232457.19969-1-kamal@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,Bionic] blk-mq: silence false positive warnings in hctx_unlock() | expand |
On 10/01/2020 23:24, Kamal Mostafa wrote: > From: Jens Axboe <axboe@kernel.dk> > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > In some stupider versions of gcc, it complains: > > block/blk-mq.c: In function ‘blk_mq_complete_request’: > ./include/linux/srcu.h:175:2: warning: ‘srcu_idx’ may be used uninitialized in this function [-Wmaybe-uninitialized] > __srcu_read_unlock(sp, idx); > ^ > block/blk-mq.c:620:6: note: ‘srcu_idx’ was declared here > int srcu_idx; > ^ > > which is completely bogus, since we only use srcu_idx when > hctx->flags & BLK_MQ_F_BLOCKING is set, and that's the case where > hctx_lock() has initialized it. > > Just set it to '0' in the normal path in hctx_lock() to silence > this annoying warning. > > Fixes: 04ced159cec8 ("blk-mq: move hctx lock/unlock into a helper") > Fixes: 5197c05e16b4 ("blk-mq: protect completion path with RCU") > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (backported from commit 08b5a6e2a769f720977b245431b45134c0bdd377) > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > Recent blk-mq patches to Bionic for this LP bug introduce more annoying > instances of this false-positive build warning. This trivial backport > from mainline hushes them. > > -Kamal > > --- > block/blk-mq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 52d41d92555e..b4be8229a5cf 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -590,9 +590,11 @@ static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx) > > static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx) > { > - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) > + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) { > + /* shut up gcc false positive */ > + *srcu_idx = 0; > rcu_read_lock(); > - else > + } else > *srcu_idx = srcu_read_lock(hctx->queue_rq_srcu); > } > > Acked-by: Colin Ian King <colin.king@canonical.com>
On Fri, Jan 10, 2020 at 03:24:57PM -0800, Kamal Mostafa wrote: > From: Jens Axboe <axboe@kernel.dk> > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > In some stupider versions of gcc, it complains: > > block/blk-mq.c: In function ‘blk_mq_complete_request’: > ./include/linux/srcu.h:175:2: warning: ‘srcu_idx’ may be used uninitialized in this function [-Wmaybe-uninitialized] > __srcu_read_unlock(sp, idx); > ^ > block/blk-mq.c:620:6: note: ‘srcu_idx’ was declared here > int srcu_idx; > ^ > > which is completely bogus, since we only use srcu_idx when > hctx->flags & BLK_MQ_F_BLOCKING is set, and that's the case where > hctx_lock() has initialized it. > > Just set it to '0' in the normal path in hctx_lock() to silence > this annoying warning. > > Fixes: 04ced159cec8 ("blk-mq: move hctx lock/unlock into a helper") > Fixes: 5197c05e16b4 ("blk-mq: protect completion path with RCU") > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (backported from commit 08b5a6e2a769f720977b245431b45134c0bdd377) > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > Recent blk-mq patches to Bionic for this LP bug introduce more annoying > instances of this false-positive build warning. This trivial backport > from mainline hushes them. > > -Kamal The bug report seems to be marked already as 'Fix Released' in bionic. Maybe we need a new bug? Apart than that: Acked-by: Andrea Righi <andrea.righi@canonical.com> > > --- > block/blk-mq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 52d41d92555e..b4be8229a5cf 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -590,9 +590,11 @@ static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx) > > static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx) > { > - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) > + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) { > + /* shut up gcc false positive */ > + *srcu_idx = 0; > rcu_read_lock(); > - else > + } else > *srcu_idx = srcu_read_lock(hctx->queue_rq_srcu); > } > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 11.01.20 00:24, Kamal Mostafa wrote: > From: Jens Axboe <axboe@kernel.dk> > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > In some stupider versions of gcc, it complains: > > block/blk-mq.c: In function ‘blk_mq_complete_request’: > ./include/linux/srcu.h:175:2: warning: ‘srcu_idx’ may be used uninitialized in this function [-Wmaybe-uninitialized] > __srcu_read_unlock(sp, idx); > ^ > block/blk-mq.c:620:6: note: ‘srcu_idx’ was declared here > int srcu_idx; > ^ > > which is completely bogus, since we only use srcu_idx when > hctx->flags & BLK_MQ_F_BLOCKING is set, and that's the case where > hctx_lock() has initialized it. > > Just set it to '0' in the normal path in hctx_lock() to silence > this annoying warning. > > Fixes: 04ced159cec8 ("blk-mq: move hctx lock/unlock into a helper") > Fixes: 5197c05e16b4 ("blk-mq: protect completion path with RCU") > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (backported from commit 08b5a6e2a769f720977b245431b45134c0bdd377) > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > > BugLink: https://bugs.launchpad.net/bugs/1848739 > > Recent blk-mq patches to Bionic for this LP bug introduce more annoying > instances of this false-positive build warning. This trivial backport > from mainline hushes them. > > -Kamal > > --- > block/blk-mq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 52d41d92555e..b4be8229a5cf 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -590,9 +590,11 @@ static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx) > > static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx) > { > - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) > + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) { > + /* shut up gcc false positive */ > + *srcu_idx = 0; > rcu_read_lock(); > - else > + } else > *srcu_idx = srcu_read_lock(hctx->queue_rq_srcu); > } > > Applied to bionic/linux. Thanks, Kleber
diff --git a/block/blk-mq.c b/block/blk-mq.c index 52d41d92555e..b4be8229a5cf 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -590,9 +590,11 @@ static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx) static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx) { - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) { + /* shut up gcc false positive */ + *srcu_idx = 0; rcu_read_lock(); - else + } else *srcu_idx = srcu_read_lock(hctx->queue_rq_srcu); }