Re: [RFC PATCH v3 01/11] fs: Add counter to track inflight writes that need stable pages

[Ojaswin Mujoo]

On Wed, Aug 05, 2026 at 11:58:07AM +0530, Ojaswin Mujoo wrote:
> The current flag-style stable write implementation uses idempotent set
> and clear functions. This is okay because the users for the most part
> just want to set or clear it once based on factors like underlying
> device support.
> 
> However, this scheme doesn't play well when we have parallel users
> wanting to temporarily set and unset stable writes. For example, the
> upcoming RWF_WRITETHROUGH patches need stable writes to be enabled for
> the duration of the IO. The current scheme can lead to bugs like:
> 
> RWF_WRITETHROUGH write 1             RWF_WRITETHROUGH write 2
> 
> enable stable write		     enable stable write
> submit IO
> disable stable write   <---- WRONG
> 
>                                      submit IO
>                                      disable stable write
> 
> The 2nd write loses the stable write guarantee midway which is not
> correct. Fix this by introducing a new inflight_stable_write counter
> which can be used by parallel users safely.
> 
> Unfortunately, due to the way the current users are designed, we cannot
> directly migrate them to the counter approach hence for now we will
> have to keep both methods till all the users adapt to the counters.
> 
> Suggested-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> ---
>  include/linux/fs.h      |  1 +
>  include/linux/pagemap.h | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8e9bc9dda0cb..5f17aa0ed4c7 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -483,6 +483,7 @@ struct address_space {
>  	errseq_t		wb_err;
>  	spinlock_t		i_private_lock;
>  	struct rw_semaphore	i_mmap_rwsem;
> +	atomic_t		inflight_stable_writes_count;
>  } __attribute__((aligned(sizeof(long)))) __randomize_layout;
>  	/*
>  	 * On most architectures that alignment is already the case; but
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 2c3718d592d6..eb8b7e292478 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -306,7 +306,8 @@ static inline void mapping_clear_release_always(struct address_space *mapping)
>  
>  static inline bool mapping_stable_writes(const struct address_space *mapping)
>  {
> -	return test_bit(AS_STABLE_WRITES, &mapping->flags);
> +	return test_bit(AS_STABLE_WRITES, &mapping->flags) ||
> +	       atomic_read(&mapping->inflight_stable_writes_count) > 0;
>  }
>  
>  static inline void mapping_set_stable_writes(struct address_space *mapping)
> @@ -319,6 +320,17 @@ static inline void mapping_clear_stable_writes(struct address_space *mapping)
>  	clear_bit(AS_STABLE_WRITES, &mapping->flags);
>  }
>  
> +static inline void mapping_inc_inflight_stable_writes(struct address_space *mapping)
> +{
> +	atomic_inc(&mapping->inflight_stable_writes_count);
> +}
> +
> +static inline void mapping_dec_inflight_stable_writes(struct address_space *mapping)
> +{
> +	WARN_ON_ONCE(atomic_read(&mapping->inflight_stable_writes_count) == 0);
> +	atomic_dec_if_positive(&mapping->inflight_stable_writes_count);

>From Sashiko:

  If two threads concurrently call mapping_dec_inflight_stable_writes() when
  the counter is 1:

  CPU1
  atomic_read() // reads 1, bypasses warning
  CPU2
  atomic_read() // reads 1, bypasses warning
  CPU1
  atomic_dec_if_positive() // successfully decrements to 0
  CPU2
  atomic_dec_if_positive() // fails to decrement and returns -1

  Since CPU2 already bypassed the WARN_ON_ONCE via its separate read, the
  unbalanced decrement fails silently without being reported.

  Could this be made atomic by checking the return value directly, such as
  using WARN_ON_ONCE(atomic_dec_if_positive(...) < 0)?


Yes theres a race here. I'll change it to 

  WARN_ON_ONCE(atomic_dec_if_positive(...) < 0)?


Regards,
ojaswin

> +}
> +
>  static inline void mapping_set_inaccessible(struct address_space *mapping)
>  {
>  	/*
> -- 
> 2.55.0
>