[PATCH v4 1/2] ext4: use fsdata to track inline data write state and fix race

[PATCH v4 1/2] ext4: use fsdata to track inline data write state and fix race

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Instead of checking the live inode state (ext4_has_inline_data(inode)
and ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in the
write_end handlers, use the fsdata parameter of the address space
operations to explicitly pass down the state in which write_begin
prepared the write.

A concurrent thread (such as ext4_page_mkwrite()) can convert the
inline data to an extent between write_begin and write_end. If this
happens, the write_end handlers would previously miss the inline
write_end path and fall through to extent-based write_end logic.
However, since block buffers were never allocated in write_begin,
this resulted in NULL pointer dereferences or data loss because
folio_buffers(folio) was NULL.

Define EXT4_WRITE_DATA_INLINE (4) as a bit flag (Bit 2), treating
fsdata as bitwise flags rather than mutually exclusive enums to keep
states of the write path independent. Communicate this state via
fsdata:
1) ext4_write_begin() and ext4_da_write_begin() set the
   EXT4_WRITE_DATA_INLINE bit in *fsdata via bitwise OR when an inline
   write is successfully prepared.
2) On entry, ext4_write_begin() clears the EXT4_WRITE_DATA_INLINE bit
   to safely handle VFS retries (where generic_perform_write() bypasses
   the fsdata initialization on its retry jump).
3) The write_end handlers perform a bitwise AND to check if the
   EXT4_WRITE_DATA_INLINE bit is set and invoke the inline write_end
   helper accordingly.

Furthermore, during a buffered write, ext4_write_inline_data_end()
acquires the xattr lock after preparing the write. If a concurrent
page fault (ext4_page_mkwrite()) converts the inline data to an extent
after the write_end handlers check the state but before
ext4_write_inline_data_end() acquires the xattr write lock, the
subsequent check will trigger a kernel panic via
BUG_ON(!ext4_has_inline_data(inode)).

To keep git history working and bisectability clean, replace the
BUG_ON check in ext4_write_inline_data_end() with a graceful error-
handling retry path in this same commit. If the inline data is cleared
after locking the xattr, we safely release all resources (releasing
iloc.bh, unlocking/putting the folio, stopping the active journal
transaction handle) and return 0 (VFS retry) to let the generic write
path retry the operation safely.

Reported-by: syzbot+0c89d865531d053abb2d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0c89d865531d053abb2d
Fixes: 3fdcfb668fd7 ("ext4: add journalled write support for inline data")
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
Changes in v4:
- Resent the bug fix (Patch 1) and the cleanup (Patch 2) as a unified
  2-patch series to ensure clean upstream application and perfect
  logical separation, as suggested by Jan Kara.

Changes in v3:
- Changed EXT4_WRITE_DATA_INLINE to 4 (Bit 2) and treat fsdata as
  bitwise flags, allowing decoupling of independent states in the
  write path (e.g. standard fallback and inline writes), as suggested
  by Jan Kara.
- Clear the EXT4_WRITE_DATA_INLINE bit on entry to ext4_write_begin() to
  safely unroll state for any VFS write retries.
- Perform bitwise AND checks for EXT4_WRITE_DATA_INLINE and
  FALL_BACK_TO_NONDELALLOC flags in the write_end and da_write_end
  handlers.

Changes in v2:
- Folded the BUG_ON fix from the second patch into the first one to
  ensure bisectability across git history, as suggested by Jan Kara.
- Removed the pointless initialization `*fsdata = NULL` on entry to
  `ext4_write_begin()`.
- Removed the redundant check `if (fsdata)` in `ext4_write_begin()`.

 fs/ext4/ext4.h   |  1 +
 fs/ext4/inline.c | 14 +++++++++++++-
 fs/ext4/inode.c  | 24 +++++++++++++-----------
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..9e6f6467bdeb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3138,6 +3138,7 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
 void ext4_set_inode_mapping_order(struct inode *inode);
 #define FALL_BACK_TO_NONDELALLOC 1
 #define CONVERT_INLINE_DATA	 2
+#define EXT4_WRITE_DATA_INLINE	 4
 
 typedef enum {
 	EXT4_IGET_NORMAL =	0,
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 8045e4ff270c..cfd591dc1d9c 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -812,7 +812,19 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
 			goto out;
 		}
 		ext4_write_lock_xattr(inode, &no_expand);
-		BUG_ON(!ext4_has_inline_data(inode));
+		/*
+		 * We could have raced with ext4_page_mkwrite() converting
+		 * the inode and clearing the inline data flag, so we just
+		 * release resources and retry the whole write.
+		 */
+		if (unlikely(!ext4_has_inline_data(inode))) {
+			ext4_write_unlock_xattr(inode, &no_expand);
+			brelse(iloc.bh);
+			folio_unlock(folio);
+			folio_put(folio);
+			ext4_journal_stop(handle);
+			return 0;
+		}
 
 		/*
 		 * ei->i_inline_off may have changed since
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..4e1bf54e511d 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1302,6 +1302,8 @@ static int ext4_write_begin(const struct kiocb *iocb,
 	if (unlikely(ret))
 		return ret;
 
+	*fsdata = (void *)((unsigned long)*fsdata & ~EXT4_WRITE_DATA_INLINE);
+
 	trace_ext4_write_begin(inode, pos, len);
 	/*
 	 * Reserve one block more for addition to orphan list in case
@@ -1316,8 +1318,10 @@ static int ext4_write_begin(const struct kiocb *iocb,
 						    foliop);
 		if (ret < 0)
 			return ret;
-		if (ret == 1)
+		if (ret == 1) {
+			*fsdata = (void *)((unsigned long)*fsdata | EXT4_WRITE_DATA_INLINE);
 			return 0;
+		}
 	}
 
 	/*
@@ -1450,8 +1454,7 @@ static int ext4_write_end(const struct kiocb *iocb,
 
 	trace_ext4_write_end(inode, pos, len, copied);
 
-	if (ext4_has_inline_data(inode) &&
-	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
+	if ((unsigned long)fsdata & EXT4_WRITE_DATA_INLINE)
 		return ext4_write_inline_data_end(inode, pos, len, copied,
 						  folio);
 
@@ -1560,8 +1563,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
 
 	BUG_ON(!ext4_handle_valid(handle));
 
-	if (ext4_has_inline_data(inode) &&
-	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
+	if ((unsigned long)fsdata & EXT4_WRITE_DATA_INLINE)
 		return ext4_write_inline_data_end(inode, pos, len, copied,
 						  folio);
 
@@ -3161,8 +3163,10 @@ static int ext4_da_write_begin(const struct kiocb *iocb,
 						     foliop, fsdata, true);
 		if (ret < 0)
 			return ret;
-		if (ret == 1)
+		if (ret == 1) {
+			*fsdata = (void *)((unsigned long)*fsdata | EXT4_WRITE_DATA_INLINE);
 			return 0;
+		}
 	}
 
 retry:
@@ -3291,17 +3295,15 @@ static int ext4_da_write_end(const struct kiocb *iocb,
 			     struct folio *folio, void *fsdata)
 {
 	struct inode *inode = mapping->host;
-	int write_mode = (int)(unsigned long)fsdata;
+	unsigned long write_mode = (unsigned long)fsdata;
 
-	if (write_mode == FALL_BACK_TO_NONDELALLOC)
+	if (write_mode & FALL_BACK_TO_NONDELALLOC)
 		return ext4_write_end(iocb, mapping, pos,
 				      len, copied, folio, fsdata);
 
 	trace_ext4_da_write_end(inode, pos, len, copied);
 
-	if (write_mode != CONVERT_INLINE_DATA &&
-	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA) &&
-	    ext4_has_inline_data(inode))
+	if (write_mode & EXT4_WRITE_DATA_INLINE)
 		return ext4_write_inline_data_end(inode, pos, len, copied,
 						  folio);
 
-- 
2.47.3

[PATCH v4 2/2] ext4: cleanup unused CONVERT_INLINE_DATA flag

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

After implementing bitwise flags for tracking the inline data write
state in the address space fsdata parameter, the CONVERT_INLINE_DATA
state flag is left unused and can be removed.

Perform this clean-up by:
1) Deleting the CONVERT_INLINE_DATA definition from ext4.h.
2) Removing the void **fsdata argument from both the forward
   declaration and the definition of the internal helper
   ext4_da_convert_inline_data_to_extent().
3) Removing the void **fsdata argument from the declaration and
   definition of ext4_generic_write_inline_data() and updating
   the caller ext4_try_to_write_inline_data() and the internal
   re-alloc retry logic accordingly.
4) Updating ext4_da_write_begin() to call
   ext4_generic_write_inline_data() without the fsdata parameter.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
Changes in v4:
- Resent as Patch 2/2 of the v4 series, as suggested by Jan Kara.

Changes in v3:
- Initial version of the cleanup patch.

 fs/ext4/ext4.h   |  5 ++---
 fs/ext4/inline.c | 13 +++++--------
 fs/ext4/inode.c  |  2 +-
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9e6f6467bdeb..c2e4262406a7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3137,8 +3137,7 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
 				struct buffer_head *bh);
 void ext4_set_inode_mapping_order(struct inode *inode);
 #define FALL_BACK_TO_NONDELALLOC 1
-#define CONVERT_INLINE_DATA	 2
-#define EXT4_WRITE_DATA_INLINE	 4
+#define EXT4_WRITE_DATA_INLINE	 2
 
 typedef enum {
 	EXT4_IGET_NORMAL =	0,
@@ -3749,7 +3748,7 @@ extern int ext4_generic_write_inline_data(struct address_space *mapping,
 					  struct inode *inode,
 					  loff_t pos, unsigned len,
 					  struct folio **foliop,
-					  void **fsdata, bool da);
+					  bool da);
 extern int ext4_try_add_inline_entry(handle_t *handle,
 				     struct ext4_filename *fname,
 				     struct inode *dir, struct inode *inode);
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index cfd591dc1d9c..0cb2dc195d23 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -22,8 +22,7 @@
 
 
 static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
-						 struct inode *inode,
-						 void **fsdata);
+						 struct inode *inode);
 
 static int ext4_get_inline_size(struct inode *inode)
 {
@@ -697,7 +696,7 @@ int ext4_generic_write_inline_data(struct address_space *mapping,
 					  struct inode *inode,
 					  loff_t pos, unsigned len,
 					  struct folio **foliop,
-					  void **fsdata, bool da)
+					  bool da)
 {
 	int ret;
 	handle_t *handle;
@@ -728,7 +727,7 @@ int ext4_generic_write_inline_data(struct address_space *mapping,
 			return ext4_convert_inline_data_to_extent(mapping, inode);
 		}
 
-		ret = ext4_da_convert_inline_data_to_extent(mapping, inode, fsdata);
+		ret = ext4_da_convert_inline_data_to_extent(mapping, inode);
 		if (ret == -ENOSPC &&
 		    ext4_should_retry_alloc(inode->i_sb, &retries))
 			goto retry_journal;
@@ -788,7 +787,7 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
 	if (pos + len > ext4_get_max_inline_size(inode))
 		return ext4_convert_inline_data_to_extent(mapping, inode);
 	return ext4_generic_write_inline_data(mapping, inode, pos, len,
-					      foliop, NULL, false);
+					      foliop, false);
 }
 
 int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
@@ -895,8 +894,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
  *    need to start the journal since the file's metadata isn't changed now.
  */
 static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
