Re: [PATCH v2] ext4: use fsdata to track inline data write state and fix race

[Jan Kara]

Hi Aditya,

On Thu 02-07-26 18:15:21, Aditya Prakash Srivastava wrote:
> Thought a bit more about it. Treating fsdata as
> bitwise flags rather than mutually exclusive enums is a much
> cleaner and more robust design. It completely decouples the
> different write path states.
> 
> To implement this, I will define:
> 
> #define FALL_BACK_TO_NONDELALLOC 1
> #define CONVERT_INLINE_DATA 2
> #define EXT4_WRITE_DATA_INLINE 4
> 
> This ensures that the flags are fully independent and do not
> overlap.
> 
> Regarding unrolling the state on exit, there is a slight
> asymmetry in the VFS interface. ext4_write_begin() receives
> fsdata as a pointer-to-pointer (void **fsdata), while
> ext4_write_end() receives it by value (void *fsdata).
> 
> Because write_end() receives fsdata by value, modifying it in
> the write_end handlers has no effect on the caller's
> (generic_perform_write()) local stack variable. As a result, a
> stale EXT4_WRITE_DATA_INLINE bit would remain in the VFS across
> a retry.

Right, my bad.

> To handle this cleanly within the constraints of the VFS
> interface, I propose clearing the stale bit on entry to
> ext4_write_begin():
> 
> static int ext4_write_begin(..., void **fsdata)
> {
> ...
> 
> /* Clear any stale inline flag from a previous retry. */
> *fsdata = (void *)((unsigned long)*fsdata &
>   ~EXT4_WRITE_DATA_INLINE);
> 
> ...
> }
> 
> This ensures that:
> 
> any stale inline flag from a previous retry is cleared from
> the VFS stack variable before the next attempt;
> 
> the FALL_BACK_TO_NONDELALLOC bit remains preserved; and
> 
> if the retry prepares an inline write, ext4_write_begin()
> simply sets the bit again.
> 
> Please let me know whether this write_begin unrolling approach
> looks good to you, and I'll submit the v3 series shortly.

Yes, that should work and should be clean.

								Honza

-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR