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

[Aditya Prakash Srivastava]

Hi Jan,

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.

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.

Thanks,
Aditya