Unlink #2573
Unlink #2573
Conversation
|
@prometheanfire @ryao could you please review and test this pull request. I believe it addresses all the issues which prevented the previous #2408 pull request from being merged. It's passed a few hundred hours of automated testing and I manually tested all the specific workloads I was concerned about. Thus far it's looking very good. Thanks. |
As originally implemented the mzap_upgrade() function will perform up to SPA_MAXBLOCKSIZE allocations using kmem_alloc(). These large allocations can potentially block indefinitely if contiguous memory is not available. Since this allocation is done under the zap->zap_rwlock it can appear as if there is a deadlock in zap_lockdir(). This is shown below. The optimal fix for this would be to rework mzap_upgrade() such that no large allocations are required. This could be done but it would result in us diverging further from the other implementations. Therefore I've opted against doing this unless it becomes absolutely necessary. Instead mzap_upgrade() has been updated to use zio_buf_alloc() which can reliably provide buffers of up to SPA_MAXBLOCKSIZE. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#2580
Handle all iputs in zfs_purgedir() and zfs_inode_destroy() asynchronously to prevent deadlocks. When the iputs are allowed to run synchronously in the destroy call path deadlocks between xattr directory inodes and their parent file inodes are possible. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#457
This reverts commit 7973e46 which brings the basic flow of the code back in line with the other ZFS implementations. This was possible due to the following related changes. e89260a Directory xattr znodes hold a reference on their parent 6f9548c Fix deadlock in zfs_zget() ca043ca Add zfs_iput_async() interface dd2a794 Avoid 128K kmem allocations in mzap_upgrade() Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#457 Issue openzfs#2058 Issue openzfs#2128 Issue openzfs#2240
|
@behlendorf This looks good to me. |
|
I should be able to review it today hopefully |
|
@prometheanfire That would be great. I know you managed to find some issues in the previous patch. |
|
I didn't see any load from doing a make clean in a kernel dir, mounting on reboot went fast as well, I'll leave it running for a day, hopefully I'll remember to do the power pull test tomorrow. |
|
@prometheanfire Sounds good. I'm going to merge this since it's thus survived all the testing, but please let us know if you encounter any unexpected issues. |
This stack of patches is designed to allow us to safely revert the async purgedir changes without reintroducing the original deadlock. The goals are the same as those of #2408. With this entire stack of patches applied I'm able to pass the full test suite. These changes are ready to receive some wider testing.