Skip to content

Conversation

@problame
Copy link
Contributor

@problame problame commented Nov 7, 2021

This PR is a large patch series, roughly ordered as followed:

  • a bunch of preliminary refactorings of the ZIL and related code
  • ZIL Kinds,
  • and ZIL-PMEM.

All of this is based on my master's thesis.

I'll give a presentation on all of this at the OpenZFS 2021 Developer Summit this Monday.

Will update the PR with more details afterward.

Signed-off-by: Christian Schwarz <me@cschwarz.com>
Was leaking memory, detected by ASAN.

Signed-off-by: Christian Schwarz <me@cschwarz.com>
This allows more subsystem's *_init() functions to use zfs_dbgmsg
and/or dprintf.

Signed-off-by: Christian Schwarz <me@cschwarz.com>
Signed-off-by: Christian Schwarz <me@cschwarz.com>
Signed-off-by: Christian Schwarz <me@cschwarz.com>
Signed-off-by: Christian Schwarz <me@cschwarz.com>
…m in zil_write_issue

Signed-off-by: Christian Schwarz <me@cschwarz.com>
Signed-off-by: Christian Schwarz <me@cschwarz.com>
…t request

Signed-off-by: Christian Schwarz <me@cschwarz.com>
We are careful to not change anything, particularly not
- the function signatures
- the function bodies
- the relative order of the LWB-specific functions

This makes the diff easy to review:
1. Open the delete-heavy diff for zil.c, and the newly added zil_lwb.c
   side-by-side.
2. Scroll to the top for both the zil.c diff and zil_lwb.c, then go
   through the changes to zil.c top to bottom, performing the following
   check for each function F deleted from zil.c:
   ```
   Search for function's code in zil_lwb.c.
   It should
   - be present verbatim (not even whitespace changes)
   - in the same order relative to the other functions deleted from zil.c
   ```
3. Ensure that no extraneous code was added to zil_lwb.c, i.e., we
   reached the end of zil_lwb.c in step (2).
…lic zil_* funcs

Later, we'll convert the stubs to do dynamic dispatch via a vtable (zilog_vtable_t).
…og_t

In a later commit, we'll introduce the generic struct zilog and update the typedef again.
…rt handler function

This can be used to implement tests that ensure that certain conditions
are covered by an assertion.
WR_INDIRECT is full of subtle opportunities to leak log blocks.
Make it optional for a ZIL implementation to support it.
These casts are currently correct because fletcher_4_ctx_t is a union
type with zio_cksum_t as the fletcher_4_ctx_t::scaler member.

However, they
a) make the code unnecessarily hard to follow and
b) are brittle with regards to changes to the fletcher_4_ctx_t type
   (See next patch in this series)

Signed-off-by: Christian Schwarz <me@cschwarz.com>
zfs_kfpu_ctx_t enables nested kfpu usage by calling
kfpu_begin() on first zfs_kfpu_enter() and
kfpu_end() on last kfpu_exit().

Signed-off-by: Christian Schwarz <me@cschwarz.com>
This change adds fletcher_4_native_kfpu_ctx() which allows using
fletcher_4_native() in an existing zfs_kfpu_ctx_t.

Signed-off-by: Christian Schwarz <me@cschwarz.com>
With the avx512 pmem ops, this cuts down latency because we only need to
XSAVE once per entry instead of thrice for header, body bulk, body tail.
@liyimeng
Copy link

liyimeng commented Feb 8, 2022

@problame This indeed a great work. But is is a huge PR, do you see any chance for it to get merged in near future?

@problame
Copy link
Contributor Author

problame commented Feb 9, 2022

I think some of the preliminary ones could be merged independent of the ZIL-PMEM feature.
I think some ZIL redesign work is inevitable in the next few years if we want to keep up with storage performance.
Whether it's going to be the design fromn
my thesis, a variation thereof, or something entirely different is an open question.
I think we didn't have that discussion yet, in the larger community.

@liyimeng
Copy link

I love zfs because if keep data safe. I am side with you on the performance. Giving the storage technology keep evolving in recent years, especially nvme and pmem, make zfs performance metric look so out-dated. The community need contributions like your one to keep up. Otherwise, it soon become a passed thing :(

Maybe you can break down your PR in small ones, make it get merged step by step? :) Since no one can review such a big change. I don't have competence in doing the work, but I'm happy to help with testing.

@problame
Copy link
Contributor Author

For future reference, all patches up to and including patch make zilog_t* opaque for dsl_{pool,scan}.h are mere refactorings that are a net improvement to the code base in my opinion, even without ZIL Kinds and ZIL PMEM.

@unkownerror1
Copy link

@problame Does this PR improve the performance of synchronous writes, for non-PMEM

@jumbi77
Copy link
Contributor

jumbi77 commented Nov 12, 2022

For future reference, all patches up to and including patch make zilog_t* opaque for dsl_{pool,scan}.h are mere refactorings that are a net improvement to the code base in my opinion, even without ZIL Kinds and ZIL PMEM.

That sounds already nice. Is it possible to split out the refactoring/improvement from ZIL Kinds/ZIL PMEM in separate PR? Maybe its easier to review and getting a higher chance to be upstreamed?

Thanks anyway for that impressive work.

@amotin amotin added Status: Work in Progress Not yet ready for general review Status: Stale No recent activity for issue labels Oct 29, 2024
@amotin
Copy link
Member

amotin commented Jan 9, 2025

While it was interesting project 3 years ago, since then Intel discontinued DCPMM, Linux DAX API is under GPL, ZIL got relocked moving memory copies out of the lock and making them parallel, and in general this PR got a significant conflicts with main. It makes this PR in its present form not applicable. It would be good it somebody could look though those commits to find anything still useful, but in general I am closing this PR in its present state as stale.

@amotin amotin closed this Jan 9, 2025
@robn
Copy link
Member

robn commented Jan 9, 2025

FWIW, I am still interested in some of the broad shapes here, in particular the "kinds" work. But I agree that they're unlikely to proceed in this form, so I'm cool with closing too. I will be coming back to it, but maybe not for a year or two.

@problame thank you for your work on this. I learned a ton from it early on, and it's great to have the research and the prototype work in the bank. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Stale No recent activity for issue Status: Work in Progress Not yet ready for general review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants