Re: [PATCH 05/10] ext4: serialize dio nonlocked reads with defrag workers V3
From: Jan Kara <jack@suse.cz>
Date: 2012-09-26 13:49:51
On Mon 24-09-12 15:44:15, Dmitry Monakhov wrote:
quoted hunk ↗ jump to hunk
Inode's block defrag and ext4_change_inode_journal_flag() may affect nonlocked DIO reads result, so proper synchronization required. - Add missed inode_dio_wait() calls where appropriate - Check inode state under extra i_dio_count reference. Changes from V1: - add missed memory bariers - move DIOREAD_LOCK state check out from generic should_dioread_nolock otherwise it will affect existing DIO readers. Signed-off-by: Dmitry Monakhov <redacted> --- fs/ext4/ext4.h | 17 +++++++++++++++++ fs/ext4/indirect.c | 13 +++++++++++++ fs/ext4/inode.c | 5 +++++ fs/ext4/move_extent.c | 8 ++++++++ 4 files changed, 43 insertions(+), 0 deletions(-)diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index cc423cf..fccd263 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h@@ -1350,6 +1350,8 @@ enum { EXT4_STATE_DIO_UNWRITTEN, /* need convert on dio done*/ EXT4_STATE_NEWENTRY, /* File just added to dir */ EXT4_STATE_DELALLOC_RESERVED, /* blks already reserved for delalloc */ + EXT4_STATE_DIOREAD_LOCK, /* Disable support for dio read + nolocking */ }; #define EXT4_INODE_BIT_FNS(name, field, offset) \@@ -2462,6 +2464,21 @@ static inline void set_bitmap_uptodate(struct buffer_head *bh) set_bit(BH_BITMAP_UPTODATE, &(bh)->b_state); } +/* + * Disable DIO read nolock optimization, so new dioreaders will be forced + * to grab i_mutex + */ +static inline void ext4_inode_block_unlocked_dio(struct inode *inode) +{ + ext4_set_inode_state(inode, EXT4_STATE_DIOREAD_LOCK); + smp_mb(); +} +static inline void ext4_inode_resume_unlocked_dio(struct inode *inode) +{ + smp_mb(); + ext4_clear_inode_state(inode, EXT4_STATE_DIOREAD_LOCK); +} + #define in_range(b, first, len) ((b) >= (first) && (b) <= (first) + (len) - 1) /* For ioend & aio unwritten conversion wait queues */diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c index 251e35f..ec38f81 100644 --- a/fs/ext4/indirect.c +++ b/fs/ext4/indirect.c@@ -810,10 +810,23 @@ retry: if (unlikely(!list_empty(&ei->i_completed_io_list))) ext4_unwritten_wait(inode); + /* + * Nolock dioread optimization may be dynamically disabled + * via ext4_inode_block_unlocked_dio(). Check inode's state + * while holding extra i_dio_count ref. + */ + atomic_inc(&inode->i_dio_count); + smp_mb(); + if (!unlikely(ext4_test_inode_state(inode, + EXT4_STATE_DIOREAD_LOCK))) {
Isn't this condition reverted? We want to retry if EXT4_STATE_DIOREAD_LOCK is set, don't we? Honza -- Jan Kara [off-list ref] SUSE Labs, CR