From: Kamil Chojnowski attr_make_nonresident() copies resident $DATA into folio 0 and locks that folio while its callers hold ni_lock and run_lock: write(2) / ftruncate(2) / fallocate(2) ntfs_set_size() / attr_force_nonresident() ni_lock, run_lock (W) attr_set_size_ex() attr_make_nonresident() __filemap_get_folio(0, FGP_LOCK) <- waits for folio 0 read_folio and writeback take the same locks in the opposite order: filemap_read_folio() folio 0 locked iomap_read_folio() ntfs_iomap_begin() attr_data_get_block() run_lock, ni_lock ntfs_writepages() folio 0 locked ni_lock() (resident file) So a small file that grows past what fits in its MFT record (about 700 bytes) deadlocks against a task reading it from a cold page cache, or against writeback of an mmap-dirtied folio. Both tasks sit in D state; the writer holds i_rwsem and ni_lock, so everything that touches the file next (rename and unlink in its directory, sync, the bdi flusher) piles up behind them. Folio locks are not tracked by lockdep, so it never reported this. The inversion is as old as the driver: before the iomap conversion, ntfs_readpage() also took ni_lock with the page locked to read resident data, and attr_make_nonresident() locked page 0 under ni_lock. Lock folio 0 before ni_lock in every path that can convert $DATA (ntfs_set_size(), fallocate's attr_force_nonresident() and attr_insert_range()) and hand it to attr_make_nonresident(). Callers hold i_rwsem, so is_resident() cannot change between the check and the conversion. ext4 converts inline data in the same order. Reproducer: create a file and grow it to 4 KiB in 64-byte steps (append, ftruncate or fallocate) while another thread does fadvise(DONTNEED) + pread on it, four such pairs at a time. On ntfs3 master this hangs within a minute; with this patch it runs to the end. Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Kamil Chojnowski --- fs/ntfs3/attrib.c | 38 +++++++++++++++++++++++++------------- fs/ntfs3/attrlist.c | 4 ++-- fs/ntfs3/file.c | 21 ++++++++++++++++++--- fs/ntfs3/frecord.c | 4 ++-- fs/ntfs3/fsntfs.c | 2 +- fs/ntfs3/inode.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- fs/ntfs3/ntfs_fs.h | 13 ++++++++----- 7 files changed, 100 insertions(+), 29 deletions(-) diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c index 805acc3ed869..080edebe529e 100644 --- a/fs/ntfs3/attrib.c +++ b/fs/ntfs3/attrib.c @@ -258,11 +258,17 @@ out: * * If page is not NULL - it is already contains resident data * and locked (called from ni_write_frame()). + * + * If folio is not NULL - it is folio 0 of $DATA, locked by the caller + * before ni_lock (see ntfs_lock_folio0()). The resident data is copied + * into it. Without it, folio 0 is locked here, under ni_lock and run_lock, + * which deadlocks against read_folio/writeback: they hold the folio lock + * and then take run_lock/ni_lock in ntfs_iomap_begin. */ int attr_make_nonresident(struct ntfs_inode *ni, struct ATTRIB *attr, struct ATTR_LIST_ENTRY *le, struct mft_inode *mi, struct runs_tree *run, struct ATTRIB **ins_attr, - struct page *page) + struct page *page, struct folio *folio) { struct ntfs_sb_info *sbi; struct ATTRIB *attr_s; @@ -323,10 +329,15 @@ int attr_make_nonresident(struct ntfs_inode *ni, struct ATTRIB *attr, err = ntfs_sb_write_run(sbi, run, 0, data, rsize, 0); if (err) goto out2; + } else if (folio) { + folio_fill_tail(folio, 0, data, rsize); + folio_mark_uptodate(folio); + folio_mark_dirty(folio); } else if (!page) { struct address_space *mapping = ni->vfs_inode.i_mapping; - struct folio *folio; + WARN_ONCE(1, "ntfs3: r=%llx: folio 0 locked under ni_lock", + (u64)ni->mi.rno); folio = __filemap_get_folio( mapping, 0, FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mapping_gfp_mask(mapping)); @@ -390,7 +401,7 @@ out: static int attr_set_size_res(struct ntfs_inode *ni, struct ATTRIB *attr, struct ATTR_LIST_ENTRY *le, struct mft_inode *mi, u64 new_size, struct runs_tree *run, - struct ATTRIB **ins_attr) + struct ATTRIB **ins_attr, struct folio *folio) { struct ntfs_sb_info *sbi = mi->sbi; struct MFT_REC *rec = mi->mrec; @@ -405,7 +416,7 @@ static int attr_set_size_res(struct ntfs_inode *ni, struct ATTRIB *attr, if (new_size > sbi->record_size || (dsize > 0 && used + dsize > sbi->max_bytes_per_attr)) { return attr_make_nonresident(ni, attr, le, mi, run, ins_attr, - NULL); + NULL, folio); } if (dsize < 0) { @@ -440,7 +451,7 @@ static int attr_set_size_res(struct ntfs_inode *ni, struct ATTRIB *attr, int attr_set_size_ex(struct ntfs_inode *ni, enum ATTR_TYPE type, const __le16 *name, u8 name_len, struct runs_tree *run, u64 new_size, const u64 *new_valid, bool keep_prealloc, - struct ATTRIB **ret, bool no_da) + struct ATTRIB **ret, bool no_da, struct folio *folio) { int err = 0; struct ntfs_inode *nb = ni->base; @@ -481,7 +492,7 @@ again: if (!attr_b->non_res) { err = attr_set_size_res(ni, attr_b, le_b, mi_b, new_size, run, - &attr_b); + &attr_b, folio); if (err) return err; @@ -2134,7 +2145,7 @@ int attr_collapse_range(struct ntfs_inode *ni, u64 vbo, u64 bytes) err = attr_set_size_ex(ni, ATTR_DATA, ni->file.ads.name, ni->file.ads.len, run, i_size - bytes, NULL, false, - NULL, true); + NULL, true, NULL); goto out; } @@ -2580,7 +2591,8 @@ undo_punch: * attr_insert_range - Insert range (hole) in file. * Not for normal files. */ -int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes) +int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes, + struct folio *folio) { int err = 0; struct runs_tree *run = &ni->file.run; @@ -2645,9 +2657,9 @@ int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes) down_write(&ni->file.run_lock); if (!attr_b->non_res) { - err = attr_set_size(ni, ATTR_DATA, ni->file.ads.name, - ni->file.ads.len, run, data_size + bytes, - NULL, false); + err = attr_set_size_ex(ni, ATTR_DATA, ni->file.ads.name, + ni->file.ads.len, run, data_size + bytes, + NULL, false, NULL, false, folio); le_b = NULL; attr_b = ni_find_attr(ni, NULL, &le_b, ATTR_DATA, @@ -2839,7 +2851,7 @@ undo_insert_range: * * Convert default data attribute into non resident form. */ -int attr_force_nonresident(struct ntfs_inode *ni) +int attr_force_nonresident(struct ntfs_inode *ni, struct folio *folio) { int err; struct ATTRIB *attr; @@ -2860,7 +2872,7 @@ int attr_force_nonresident(struct ntfs_inode *ni) down_write(&ni->file.run_lock); err = attr_make_nonresident(ni, attr, le, mi, &ni->file.run, &attr, - NULL); + NULL, folio); up_write(&ni->file.run_lock); return err; diff --git a/fs/ntfs3/attrlist.c b/fs/ntfs3/attrlist.c index 8710560cf294..38be7af82420 100644 --- a/fs/ntfs3/attrlist.c +++ b/fs/ntfs3/attrlist.c @@ -352,7 +352,7 @@ int al_add_le(struct ntfs_inode *ni, enum ATTR_TYPE type, const __le16 *name, memcpy(le->name, name, sizeof(short) * name_len); err = attr_set_size_ex(ni, ATTR_LIST, NULL, 0, &al->run, new_size, - &new_size, true, &attr, false); + &new_size, true, &attr, false, NULL); if (err) { /* Undo memmove above. */ memmove(le, Add2Ptr(le, sz), old_size - off); @@ -416,7 +416,7 @@ int al_update(struct ntfs_inode *ni, int sync) * Attribute list decreased here. */ err = attr_set_size_ex(ni, ATTR_LIST, NULL, 0, &al->run, al->size, NULL, - false, &attr, false); + false, &attr, false, NULL); if (err) goto out; diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c index c11da747270f..384e61a33a84 100644 --- a/fs/ntfs3/file.c +++ b/fs/ntfs3/file.c @@ -606,6 +606,8 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len) if (err) goto out; } else if (mode & FALLOC_FL_INSERT_RANGE) { + struct folio *folio; + /* Check new size. */ err = inode_newsize_ok(inode, new_size); if (err) @@ -618,9 +620,15 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len) goto out; truncate_pagecache(inode, vbo_down); + folio = ntfs_lock_folio0(inode); + if (IS_ERR(folio)) { + err = PTR_ERR(folio); + goto out; + } ni_lock(ni); - err = attr_insert_range(ni, vbo, len); + err = attr_insert_range(ni, vbo, len, folio); ni_unlock(ni); + ntfs_unlock_folio0(folio); if (err) goto out; } else { @@ -629,9 +637,16 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len) /* Be sure file is non resident. */ if (is_resident(ni)) { + struct folio *folio = ntfs_lock_folio0(inode); + + if (IS_ERR(folio)) { + err = PTR_ERR(folio); + goto out; + } ni_lock(ni); - err = attr_force_nonresident(ni); + err = attr_force_nonresident(ni, folio); ni_unlock(ni); + ntfs_unlock_folio0(folio); if (err) goto out; } @@ -1450,7 +1465,7 @@ static int ntfs_file_release(struct inode *inode, struct file *file) err = attr_set_size_ex(ni, ATTR_DATA, ni->file.ads.name, ni->file.ads.len, &ni->file.run, inode->i_size, &ni->i_valid, false, NULL, - true); + true, NULL); up_write(&ni->file.run_lock); ni_unlock(ni); diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c index 16ac847de48d..3d10b1519c86 100644 --- a/fs/ntfs3/frecord.c +++ b/fs/ntfs3/frecord.c @@ -2573,7 +2573,7 @@ int ni_write_frame(struct ntfs_inode *ni, struct page **pages, if (!attr->non_res) { down_write(&ni->file.run_lock); err = attr_make_nonresident(ni, attr, le, mi, &ni->file.run, - &attr, pages[0]); + &attr, pages[0], NULL); up_write(&ni->file.run_lock); if (err) goto out; @@ -3373,7 +3373,7 @@ int ni_allocate_da_blocks_locked(struct ntfs_inode *ni) err = attr_set_size_ex(ni, ATTR_DATA, ni->file.ads.name, ni->file.ads.len, &ni->file.run, ni->vfs_inode.i_size, &ni->i_valid, - false, NULL, true); + false, NULL, true, NULL); } return err; diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c index b55311ccd229..6d8cddb148cc 100644 --- a/fs/ntfs3/fsntfs.c +++ b/fs/ntfs3/fsntfs.c @@ -539,7 +539,7 @@ static int ntfs_extend_mft(struct ntfs_sb_info *sbi) /* Step 1: Resize $MFT::DATA. */ down_write(&ni->file.run_lock); err = attr_set_size_ex(ni, ATTR_DATA, NULL, 0, &ni->file.run, - new_mft_bytes, NULL, false, &attr, false); + new_mft_bytes, NULL, false, &attr, false, NULL); if (err) { up_write(&ni->file.run_lock); diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index 8adbdf1a7d11..913ebd2a2196 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -887,11 +887,43 @@ static void ntfs_readahead(struct readahead_control *rac) iomap_readahead(&ntfs_iomap_ops, &ctx, NULL); } +/* + * ntfs_lock_folio0 - Lock folio 0 before converting resident $DATA. + * + * Moving resident $DATA out of the MFT record copies it into folio 0. + * read_folio and writeback hold the folio lock when they enter + * ntfs_iomap_begin and take run_lock/ni_lock, so the folio must be locked + * before ni_lock, never under it. Callers hold i_rwsem, which keeps the + * file from being converted under them, so is_resident() is stable here. + * + * Returns NULL if the file is not resident. + */ +struct folio *ntfs_lock_folio0(struct inode *inode) +{ + struct address_space *mapping = inode->i_mapping; + + if (!is_resident(ntfs_i(inode))) + return NULL; + + return __filemap_get_folio(mapping, 0, + FGP_LOCK | FGP_ACCESSED | FGP_CREAT, + mapping_gfp_mask(mapping)); +} + +void ntfs_unlock_folio0(struct folio *folio) +{ + if (folio) { + folio_unlock(folio); + folio_put(folio); + } +} + int ntfs_set_size(struct inode *inode, u64 new_size) { struct super_block *sb = inode->i_sb; struct ntfs_sb_info *sbi = sb->s_fs_info; struct ntfs_inode *ni = ntfs_i(inode); + struct folio *folio = NULL; int err; /* Check for maximum file size. */ @@ -906,17 +938,26 @@ int ntfs_set_size(struct inode *inode, u64 new_size) /* Mark rw ntfs as dirty. It will be cleared at umount. */ ntfs_set_state(sbi, NTFS_DIRTY_DIRTY); + /* Only growing can push resident $DATA out of the record. */ + if (new_size) { + folio = ntfs_lock_folio0(inode); + if (IS_ERR(folio)) + return PTR_ERR(folio); + } + ni_lock(ni); down_write(&ni->file.run_lock); if (new_size < ni->i_valid) ni->i_valid = new_size; - /* last 'true' means keep preallocated. */ - err = attr_set_size(ni, ATTR_DATA, ni->file.ads.name, ni->file.ads.len, - &ni->file.run, new_size, &ni->i_valid, true); + /* 'true' means keep preallocated. */ + err = attr_set_size_ex(ni, ATTR_DATA, ni->file.ads.name, + ni->file.ads.len, &ni->file.run, new_size, + &ni->i_valid, true, NULL, false, folio); up_write(&ni->file.run_lock); ni_unlock(ni); + ntfs_unlock_folio0(folio); return err; } diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h index 90ccfde951dc..6846c9b9ef46 100644 --- a/fs/ntfs3/ntfs_fs.h +++ b/fs/ntfs3/ntfs_fs.h @@ -466,18 +466,18 @@ int attr_allocate_clusters(struct ntfs_sb_info *sbi, struct runs_tree *run, int attr_make_nonresident(struct ntfs_inode *ni, struct ATTRIB *attr, struct ATTR_LIST_ENTRY *le, struct mft_inode *mi, struct runs_tree *run, struct ATTRIB **ins_attr, - struct page *page); + struct page *page, struct folio *folio); int attr_set_size_ex(struct ntfs_inode *ni, enum ATTR_TYPE type, const __le16 *name, u8 name_len, struct runs_tree *run, u64 new_size, const u64 *new_valid, bool keep_prealloc, - struct ATTRIB **ret, bool no_da); + struct ATTRIB **ret, bool no_da, struct folio *folio); static inline int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type, const __le16 *name, u8 name_len, struct runs_tree *run, u64 new_size, const u64 *new_valid, bool keep_prealloc) { return attr_set_size_ex(ni, type, name, name_len, run, new_size, - new_valid, keep_prealloc, NULL, false); + new_valid, keep_prealloc, NULL, false, NULL); } int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn, CLST *len, bool *new, bool zero, void **res, @@ -501,9 +501,10 @@ int attr_is_frame_compressed(struct ntfs_inode *ni, struct ATTRIB *attr, int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, u64 new_valid); int attr_collapse_range(struct ntfs_inode *ni, u64 vbo, u64 bytes); -int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes); +int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes, + struct folio *folio); int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size); -int attr_force_nonresident(struct ntfs_inode *ni); +int attr_force_nonresident(struct ntfs_inode *ni, struct folio *folio); /* Functions from attrlist.c */ void al_destroy(struct ntfs_inode *ni); @@ -801,6 +802,8 @@ static inline struct inode *ntfs_iget5(struct super_block *sb, return ntfs_iget5_flags(sb, ref, name, 0); } int ntfs_set_size(struct inode *inode, u64 new_size); +struct folio *ntfs_lock_folio0(struct inode *inode); +void ntfs_unlock_folio0(struct folio *folio); int ntfs3_write_inode(struct inode *inode, struct writeback_control *wbc); int ntfs_sync_inode(struct inode *inode); int inode_read_data(struct inode *inode, void *data, size_t bytes); -- 2.56.0