fs/buffer: Make BH_Uptodate_Lock bit_spin_lock a regular spinlock_t

Bit spinlocks are problematic if PREEMPT_RT is enabled, because they
disable preemption, which is undesired for latency reasons and breaks when
regular spinlocks are taken within the bit_spinlock locked region because
regular spinlocks are converted to 'sleeping spinlocks' on RT.

PREEMPT_RT replaced the bit spinlocks with regular spinlocks to avoid this
problem. The replacement was done conditionaly at compile time, but
Christoph requested to do an unconditional conversion.

Jan suggested to move the spinlock into a existing padding hole which
avoids a size increase of struct buffer_head on production kernels.

As a benefit the lock gains lockdep coverage.

[ bigeasy: Remove the wrapper and use always spinlock_t and move it into
           the padding hole ]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@infradead.org>
Link: https://lkml.kernel.org/r/20191118132824.rclhrbujqh4b4g4d@linutronix.de

diff --git a/fs/buffer.c b/fs/buffer.c
index b8d28370cfd7f25cb5ed6e44fa7a073547ba5319..6b3495827cad3ad2b9518a2b369e845328e18c14 100644 (file)
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -274,8 +274,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
         * decide that the page is now completely done.
         */
        first = page_buffers(page);
-       local_irq_save(flags);
-       bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+       spin_lock_irqsave(&first->b_uptodate_lock, flags);
        clear_buffer_async_read(bh);
        unlock_buffer(bh);
        tmp = bh;
@@ -288,8 +287,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
                }
                tmp = tmp->b_this_page;
        } while (tmp != bh);
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
 
        /*
         * If none of the buffers had errors and they are all
@@ -301,8 +299,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
        return;
 
 still_busy:
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
        return;
 }
 
@@ -371,8 +368,7 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate)
        }
 
        first = page_buffers(page);
-       local_irq_save(flags);
-       bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+       spin_lock_irqsave(&first->b_uptodate_lock, flags);
 
        clear_buffer_async_write(bh);
        unlock_buffer(bh);
@@ -384,14 +380,12 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate)
                }
                tmp = tmp->b_this_page;
        }
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
        end_page_writeback(page);
        return;
 
 still_busy:
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
        return;
 }
 EXPORT_SYMBOL(end_buffer_async_write);
@@ -3385,6 +3379,7 @@ struct buffer_head *alloc_buffer_head(gfp_t gfp_flags)
        struct buffer_head *ret = kmem_cache_zalloc(bh_cachep, gfp_flags);
        if (ret) {
                INIT_LIST_HEAD(&ret->b_assoc_buffers);
+               spin_lock_init(&ret->b_uptodate_lock);
                preempt_disable();
                __this_cpu_inc(bh_accounting.nr);
                recalc_bh_state();

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 68b39e75446a158d4bc8db2381f6072c5bb2ffb0..de6fe969f77371e57893cbcc43c435cf69154c9a 100644 (file)
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -125,11 +125,10 @@ static void ext4_finish_bio(struct bio *bio)
                }
                bh = head = page_buffers(page);
                /*
-                * We check all buffers in the page under BH_Uptodate_Lock
+                * We check all buffers in the page under b_uptodate_lock
                 * to avoid races with other end io clearing async_write flags
                 */
-               local_irq_save(flags);
-               bit_spin_lock(BH_Uptodate_Lock, &head->b_state);
+               spin_lock_irqsave(&head->b_uptodate_lock, flags);
                do {
                        if (bh_offset(bh) < bio_start ||
                            bh_offset(bh) + bh->b_size > bio_end) {
@@ -141,8 +140,7 @@ static void ext4_finish_bio(struct bio *bio)
                        if (bio->bi_status)
                                buffer_io_error(bh);
                } while ((bh = bh->b_this_page) != head);
-               bit_spin_unlock(BH_Uptodate_Lock, &head->b_state);
-               local_irq_restore(flags);
+               spin_unlock_irqrestore(&head->b_uptodate_lock, flags);
                if (!under_io) {
                        fscrypt_free_bounce_page(bounce_page);
                        end_page_writeback(page);

diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
index 7202a1e39d70c2c4cada9fa7ff2ac23724321575..554b744f41bf8a6f72d145fe0733180772d2a435 100644 (file)
--- a/fs/ntfs/aops.c
+++ b/fs/ntfs/aops.c
@@ -92,8 +92,7 @@ static void ntfs_end_buffer_async_read(struct buffer_head *bh, int uptodate)
                                "0x%llx.", (unsigned long long)bh->b_blocknr);
        }
        first = page_buffers(page);
-       local_irq_save(flags);
-       bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+       spin_lock_irqsave(&first->b_uptodate_lock, flags);
        clear_buffer_async_read(bh);
        unlock_buffer(bh);
        tmp = bh;
@@ -108,8 +107,7 @@ static void ntfs_end_buffer_async_read(struct buffer_head *bh, int uptodate)
                }
                tmp = tmp->b_this_page;
        } while (tmp != bh);
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
        /*
         * If none of the buffers had errors then we can set the page uptodate,
         * but we first have to perform the post read mst fixups, if the
@@ -142,8 +140,7 @@ static void ntfs_end_buffer_async_read(struct buffer_head *bh, int uptodate)
        unlock_page(page);
        return;
 still_busy:
-       bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-       local_irq_restore(flags);
+       spin_unlock_irqrestore(&first->b_uptodate_lock, flags);
        return;
 }
 

diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 7b73ef7f902d4ffc1d481e4031e708b815f4513f..e0b020eaf32e252ba7536958f5035747fe358d0a 100644 (file)
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -22,9 +22,6 @@ enum bh_state_bits {
        BH_Dirty,       /* Is dirty */
        BH_Lock,        /* Is locked */
        BH_Req,         /* Has been submitted for I/O */
-       BH_Uptodate_Lock,/* Used by the first bh in a page, to serialise
-                         * IO completion of other buffers in the page
-                         */
 
        BH_Mapped,      /* Has a disk mapping */
        BH_New,         /* Disk mapping was newly created by get_block */
@@ -76,6 +73,9 @@ struct buffer_head {
        struct address_space *b_assoc_map;      /* mapping this buffer is
                                                   associated with */
        atomic_t b_count;               /* users using this buffer_head */
+       spinlock_t b_uptodate_lock;     /* Used by the first bh in a page, to
+                                        * serialise IO completion of other
+                                        * buffers in the page */
 };
 
 /*