2024-01-24 07:25:51

by Maksim Kiselev

Subject: [PATCH v1 1/1] aoe: avoid potential deadlock at set_capacity

Move set_capacity() outside of the section procected by (&d->lock).
To avoid possible interrupt unsafe locking scenario:

CPU0 CPU1
---- ----
[1] lock(&bdev->bd_size_lock);
local_irq_disable();
[2] lock(&d->lock);
[3] lock(&bdev->bd_size_lock);
<Interrupt>
[4] lock(&d->lock);

*** DEADLOCK ***

Where [1](&bdev->bd_size_lock) hold by zram_add()->set_capacity().
[2]lock(&d->lock) hold by aoeblk_gdalloc(). And aoeblk_gdalloc()
is trying to acquire [3](&bdev->bd_size_lock) at set_capacity() call.
In this situation an attempt to acquire [4]lock(&d->lock) from
aoecmd_cfg_rsp() will lead to deadlock.

So the simplest solution is breaking lock dependency
[2](&d->lock) -> [3](&bdev->bd_size_lock) by moving set_capacity()
outside.

Signed-off-by: Maksim Kiselev <[email protected]>
---
drivers/block/aoe/aoeblk.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index d2dbf8aaccb5..b1b47d88f5db 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -333,6 +333,7 @@ aoeblk_gdalloc(void *vp)
struct gendisk *gd;
mempool_t *mp;
struct blk_mq_tag_set *set;
+ sector_t ssize;
ulong flags;
int late = 0;
int err;
@@ -396,7 +397,7 @@ aoeblk_gdalloc(void *vp)
gd->minors = AOE_PARTITIONS;
gd->fops = &aoe_bdops;
gd->private_data = d;
- set_capacity(gd, d->ssize);
+ ssize = d->ssize;
snprintf(gd->disk_name, sizeof gd->disk_name, "etherd/e%ld.%d",
d->aoemajor, d->aoeminor);

@@ -405,6 +406,8 @@ aoeblk_gdalloc(void *vp)

spin_unlock_irqrestore(&d->lock, flags);

+ set_capacity(gd, ssize);
+
err = device_add_disk(NULL, gd, aoe_attr_groups);
if (err)
goto out_disk_cleanup;
--
2.40.1

2024-01-24 10:23:45

by Maksim Kiselev

Subject: Re: [PATCH v1 1/1] aoe: avoid potential deadlock at set_capacity

Hi, Christoph

ср, 24 янв. 2024 г. в 12:18, Christoph Hellwig <[email protected]>:
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> Just curious: what is your rason for using aeo over nbd?

The main reason why I use aoe, because it's not require a userspace
client. All magic can be done by the kernel itself :)
Just enough to specify an aoe interface via cmdline or bootparams.

Cheers,
Maksim

2024-01-24 14:12:21

by Christoph Hellwig

Subject: Re: [PATCH v1 1/1] aoe: avoid potential deadlock at set_capacity

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

Just curious: what is your rason for using aeo over nbd?