zio_dva_throttle_done() should conditionally accept zinjected ZIOs

[sanjeevbagewadi]

#6383)

zinject enables ZIO_FLAG_IO_RETRY to ensure FMA events are generated. These ZIOs
could have ZIO_FLAG_IO_ALOCATING set. Hence, conditionally allow such ZIOs in
zio_dva_throttle_done() and do not fail the ASSERT() for ZIO_FLAG_IO_RETRY.

Signed-off-by: Sanjeev Bagewadi sanjeev.bagewadi@gmail.com

Description

If fault injection is enabled, the ZIO_FLAG_IO_RETRY could be set by
zio_handle_device_injection() to generate the FMA events and update
stats. Hence, ignore the flag and process such zios.

A better fix would be to add another flag in the zio_t to indicate
that the zio is failed because of a zinject rule. However, considering the
fact that we do this in debug bits, we could do with the crude check using the
global flag zio_injection_enabled which is set to 1 when zinject records are added.

Motivation and Context

If zinject faults are enabled, it fails the ZIOs and adds the ZIO_FLAG_IO_RETRY flag to ensure that FMA events are raised. And we could have ZIOs with ZIO_FLAG_IO_ALLOCATING enabled. However, for such IOs in the zio_done() when zio_dva_throttle_done() we hit the ASSERT() for ZIO_FLAG_IO_RETRY. We could apply the ASSERT() only if zinject is not enabled.

How Has This Been Tested?

Ran internal tests which do the following :

• zinject -a -d /dev/sdad -e io testpool -T all
• Generate IO to the testpool (by issuing some writes e.g.: cp -R /usr/lib /testpool/dir1)

Without the fix this would have resulted in a panic (one-debug builds). Ensured that with the fix no panic was seen.

NOTE : The cp command ensures that we have ALLOCATING ZIOs generated.

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)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.
• Change has been approved by a ZFS on Linux member.

[behlendorf]

Nice find, and thanks for the patch.

In addition to the inline comments there's a tiny typo in the commit message, s/donot/do not/. When you refresh this please go ahead and rebase it on master, and before you push it run make check locally and fix any style issues.

[behlendorf]

You're going to want to remove ZIO_FLAG_IO_RETRY from this line otherwise you'll still hit the ASSERT.

[sanjeevbagewadi]

Oops... missed that out.

[behlendorf]

Normally I'm all for detailed comments but this one really breaks up the flow of this function. Since the updated ASSERTs are largely self explanatory I think it'd be OK to simply adding this information to the commit message and remove it from here.

[sanjeevbagewadi]

Sure. Let me clean that up.

[sanjeevbagewadi]

Thank you for the review Brian ! Updated the fix. I am still ramping up on github usage so might end up making some mistakes :-)

[behlendorf]

@sanjeevbagewadi no problem. The updated version looks good. If you can squash these two commit and then git rebase them on the current master that should resolve the build and test failures. You'll need to force update the branch on Github which is fine.