null_blk: Fix scheduling in atomic with zoned mode

Commit aa1c09cb65e2 ("null_blk: Fix locking in zoned mode") changed
zone locking to using the potentially sleeping wait_on_bit_io()
function. This is acceptable when memory backing is enabled as the
device queue is in that case marked as blocking, but this triggers a
scheduling while in atomic context with memory backing disabled.

Fix this by relying solely on the device zone spinlock for zone
information protection without temporarily releasing this lock around
null_process_cmd() execution in null_zone_write(). This is OK to do
since when memory backing is disabled, command processing does not
block and the memory backing lock nullb->lock is unused. This solution
avoids the overhead of having to mark a zoned null_blk device queue as
blocking when memory backing is unused.

This patch also adds comments to the zone locking code to explain the
unusual locking scheme.

Fixes: aa1c09cb65e2 ("null_blk: Fix locking in zoned mode")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: stable@vger.kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index cfd00ad403556eb2acc4427efddefbdb93e99124..c24d9b5ad81af26c2761daf8a29948a44e8ff607 100644 (file)
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -47,7 +47,7 @@ struct nullb_device {
        unsigned int nr_zones_closed;
        struct blk_zone *zones;
        sector_t zone_size_sects;
        unsigned int nr_zones_closed;
        struct blk_zone *zones;
        sector_t zone_size_sects;

-       spinlock_t zone_dev_lock;
+       spinlock_t zone_lock;

        unsigned long *zone_locks;
 
        unsigned long size; /* device size in MB */
        unsigned long *zone_locks;
 
        unsigned long size; /* device size in MB */

diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index 8775acbb4f8fa7db382a97251c6723164a2b357a..beb34b4f76b0e6c7f2fe91167fea922c786e5f3a 100644 (file)
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -46,11 +46,20 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
        if (!dev->zones)
                return -ENOMEM;
 
        if (!dev->zones)
                return -ENOMEM;
 

-       spin_lock_init(&dev->zone_dev_lock);
-       dev->zone_locks = bitmap_zalloc(dev->nr_zones, GFP_KERNEL);
-       if (!dev->zone_locks) {
-               kvfree(dev->zones);
-               return -ENOMEM;
+       /*
+        * With memory backing, the zone_lock spinlock needs to be temporarily
+        * released to avoid scheduling in atomic context. To guarantee zone
+        * information protection, use a bitmap to lock zones with
+        * wait_on_bit_lock_io(). Sleeping on the lock is OK as memory backing
+        * implies that the queue is marked with BLK_MQ_F_BLOCKING.
+        */
+       spin_lock_init(&dev->zone_lock);
+       if (dev->memory_backed) {
+               dev->zone_locks = bitmap_zalloc(dev->nr_zones, GFP_KERNEL);
+               if (!dev->zone_locks) {
+                       kvfree(dev->zones);
+                       return -ENOMEM;
+               }

        }
 
        if (dev->zone_nr_conv >= dev->nr_zones) {
        }
 
        if (dev->zone_nr_conv >= dev->nr_zones) {


@@ -137,12 +146,17 @@ void null_free_zoned_dev(struct nullb_device *dev)
 
 static inline void null_lock_zone(struct nullb_device *dev, unsigned int zno)
 {
 
 static inline void null_lock_zone(struct nullb_device *dev, unsigned int zno)
 {

-       wait_on_bit_lock_io(dev->zone_locks, zno, TASK_UNINTERRUPTIBLE);
+       if (dev->memory_backed)
+               wait_on_bit_lock_io(dev->zone_locks, zno, TASK_UNINTERRUPTIBLE);
+       spin_lock_irq(&dev->zone_lock);

 }
 
 static inline void null_unlock_zone(struct nullb_device *dev, unsigned int zno)
 {
 }
 
 static inline void null_unlock_zone(struct nullb_device *dev, unsigned int zno)
 {

-       clear_and_wake_up_bit(zno, dev->zone_locks);
+       spin_unlock_irq(&dev->zone_lock);
+
+       if (dev->memory_backed)
+               clear_and_wake_up_bit(zno, dev->zone_locks);

 }
 
 int null_report_zones(struct gendisk *disk, sector_t sector,
 }
 
 int null_report_zones(struct gendisk *disk, sector_t sector,


@@ -322,7 +336,6 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
                return null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);
 
        null_lock_zone(dev, zno);
                return null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);
 
        null_lock_zone(dev, zno);

-       spin_lock(&dev->zone_dev_lock);

 
        switch (zone->cond) {
        case BLK_ZONE_COND_FULL:
 
        switch (zone->cond) {
        case BLK_ZONE_COND_FULL:


@@ -375,9 +388,17 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
        if (zone->cond != BLK_ZONE_COND_EXP_OPEN)
                zone->cond = BLK_ZONE_COND_IMP_OPEN;
 
        if (zone->cond != BLK_ZONE_COND_EXP_OPEN)
                zone->cond = BLK_ZONE_COND_IMP_OPEN;
 

-       spin_unlock(&dev->zone_dev_lock);
+       /*
+        * Memory backing allocation may sleep: release the zone_lock spinlock
+        * to avoid scheduling in atomic context. Zone operation atomicity is
+        * still guaranteed through the zone_locks bitmap.
+        */
+       if (dev->memory_backed)
+               spin_unlock_irq(&dev->zone_lock);

        ret = null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);
        ret = null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);

-       spin_lock(&dev->zone_dev_lock);
+       if (dev->memory_backed)
+               spin_lock_irq(&dev->zone_lock);
+

        if (ret != BLK_STS_OK)
                goto unlock;
 
        if (ret != BLK_STS_OK)
                goto unlock;
 


@@ -392,7 +413,6 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
        ret = BLK_STS_OK;
 
 unlock:
        ret = BLK_STS_OK;
 
 unlock:

-       spin_unlock(&dev->zone_dev_lock);

        null_unlock_zone(dev, zno);
 
        return ret;
        null_unlock_zone(dev, zno);
 
        return ret;


@@ -516,9 +536,7 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
                        null_lock_zone(dev, i);
                        zone = &dev->zones[i];
                        if (zone->cond != BLK_ZONE_COND_EMPTY) {
                        null_lock_zone(dev, i);
                        zone = &dev->zones[i];
                        if (zone->cond != BLK_ZONE_COND_EMPTY) {

-                               spin_lock(&dev->zone_dev_lock);

                                null_reset_zone(dev, zone);
                                null_reset_zone(dev, zone);

-                               spin_unlock(&dev->zone_dev_lock);

                                trace_nullb_zone_op(cmd, i, zone->cond);
                        }
                        null_unlock_zone(dev, i);
                                trace_nullb_zone_op(cmd, i, zone->cond);
                        }
                        null_unlock_zone(dev, i);


@@ -530,7 +548,6 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
        zone = &dev->zones[zone_no];
 
        null_lock_zone(dev, zone_no);
        zone = &dev->zones[zone_no];
 
        null_lock_zone(dev, zone_no);

-       spin_lock(&dev->zone_dev_lock);

 
        switch (op) {
        case REQ_OP_ZONE_RESET:
 
        switch (op) {
        case REQ_OP_ZONE_RESET:


@@ -550,8 +567,6 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
                break;
        }
 
                break;
        }
 

-       spin_unlock(&dev->zone_dev_lock);
-

        if (ret == BLK_STS_OK)
                trace_nullb_zone_op(cmd, zone_no, zone->cond);
 
        if (ret == BLK_STS_OK)
                trace_nullb_zone_op(cmd, zone_no, zone->cond);