Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dbuf: Handle arcbuf assignment after block cloning #15653

Merged
merged 1 commit into from Dec 12, 2023
Merged

dbuf: Handle arcbuf assignment after block cloning #15653

merged 1 commit into from Dec 12, 2023

Conversation

amotin
Copy link
Member

@amotin amotin commented Dec 7, 2023

In some cases dbuf_assign_arcbuf() may be called on a block that was recently cloned. If it happened in current TXG we must undo the block cloning first, since the only one dirty record per TXG can't and shouldn't mean both cloning and overwrite same time.

I think this should be the fix to the problem discussed in #15139 .

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@amotin
dbuf: Handle arcbuf assignment after block cloning
35c4ef6 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
@amotin
Copy link
Member Author

amotin commented Dec 7, 2023

@oromenahar @robn What do you think about this instead of #15139?

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Dec 7, 2023
@oromenahar oromenahar mentioned this pull request Dec 9, 2023
Open
13 tasks
oromenahar
oromenahar approved these changes Dec 10, 2023
View reviewed changes
Copy link
Contributor

@oromenahar oromenahar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add your code to #15139 or add my test to this code here @amotin. See my last message in #15139 asked there as well in which direction you would like to add the code.
Would prefer your code in #15139 because everything is discussed there and having the code there make sense IMO. But nice work thanks.

behlendorf
behlendorf approved these changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. @amotin I think there's value in pulling in a version of test case as long as it only takes a few seconds, even if it's not 100% reproducible.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 11, 2023
@oromenahar
Copy link
Contributor

oromenahar commented Dec 11, 2023 •
edited

@behlendorf going o pull this code into my pr and clean it up. I think the test is worth it. Even if it needs a few seconds or one minute.

Will clean it this evening

@behlendorf behlendorf merged commit 86063d9 into openzfs:master Dec 12, 2023
25 of 26 checks passed
@amotin amotin deleted the assign_after_clone branch December 12, 2023 21:29
ixhamza pushed a commit to ixhamza/zfs that referenced this pull request Dec 13, 2023
@amotin @ixhamza
dbuf: Handle arcbuf assignment after block cloning
3870483 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
ixhamza pushed a commit to truenas/zfs that referenced this pull request Dec 13, 2023
@amotin @ixhamza
dbuf: Handle arcbuf assignment after block cloning
57f7291 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
ixhamza pushed a commit to truenas/zfs that referenced this pull request Dec 13, 2023
@amotin @ixhamza
dbuf: Handle arcbuf assignment after block cloning
f016481 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
mmatuska pushed a commit to mmatuska/zfs that referenced this pull request Dec 26, 2023
@amotin @mmatuska
dbuf: Handle arcbuf assignment after block cloning
32effd8 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
@mmatuska mmatuska mentioned this pull request Dec 26, 2023
Merged
13 tasks
mmatuska pushed a commit to mmatuska/zfs that referenced this pull request Dec 27, 2023
@amotin @mmatuska
dbuf: Handle arcbuf assignment after block cloning
0ad2e38 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
mmatuska pushed a commit to mmatuska/zfs that referenced this pull request Dec 27, 2023
@amotin @mmatuska
dbuf: Handle arcbuf assignment after block cloning
784c9cc 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
behlendorf pushed a commit that referenced this pull request Jan 9, 2024
@amotin @behlendorf
dbuf: Handle arcbuf assignment after block cloning
a701548 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes #15653
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@amotin @lundman
dbuf: Handle arcbuf assignment after block cloning
b5727d4 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@amotin @lundman
dbuf: Handle arcbuf assignment after block cloning
f9db3ab 
In some cases dbuf_assign_arcbuf() may be called on a block that
was recently cloned.  If it happened in current TXG we must undo
the block cloning first, since the only one dirty record per TXG
can't and shouldn't mean both cloning and overwrite same time.

Reviewed-by: Kay Pedersen <mail@mkwg.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15653
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@oromenahar oromenahar oromenahar approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@amotin @oromenahar @behlendorf