* [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
@ 2019-02-11 18:32 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-02-11 18:32 UTC (permalink / raw)
To: Chris Mason, Anand Jain
Cc: Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
We should drop the lock on this error path. This is from static
analysis and I don't know if it's possible to hit this error path in
real life.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
fs/btrfs/dev-replace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 13863354ff9d..ee193c5222b2 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -862,6 +862,7 @@ int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)
btrfs_destroy_dev_replace_tgtdev(tgt_device);
break;
default:
+ up_write(&dev_replace->rwsem);
result = -EINVAL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
@ 2019-02-11 18:32 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-02-11 18:32 UTC (permalink / raw)
To: Chris Mason, Anand Jain
Cc: Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
We should drop the lock on this error path. This is from static
analysis and I don't know if it's possible to hit this error path in
real life.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
fs/btrfs/dev-replace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 13863354ff9d..ee193c5222b2 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -862,6 +862,7 @@ int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)
btrfs_destroy_dev_replace_tgtdev(tgt_device);
break;
default:
+ up_write(&dev_replace->rwsem);
result = -EINVAL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
2019-02-11 18:32 ` Dan Carpenter
@ 2019-02-19 19:04 ` David Sterba
-1 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-02-19 19:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: Chris Mason, Anand Jain, Josef Bacik, David Sterba, linux-btrfs,
kernel-janitors
On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
> We should drop the lock on this error path. This is from static
> analysis and I don't know if it's possible to hit this error path in
> real life.
Yes the lock needs to be released, it's there to protect access to the
dev_replace members and is not supposed to be left locked. The value of
state that's being switched would need to be artifically changed to an
invalid value so the default: branch is taken.
It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
a regression but I don't think it's urgent enough to be sent to a late
rc. It'll go through the stable tree channel. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
@ 2019-02-19 19:04 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-02-19 19:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: Chris Mason, Anand Jain, Josef Bacik, David Sterba, linux-btrfs,
kernel-janitors
On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
> We should drop the lock on this error path. This is from static
> analysis and I don't know if it's possible to hit this error path in
> real life.
Yes the lock needs to be released, it's there to protect access to the
dev_replace members and is not supposed to be left locked. The value of
state that's being switched would need to be artifically changed to an
invalid value so the default: branch is taken.
It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
a regression but I don't think it's urgent enough to be sent to a late
rc. It'll go through the stable tree channel. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
2019-02-19 19:04 ` David Sterba
@ 2019-02-20 1:20 ` Anand Jain
-1 siblings, 0 replies; 6+ messages in thread
From: Anand Jain @ 2019-02-20 1:20 UTC (permalink / raw)
To: dsterba, Dan Carpenter
Cc: Chris Mason, Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
On 2/20/19 3:04 AM, David Sterba wrote:
> On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
>> We should drop the lock on this error path. This is from static
>> analysis and I don't know if it's possible to hit this error path in
>> real life.
>
> Yes the lock needs to be released, it's there to protect access to the
> dev_replace members and is not supposed to be left locked. The value of
> state that's being switched would need to be artifically changed to an
> invalid value so the default: branch is taken.
>
> It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
> a regression but I don't think it's urgent enough to be sent to a late
> rc. It'll go through the stable tree channel. Thanks.
>
oops I missed this email. Thanks Dan and David.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel()
@ 2019-02-20 1:20 ` Anand Jain
0 siblings, 0 replies; 6+ messages in thread
From: Anand Jain @ 2019-02-20 1:20 UTC (permalink / raw)
To: dsterba, Dan Carpenter
Cc: Chris Mason, Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
On 2/20/19 3:04 AM, David Sterba wrote:
> On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
>> We should drop the lock on this error path. This is from static
>> analysis and I don't know if it's possible to hit this error path in
>> real life.
>
> Yes the lock needs to be released, it's there to protect access to the
> dev_replace members and is not supposed to be left locked. The value of
> state that's being switched would need to be artifically changed to an
> invalid value so the default: branch is taken.
>
> It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
> a regression but I don't think it's urgent enough to be sent to a late
> rc. It'll go through the stable tree channel. Thanks.
>
oops I missed this email. Thanks Dan and David.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-20 1:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 18:32 [PATCH] btrfs: drop the lock on error in btrfs_dev_replace_cancel() Dan Carpenter
2019-02-11 18:32 ` Dan Carpenter
2019-02-19 19:04 ` David Sterba
2019-02-19 19:04 ` David Sterba
2019-02-20 1:20 ` Anand Jain
2019-02-20 1:20 ` Anand Jain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.