-						 struct inode *inode,
-						 void **fsdata)
+						 struct inode *inode)
 {
 	int ret = 0, inline_size;
 	struct folio *folio;
@@ -934,7 +932,6 @@ static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
 	folio_mark_dirty(folio);
 	folio_mark_uptodate(folio);
 	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
-	*fsdata = (void *)CONVERT_INLINE_DATA;
 
 out:
 	up_read(&EXT4_I(inode)->xattr_sem);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4e1bf54e511d..9a0fbaf73ce2 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3160,7 +3160,7 @@ static int ext4_da_write_begin(const struct kiocb *iocb,
 
 	if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
 		ret = ext4_generic_write_inline_data(mapping, inode, pos, len,
-						     foliop, fsdata, true);
+						     foliop, true);
 		if (ret < 0)
 			return ret;
 		if (ret == 1) {
-- 
2.47.3

Re: [PATCH v4 1/2] ext4: use fsdata to track inline data write state and fix race

[Jan Kara]

On Fri 03-07-26 04:54:12, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> 
> Instead of checking the live inode state (ext4_has_inline_data(inode)
> and ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in the
> write_end handlers, use the fsdata parameter of the address space
> operations to explicitly pass down the state in which write_begin
> prepared the write.
> 
> A concurrent thread (such as ext4_page_mkwrite()) can convert the
> inline data to an extent between write_begin and write_end. If this
> happens, the write_end handlers would previously miss the inline
> write_end path and fall through to extent-based write_end logic.
> However, since block buffers were never allocated in write_begin,
> this resulted in NULL pointer dereferences or data loss because
> folio_buffers(folio) was NULL.
> 
> Define EXT4_WRITE_DATA_INLINE (4) as a bit flag (Bit 2), treating
> fsdata as bitwise flags rather than mutually exclusive enums to keep
> states of the write path independent. Communicate this state via
> fsdata:
> 1) ext4_write_begin() and ext4_da_write_begin() set the
>    EXT4_WRITE_DATA_INLINE bit in *fsdata via bitwise OR when an inline
>    write is successfully prepared.
> 2) On entry, ext4_write_begin() clears the EXT4_WRITE_DATA_INLINE bit
>    to safely handle VFS retries (where generic_perform_write() bypasses
>    the fsdata initialization on its retry jump).
> 3) The write_end handlers perform a bitwise AND to check if the
>    EXT4_WRITE_DATA_INLINE bit is set and invoke the inline write_end
>    helper accordingly.
> 
> Furthermore, during a buffered write, ext4_write_inline_data_end()
> acquires the xattr lock after preparing the write. If a concurrent
> page fault (ext4_page_mkwrite()) converts the inline data to an extent
> after the write_end handlers check the state but before
> ext4_write_inline_data_end() acquires the xattr write lock, the
> subsequent check will trigger a kernel panic via
> BUG_ON(!ext4_has_inline_data(inode)).
> 
> To keep git history working and bisectability clean, replace the
> BUG_ON check in ext4_write_inline_data_end() with a graceful error-
> handling retry path in this same commit. If the inline data is cleared
> after locking the xattr, we safely release all resources (releasing
> iloc.bh, unlocking/putting the folio, stopping the active journal
> transaction handle) and return 0 (VFS retry) to let the generic write
> path retry the operation safely.
> 
> Reported-by: syzbot+0c89d865531d053abb2d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0c89d865531d053abb2d
> Fixes: 3fdcfb668fd7 ("ext4: add journalled write support for inline data")
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Looks good! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes in v4:
> - Resent the bug fix (Patch 1) and the cleanup (Patch 2) as a unified
>   2-patch series to ensure clean upstream application and perfect
>   logical separation, as suggested by Jan Kara.
> 
> Changes in v3:
> - Changed EXT4_WRITE_DATA_INLINE to 4 (Bit 2) and treat fsdata as
>   bitwise flags, allowing decoupling of independent states in the
>   write path (e.g. standard fallback and inline writes), as suggested
>   by Jan Kara.
> - Clear the EXT4_WRITE_DATA_INLINE bit on entry to ext4_write_begin() to
>   safely unroll state for any VFS write retries.
> - Perform bitwise AND checks for EXT4_WRITE_DATA_INLINE and
>   FALL_BACK_TO_NONDELALLOC flags in the write_end and da_write_end
>   handlers.
> 
> Changes in v2:
> - Folded the BUG_ON fix from the second patch into the first one to
>   ensure bisectability across git history, as suggested by Jan Kara.
> - Removed the pointless initialization `*fsdata = NULL` on entry to
>   `ext4_write_begin()`.
> - Removed the redundant check `if (fsdata)` in `ext4_write_begin()`.
> 
>  fs/ext4/ext4.h   |  1 +
>  fs/ext4/inline.c | 14 +++++++++++++-
>  fs/ext4/inode.c  | 24 +++++++++++++-----------
>  3 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b37c136ea3ab..9e6f6467bdeb 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3138,6 +3138,7 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
>  void ext4_set_inode_mapping_order(struct inode *inode);
>  #define FALL_BACK_TO_NONDELALLOC 1
>  #define CONVERT_INLINE_DATA	 2
> +#define EXT4_WRITE_DATA_INLINE	 4
>  
>  typedef enum {
>  	EXT4_IGET_NORMAL =	0,
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 8045e4ff270c..cfd591dc1d9c 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -812,7 +812,19 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
>  			goto out;
>  		}
>  		ext4_write_lock_xattr(inode, &no_expand);
> -		BUG_ON(!ext4_has_inline_data(inode));
> +		/*
> +		 * We could have raced with ext4_page_mkwrite() converting
> +		 * the inode and clearing the inline data flag, so we just
> +		 * release resources and retry the whole write.
> +		 */
> +		if (unlikely(!ext4_has_inline_data(inode))) {
> +			ext4_write_unlock_xattr(inode, &no_expand);
> +			brelse(iloc.bh);
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			ext4_journal_stop(handle);
> +			return 0;
> +		}
>  
>  		/*
>  		 * ei->i_inline_off may have changed since
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ce99807c5f5b..4e1bf54e511d 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1302,6 +1302,8 @@ static int ext4_write_begin(const struct kiocb *iocb,
>  	if (unlikely(ret))
>  		return ret;
>  
> +	*fsdata = (void *)((unsigned long)*fsdata & ~EXT4_WRITE_DATA_INLINE);
> +
>  	trace_ext4_write_begin(inode, pos, len);
>  	/*
>  	 * Reserve one block more for addition to orphan list in case
> @@ -1316,8 +1318,10 @@ static int ext4_write_begin(const struct kiocb *iocb,
>  						    foliop);
>  		if (ret < 0)
>  			return ret;
> -		if (ret == 1)
> +		if (ret == 1) {
> +			*fsdata = (void *)((unsigned long)*fsdata | EXT4_WRITE_DATA_INLINE);
>  			return 0;
> +		}
>  	}
>  
>  	/*
> @@ -1450,8 +1454,7 @@ static int ext4_write_end(const struct kiocb *iocb,
>  
>  	trace_ext4_write_end(inode, pos, len, copied);
>  
> -	if (ext4_has_inline_data(inode) &&
> -	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
> +	if ((unsigned long)fsdata & EXT4_WRITE_DATA_INLINE)
>  		return ext4_write_inline_data_end(inode, pos, len, copied,
>  						  folio);
>  
> @@ -1560,8 +1563,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
>  
>  	BUG_ON(!ext4_handle_valid(handle));
>  
> -	if (ext4_has_inline_data(inode) &&
> -	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
> +	if ((unsigned long)fsdata & EXT4_WRITE_DATA_INLINE)
>  		return ext4_write_inline_data_end(inode, pos, len, copied,
>  						  folio);
>  
> @@ -3161,8 +3163,10 @@ static int ext4_da_write_begin(const struct kiocb *iocb,
>  						     foliop, fsdata, true);
>  		if (ret < 0)
>  			return ret;
> -		if (ret == 1)
> +		if (ret == 1) {
> +			*fsdata = (void *)((unsigned long)*fsdata | EXT4_WRITE_DATA_INLINE);
>  			return 0;
> +		}
>  	}
>  
>  retry:
> @@ -3291,17 +3295,15 @@ static int ext4_da_write_end(const struct kiocb *iocb,
>  			     struct folio *folio, void *fsdata)
>  {
>  	struct inode *inode = mapping->host;
> -	int write_mode = (int)(unsigned long)fsdata;
> +	unsigned long write_mode = (unsigned long)fsdata;
>  
> -	if (write_mode == FALL_BACK_TO_NONDELALLOC)
> +	if (write_mode & FALL_BACK_TO_NONDELALLOC)
>  		return ext4_write_end(iocb, mapping, pos,
>  				      len, copied, folio, fsdata);
>  
>  	trace_ext4_da_write_end(inode, pos, len, copied);
>  
> -	if (write_mode != CONVERT_INLINE_DATA &&
> -	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA) &&
> -	    ext4_has_inline_data(inode))
> +	if (write_mode & EXT4_WRITE_DATA_INLINE)
>  		return ext4_write_inline_data_end(inode, pos, len, copied,
>  						  folio);
>  
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 2/2] ext4: cleanup unused CONVERT_INLINE_DATA flag

[Jan Kara]

On Fri 03-07-26 04:54:13, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> 
> After implementing bitwise flags for tracking the inline data write
> state in the address space fsdata parameter, the CONVERT_INLINE_DATA
> state flag is left unused and can be removed.
> 
> Perform this clean-up by:
> 1) Deleting the CONVERT_INLINE_DATA definition from ext4.h.
> 2) Removing the void **fsdata argument from both the forward
>    declaration and the definition of the internal helper
>    ext4_da_convert_inline_data_to_extent().
> 3) Removing the void **fsdata argument from the declaration and
>    definition of ext4_generic_write_inline_data() and updating
>    the caller ext4_try_to_write_inline_data() and the internal
>    re-alloc retry logic accordingly.
> 4) Updating ext4_da_write_begin() to call
>    ext4_generic_write_inline_data() without the fsdata parameter.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Looks good! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes in v4:
> - Resent as Patch 2/2 of the v4 series, as suggested by Jan Kara.
> 
> Changes in v3:
> - Initial version of the cleanup patch.
> 
>  fs/ext4/ext4.h   |  5 ++---
>  fs/ext4/inline.c | 13 +++++--------
>  fs/ext4/inode.c  |  2 +-
>  3 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9e6f6467bdeb..c2e4262406a7 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3137,8 +3137,7 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
>  				struct buffer_head *bh);
>  void ext4_set_inode_mapping_order(struct inode *inode);
>  #define FALL_BACK_TO_NONDELALLOC 1
> -#define CONVERT_INLINE_DATA	 2
> -#define EXT4_WRITE_DATA_INLINE	 4
> +#define EXT4_WRITE_DATA_INLINE	 2
>  
>  typedef enum {
>  	EXT4_IGET_NORMAL =	0,
> @@ -3749,7 +3748,7 @@ extern int ext4_generic_write_inline_data(struct address_space *mapping,
>  					  struct inode *inode,
>  					  loff_t pos, unsigned len,
>  					  struct folio **foliop,
> -					  void **fsdata, bool da);
> +					  bool da);
>  extern int ext4_try_add_inline_entry(handle_t *handle,
>  				     struct ext4_filename *fname,
>  				     struct inode *dir, struct inode *inode);
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index cfd591dc1d9c..0cb2dc195d23 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -22,8 +22,7 @@
>  
>  
>  static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
> -						 struct inode *inode,
> -						 void **fsdata);
> +						 struct inode *inode);
>  
>  static int ext4_get_inline_size(struct inode *inode)
>  {
> @@ -697,7 +696,7 @@ int ext4_generic_write_inline_data(struct address_space *mapping,
>  					  struct inode *inode,
>  					  loff_t pos, unsigned len,
>  					  struct folio **foliop,
> -					  void **fsdata, bool da)
> +					  bool da)
>  {
>  	int ret;
>  	handle_t *handle;
> @@ -728,7 +727,7 @@ int ext4_generic_write_inline_data(struct address_space *mapping,
>  			return ext4_convert_inline_data_to_extent(mapping, inode);
>  		}
>  
> -		ret = ext4_da_convert_inline_data_to_extent(mapping, inode, fsdata);
> +		ret = ext4_da_convert_inline_data_to_extent(mapping, inode);
>  		if (ret == -ENOSPC &&
>  		    ext4_should_retry_alloc(inode->i_sb, &retries))
>  			goto retry_journal;
> @@ -788,7 +787,7 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
>  	if (pos + len > ext4_get_max_inline_size(inode))
>  		return ext4_convert_inline_data_to_extent(mapping, inode);
>  	return ext4_generic_write_inline_data(mapping, inode, pos, len,
> -					      foliop, NULL, false);
> +					      foliop, false);
>  }
>  
>  int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
> @@ -895,8 +894,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
>   *    need to start the journal since the file's metadata isn't changed now.
>   */
>  static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
> -						 struct inode *inode,
> -						 void **fsdata)
> +						 struct inode *inode)
>  {
>  	int ret = 0, inline_size;
>  	struct folio *folio;
> @@ -934,7 +932,6 @@ static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
>  	folio_mark_dirty(folio);
>  	folio_mark_uptodate(folio);
>  	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> -	*fsdata = (void *)CONVERT_INLINE_DATA;
>  
>  out:
>  	up_read(&EXT4_I(inode)->xattr_sem);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4e1bf54e511d..9a0fbaf73ce2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3160,7 +3160,7 @@ static int ext4_da_write_begin(const struct kiocb *iocb,
>  
>  	if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
>  		ret = ext4_generic_write_inline_data(mapping, inode, pos, len,
> -						     foliop, fsdata, true);
> +						     foliop, true);
>  		if (ret < 0)
>  			return ret;
>  		if (ret == 1) {
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR