linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul@sk.com>
To: David Hildenbrand <david@redhat.com>
Cc: akpm@linux-foundation.org, ziy@nvidia.com,
	matthew.brost@intel.com, joshua.hahnjy@gmail.com,
	rakie.kim@sk.com, gourry@gourry.net,
	ying.huang@linux.alibaba.com, apopple@nvidia.com,
	clameter@sgi.com, kravetz@us.ibm.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, max.byungchul.park@gmail.com,
	kernel_team@skhynix.com, harry.yoo@oracle.com,
	gwan-gyeong.mun@intel.com, yeoreum.yun@arm.com,
	syzkaller@googlegroups.com, ysk@kzalloc.com,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [RFC] mm/migrate: make sure folio_unlock() before folio_wait_writeback()
Date: Fri, 3 Oct 2025 11:31:16 +0900	[thread overview]
Message-ID: <20251003023116.GB29748@system.software.com> (raw)
In-Reply-To: <9a586b5b-c47f-45eb-83c8-1e86431fc83d@redhat.com>

On Thu, Oct 02, 2025 at 01:38:59PM +0200, David Hildenbrand wrote:
> > To simplify the scenario:
> > 
> 
> Just curious, where is the __folio_start_writeback() to complete the
> picture?

ext4_end_io_end() was running as a wq worker after the io completion.

DEPT report can tell that the following scenario happened with
__folio_start_writeback() called far earlier, at least, before
folio_test_writeback() was seen as true, but unfortunately DEPT doesn't
capture the exact location of __folio_start_writeback().

	Byungchul

> >     context X (wq worker)     context Y (process context)
> > 
> >                               migrate_pages_batch()
> >     ext4_end_io_end()           ...
> >       ...                       migrate_folio_unmap()
> >       ext4_get_inode_loc()        ...
> >         ...                       folio_lock() // hold the folio lock
> >         bdev_getblk()             ...
> >           ...                     folio_wait_writeback() // wait forever
> >           __find_get_block_slow()
> >             ...                           ...
> >             folio_lock() // wait forever
> >             folio_unlock()      migrate_folio_undo_src()
> >                                   ...
> >       ...                         folio_unlock() // never reachable
> >       ext4_finish_bio()
> >       ...
> >       folio_end_writeback() // never reachable
> > 
> 
> But aren't you implying that it should from this point on be disallowed
> to call folio_wait_writeback() with the folio lock held? That sounds ...
> a bit wrong.
> 
> Note that it is currently explicitly allowed: folio_wait_writeback()
> documents "If the folio is not locked, writeback may start again after
> writeback has finished.". So there is no way to prevent writeback from
> immediately starting again.
> 
> In particular, wouldn't we have to fixup other callsites to make this
> consistent and then VM_WARN_ON_ONCE() assert that in folio_wait_writeback()?
> 
> Of course, as we've never seen this deadlock before in practice, I do
> wonder if something else prevents it?
> 
> If it's a real issue, I wonder if a trylock on the writeback path could
> be an option.
> 
> --
> Cheers
> 
> David / dhildenb
> 


  parent reply	other threads:[~2025-10-03  2:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-02  8:16 Byungchul Park
2025-10-02 11:38 ` David Hildenbrand
2025-10-02 22:02   ` Hillf Danton
2025-10-03  0:48     ` Byungchul Park
2025-10-03  0:52       ` Byungchul Park
2025-10-07  6:32         ` Yunseong Kim
2025-10-07  7:04           ` David Hildenbrand
2025-10-07  7:53             ` Yeoreum Yun
2025-10-13  4:36             ` Byungchul Park
2025-10-13  8:08               ` David Hildenbrand
2025-10-03  1:02   ` Byungchul Park
2025-10-03  2:31   ` Byungchul Park [this message]
2025-10-03 14:04   ` Pedro Falcato
2025-10-02 11:42 ` Yeoreum Yun
2025-10-02 11:49   ` Yeoreum Yun
2025-10-03  2:08     ` Byungchul Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251003023116.GB29748@system.software.com \
    --to=byungchul@sk.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=clameter@sgi.com \
    --cc=david@redhat.com \
    --cc=gourry@gourry.net \
    --cc=gwan-gyeong.mun@intel.com \
    --cc=harry.yoo@oracle.com \
    --cc=joshua.hahnjy@gmail.com \
    --cc=kernel_team@skhynix.com \
    --cc=kravetz@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew.brost@intel.com \
    --cc=max.byungchul.park@gmail.com \
    --cc=rakie.kim@sk.com \
    --cc=syzkaller@googlegroups.com \
    --cc=willy@infradead.org \
    --cc=yeoreum.yun@arm.com \
    --cc=ying.huang@linux.alibaba.com \
    --cc=ysk@kzalloc.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox