Re: [PATCH v1] mm: annotate data race of f_ra.prev_pos

["Jiayuan Chen" <jiayuan.chen@linux.dev>]

February 26, 2026 at 21:21, "Jan Kara" <jack@suse.cz mailto:jack@suse.cz?to=%22Jan%20Kara%22%20%3Cjack%40suse.cz%3E > wrote:


> 
> On Thu 26-02-26 16:40:07, Jiayuan Chen wrote:
> 
> > 
> > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >  
> >  KCSAN reports a data race when concurrent readers access the same
> >  struct file:
> >  
> >  BUG: KCSAN: data-race in filemap_read / filemap_splice_read
> >  
> >  write to 0xffff88811a6f8228 of 8 bytes by task 10061 on cpu 0:
> >  filemap_splice_read+0x523/0x780 mm/filemap.c:3125
> >  ...
> >  
> >  write to 0xffff88811a6f8228 of 8 bytes by task 10066 on cpu 1:
> >  filemap_read+0x98d/0xa10 mm/filemap.c:2873
> >  ...
> >  
> >  Both filemap_read() and filemap_splice_read() update f_ra.prev_pos
> >  without synchronization. This is a benign race since prev_pos is only
> >  used as a hint for readahead heuristics in page_cache_sync_ra(), and a
> >  stale or torn value merely results in a suboptimal readahead decision,
> >  not a correctness issue.
> >  
> >  Use WRITE_ONCE/READ_ONCE to annotate all accesses to prev_pos across
> >  the tree for consistency and silence KCSAN.
> >  
> >  Reported-by: syzbot+6880f676b265dbd42d63@syzkaller.appspotmail.com
> >  Link: https://syzkaller.appspot.com/bug?extid=6880f676b265dbd42d63
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> > 
> Given this, I think it would be much less intrusive and also more
> explanatory to just mark prev_pos with __data_racy with appropriate reason
> you're mentioning in the changelog.


Thanks for the suggestion. I'm fine either way — __data_racy is indeed
cleaner and less intrusive for a purely heuristic field like this.

I'll wait a bit to see if Andrew or other mm folks have a preference
before resending. Happy to go with whichever approach they prefer.

