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

clone+mmap+write trips assert: VERIFY(arc_released(db->db_buf)) failed at dbuf.c:2150:dbuf_redirty() #15654

Closed
robn opened this issue Dec 7, 2023 · 4 comments · Fixed by #15656
Closed

clone+mmap+write trips assert: VERIFY(arc_released(db->db_buf)) failed at dbuf.c:2150:dbuf_redirty() #15654

robn opened this issue Dec 7, 2023 · 4 comments · Fixed by #15656
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@robn
Copy link
Member

robn commented Dec 7, 2023

System information

Type Version/Name
Distribution Name any
Kernel Version any
Architecture x86_64
OpenZFS Version 2.2.x+master (debug build)

Describe the problem you're observing

Test program: https://gist.github.com/robn/9804c60cd0275086d26893d73e7af35c

Sequence is:

  • clone a file (or region)
  • mmap it
  • write from the map into the file at the same spot
VERIFY(arc_released(db->db_buf)) failed
PANIC at dbuf.c:2150:dbuf_redirty()

Typical stack trace:

dbuf_redirty+0xad/0xb0 [zfs]
dbuf_dirty+0xe76/0x1310 [zfs]
dmu_write_uio_dnode+0x101/0x1b0 [zfs]
dmu_write_uio_dbuf+0x4a/0x70 [zfs]
zfs_write+0x500/0xc80 [zfs]
zpl_iter_write+0xe4/0x130 [zfs]
new_sync_write+0x119/0x1b0
vfs_write+0x1ce/0x260
__x64_sys_pwrite64+0x91/0xc0
do_syscall_64+0x30/0x40
entry_SYSCALL_64_after_hwframe+0x61/0xc6

I'm not sure if this is another case where the asserts haven't been updated to match the new state transitions, or if there's a real problem.

Also not sure the mmap is really involved, but maybe it affects refcounts.
@robn robn added the Type: Defect Incorrect behavior (e.g. crash, hang) label Dec 7, 2023
@robn
Copy link
Member Author

robn commented Dec 7, 2023 •
edited
Loading

@pjd @oromenahar @amotin FYI. Dunno if I'll have time to get to this soon.

@amotin
Copy link
Member

amotin commented Dec 8, 2023

I was unable to reproduce this panic on FreeBSD due to EFAULT returned by zfs_uio_fault_move() before that, causing understood and probably unrelated data corruption, that I reproduced with this test originally. But I've got a hint that setting sysctl debug.vn_io_fault_prefault=1 can workaround the EFAULT's, and after that I do reproduce the same panic. I'll continue digging.

@amotin amotin mentioned this issue Dec 9, 2023
Merged
13 tasks
@amotin
Copy link
Member

amotin commented Dec 9, 2023

I think I found the problem. See #15656 .

@kevans91
Copy link
Contributor

kevans91 commented Dec 9, 2023

Thanks for converting this into a smaller + better reproducer and official issue!

@amotin amotin mentioned this issue Dec 12, 2023
Merged
13 tasks
behlendorf pushed a commit that referenced this issue Dec 12, 2023
@amotin
dbuf: Set dr_data when unoverriding after clone
86e115e 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes #15654
Closes #15656
ixhamza pushed a commit to ixhamza/zfs that referenced this issue Dec 13, 2023
@amotin @ixhamza
dbuf: Set dr_data when unoverriding after clone
482b381 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
ixhamza pushed a commit to truenas/zfs that referenced this issue Dec 13, 2023
@amotin @ixhamza
dbuf: Set dr_data when unoverriding after clone
758c8da 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
ixhamza pushed a commit to truenas/zfs that referenced this issue Dec 13, 2023
@amotin @ixhamza
dbuf: Set dr_data when unoverriding after clone
ba69ae2 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
ixhamza pushed a commit to truenas/zfs that referenced this issue Dec 13, 2023
@amotin @ixhamza
dbuf: Set dr_data when unoverriding after clone
25ef79a 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
mmatuska pushed a commit to mmatuska/zfs that referenced this issue Dec 26, 2023
@amotin @mmatuska
dbuf: Set dr_data when unoverriding after clone
edc3764 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
mmatuska pushed a commit to mmatuska/zfs that referenced this issue Dec 27, 2023
@amotin @mmatuska
dbuf: Set dr_data when unoverriding after clone
ea6e518 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
mmatuska pushed a commit to mmatuska/zfs that referenced this issue Dec 27, 2023
@amotin @mmatuska
dbuf: Set dr_data when unoverriding after clone
ef34c71 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
behlendorf pushed a commit that referenced this issue Jan 9, 2024
@amotin @behlendorf
dbuf: Set dr_data when unoverriding after clone
9c40ae0 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes #15654
Closes #15656
lundman pushed a commit to openzfsonwindows/openzfs that referenced this issue Mar 13, 2024
@amotin @lundman
dbuf: Set dr_data when unoverriding after clone
791f6a1 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
lundman pushed a commit to openzfsonwindows/openzfs that referenced this issue Mar 13, 2024
@amotin @lundman
dbuf: Set dr_data when unoverriding after clone
32a8c18 
Block cloning normally creates dirty record without dr_data.  But if
the block is read after cloning, it is moved into DB_CACHED state and
receives the data buffer.  If after that we call dbuf_unoverride()
to convert the dirty record into normal write, we should give it the
data buffer from dbuf and release one.

Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored by: iXsystems, Inc.
Closes openzfs#15654
Closes openzfs#15656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

dbuf: Set dr_data when unoverriding after clone amotin/zfs
3 participants
@robn @kevans91 @amotin