Re: [PATCH v3 0/4] bcachefs: fiemap delalloc support and cleanup

Re: [PATCH v3 0/4] bcachefs: fiemap delalloc support and cleanup

[Kent Overstreet]

On Mon, Apr 08, 2024 at 10:48:42AM -0400, Brian Foster wrote:
> Hi all,
> 
> Here's v3 of the fiemap delalloc support patches for bcachefs. The main
> difference from v2 is that the pagecache seek calls are now using
> nonblocking mode to avoid deadlocks between fiemap and the write path.
> The write path locks in folio -> extent btree order while fiemap walks
> the extent btree and scans for folios in transaction context. Therefore,
> the latter must restart the iterating transaction in the event of folio
> trylock failure and restart the scan from where it left off.
> 
> The series is pushed to CI via my test branch, as usual:
> 
> https://evilpiepirate.org/~testdashboard/ci?branch=bfoster

So our testing is currently busted because of a bug in 9pfs - I need you
to rebase onto my 9p-revert branch so we can get test results we can
look at:
https://evilpiepirate.org/git/bcachefs.git/log/?h=9p-revert

Eric, is this getting fixed? If it's not, and _soon_, we need to send
this revert to Linus - then put it through more testing, more asserts,
whatever you need and send it again next cycle, _after_ you've figured
out what wwent wrong.

Patch series looks good, though - if there's nothing you think needs
special attention I'll go ahead and merge it.

I wonder if at some point we could clean up the indirect extent lookup,
that's common between this code and the read paths, but that's low
priority.

Re: [PATCH v3 0/4] bcachefs: fiemap delalloc support and cleanup

[Brian Foster]

On Mon, Apr 08, 2024 at 11:33:05PM -0400, Kent Overstreet wrote:
> On Mon, Apr 08, 2024 at 10:48:42AM -0400, Brian Foster wrote:
> > Hi all,
> > 
> > Here's v3 of the fiemap delalloc support patches for bcachefs. The main
> > difference from v2 is that the pagecache seek calls are now using
> > nonblocking mode to avoid deadlocks between fiemap and the write path.
> > The write path locks in folio -> extent btree order while fiemap walks
> > the extent btree and scans for folios in transaction context. Therefore,
> > the latter must restart the iterating transaction in the event of folio
> > trylock failure and restart the scan from where it left off.
> > 
> > The series is pushed to CI via my test branch, as usual:
> > 
> > https://evilpiepirate.org/~testdashboard/ci?branch=bfoster
> 
> So our testing is currently busted because of a bug in 9pfs - I need you
> to rebase onto my 9p-revert branch so we can get test results we can
> look at:
> https://evilpiepirate.org/git/bcachefs.git/log/?h=9p-revert
> 

Huh, Ok. I also noticed an issue recently related to some sysfs
directory structure changes. That required me to rebase on
bcachefs-testing, though it appears that has since been resolved in
master (and 9p-revert).

I just rebased my test branch on 9p-revert and pushed. What failures are
being caused by 9p, anyways?

> Eric, is this getting fixed? If it's not, and _soon_, we need to send
> this revert to Linus - then put it through more testing, more asserts,
> whatever you need and send it again next cycle, _after_ you've figured
> out what wwent wrong.
> 
> Patch series looks good, though - if there's nothing you think needs
> special attention I'll go ahead and merge it.
> 

Not really, mainly just making sure the new locking and transaction
restart parts looked sane.

> I wonder if at some point we could clean up the indirect extent lookup,
> that's common between this code and the read paths, but that's low
> priority.
> 

I'll make a note to read more into that area. Thanks.

Brian