>  Honza
> 
> > 
> > ---
> >  fs/ext4/dir.c | 2 +-
> >  fs/ntfs3/fsntfs.c | 2 +-
> >  include/trace/events/readahead.h | 2 +-
> >  mm/filemap.c | 6 +++---
> >  mm/readahead.c | 4 ++--
> >  mm/shmem.c | 2 +-
> >  6 files changed, 9 insertions(+), 9 deletions(-)
> >  
> >  diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> >  index 28b2a3deb954..1ddf7acce5ca 100644
> >  --- a/fs/ext4/dir.c
> >  +++ b/fs/ext4/dir.c
> >  @@ -200,7 +200,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
> >  sb->s_bdev->bd_mapping,
> >  &file->f_ra, file, index,
> >  1 << EXT4_SB(sb)->s_min_folio_order);
> >  - file->f_ra.prev_pos = (loff_t)index << PAGE_SHIFT;
> >  + WRITE_ONCE(file->f_ra.prev_pos, (loff_t)index << PAGE_SHIFT);
> >  bh = ext4_bread(NULL, inode, map.m_lblk, 0);
> >  if (IS_ERR(bh)) {
> >  err = PTR_ERR(bh);
> >  diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
> >  index 0df2aa81d884..d1232fc03c08 100644
> >  --- a/fs/ntfs3/fsntfs.c
> >  +++ b/fs/ntfs3/fsntfs.c
> >  @@ -1239,7 +1239,7 @@ int ntfs_read_run_nb_ra(struct ntfs_sb_info *sbi, const struct runs_tree *run,
> >  if (!ra_has_index(ra, index)) {
> >  page_cache_sync_readahead(mapping, ra, NULL,
> >  index, 1);
> >  - ra->prev_pos = (loff_t)index << PAGE_SHIFT;
> >  + WRITE_ONCE(ra->prev_pos, (loff_t)index << PAGE_SHIFT);
> >  }
> >  }
> >  
> >  diff --git a/include/trace/events/readahead.h b/include/trace/events/readahead.h
> >  index 0997ac5eceab..63d8df6c2983 100644
> >  --- a/include/trace/events/readahead.h
> >  +++ b/include/trace/events/readahead.h
> >  @@ -101,7 +101,7 @@ DECLARE_EVENT_CLASS(page_cache_ra_op,
> >  __entry->async_size = ra->async_size;
> >  __entry->ra_pages = ra->ra_pages;
> >  __entry->mmap_miss = ra->mmap_miss;
> >  - __entry->prev_pos = ra->prev_pos;
> >  + __entry->prev_pos = READ_ONCE(ra->prev_pos);
> >  __entry->req_count = req_count;
> >  ),
> >  
> >  diff --git a/mm/filemap.c b/mm/filemap.c
> >  index 63f256307fdd..d3e2d4b826b9 100644
> >  --- a/mm/filemap.c
> >  +++ b/mm/filemap.c
> >  @@ -2771,7 +2771,7 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter,
> >  int i, error = 0;
> >  bool writably_mapped;
> >  loff_t isize, end_offset;
> >  - loff_t last_pos = ra->prev_pos;
> >  + loff_t last_pos = READ_ONCE(ra->prev_pos);
> >  
> >  if (unlikely(iocb->ki_pos < 0))
> >  return -EINVAL;
> >  @@ -2870,7 +2870,7 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter,
> >  } while (iov_iter_count(iter) && iocb->ki_pos < isize && !error);
> >  
> >  file_accessed(filp);
> >  - ra->prev_pos = last_pos;
> >  + WRITE_ONCE(ra->prev_pos, last_pos);
> >  return already_read ? already_read : error;
> >  }
> >  EXPORT_SYMBOL_GPL(filemap_read);
> >  @@ -3122,7 +3122,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
> >  len -= n;
> >  total_spliced += n;
> >  *ppos += n;
> >  - in->f_ra.prev_pos = *ppos;
> >  + WRITE_ONCE(in->f_ra.prev_pos, *ppos);
> >  if (pipe_is_full(pipe))
> >  goto out;
> >  }
> >  diff --git a/mm/readahead.c b/mm/readahead.c
> >  index 7b05082c89ea..de49b35b0329 100644
> >  --- a/mm/readahead.c
> >  +++ b/mm/readahead.c
> >  @@ -142,7 +142,7 @@ void
> >  file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
> >  {
> >  ra->ra_pages = inode_to_bdi(mapping->host)->ra_pages;
> >  - ra->prev_pos = -1;
> >  + WRITE_ONCE(ra->prev_pos, -1);
> >  }
> >  EXPORT_SYMBOL_GPL(file_ra_state_init);
> >  
> >  @@ -584,7 +584,7 @@ void page_cache_sync_ra(struct readahead_control *ractl,
> >  }
> >  
> >  max_pages = ractl_max_pages(ractl, req_count);
> >  - prev_index = (unsigned long long)ra->prev_pos >> PAGE_SHIFT;
> >  + prev_index = (unsigned long long)READ_ONCE(ra->prev_pos) >> PAGE_SHIFT;
> >  /*
> >  * A start of file, oversized read, or sequential cache miss:
> >  * trivial case: (index - prev_index) == 1
> >  diff --git a/mm/shmem.c b/mm/shmem.c
> >  index 5e7dcf5bc5d3..03569199baf4 100644
> >  --- a/mm/shmem.c
> >  +++ b/mm/shmem.c
> >  @@ -3642,7 +3642,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
> >  len -= n;
> >  total_spliced += n;
> >  *ppos += n;
> >  - in->f_ra.prev_pos = *ppos;
> >  + WRITE_ONCE(in->f_ra.prev_pos, *ppos);
> >  if (pipe_is_full(pipe))
> >  break;
> >  
> >  -- 
> >  2.43.0
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>