Accommodate 0735ecb33 in trace_dmu.h #7096

akorn · 2018-01-30T10:35:36Z

0735ecb renamed tx_waited to tx_dirty_delayed, but trace_dmu.h wasn't updated.

This is a minimal fix needed to enable the code to build, avoiding the error

include/sys/trace_dmu.h:65:31: error: ‘dmu_tx_t {aka struct dmu_tx}’ has
no member named ‘tx_waited’
__entry->tx_waited = tx->tx_waited;
^

Signed-off-by: András Korn [email protected]

Description

Motivation and Context

How Has This Been Tested?

It builds successfully on latest Debian sid, with the stock Debian kernel. It didn't build before.

I don't know how I could test the functionality of the impacted code.

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.

akorn · 2018-01-30T10:36:58Z

I only have a very basic understanding of the code and have no idea whether this is the proper fix.

codecov · 2018-01-30T17:01:41Z

Codecov Report

Merging #7096 into master will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7096      +/-   ##
==========================================
+ Coverage   75.35%   75.45%   +0.09%     
==========================================
  Files         296      296              
  Lines       95715    95713       -2     
==========================================
+ Hits        72128    72216      +88     
+ Misses      23587    23497      -90

Flag	Coverage Δ
#kernel	`74.81% <ø> (-0.06%)`	⬇️
#user	`67.64% <ø> (+0.17%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d8a71b...c3c45ff. Read the comment docs.

dinatale2

Thank you for the PR! Just one comment below.

dinatale2 · 2018-01-30T20:39:12Z

include/sys/trace_dmu.h

@@ -62,7 +62,7 @@ DECLARE_EVENT_CLASS(zfs_delay_mintime_class,
 	    __entry->tx_lastsnap_txg		= tx->tx_lastsnap_txg;
 	    __entry->tx_lasttried_txg		= tx->tx_lasttried_txg;
 	    __entry->tx_anyobj			= tx->tx_anyobj;
-	    __entry->tx_waited			= tx->tx_waited;
+	    __entry->tx_waited			= tx->tx_dirty_delayed;


The printk below looks like it's attempting to print out the fields of the dmu_tx_t structure. I think you should rename tx_waited to be tx_dirty_delayed in the __entry structure to be consistent.

Yes, please update all the instances of tx_waited in this macro to tx_dirty_delayed.

behlendorf · 2018-01-30T22:51:44Z

include/sys/trace_dmu.h

@@ -62,7 +62,7 @@ DECLARE_EVENT_CLASS(zfs_delay_mintime_class,
 	    __entry->tx_lastsnap_txg		= tx->tx_lastsnap_txg;
 	    __entry->tx_lasttried_txg		= tx->tx_lasttried_txg;
 	    __entry->tx_anyobj			= tx->tx_anyobj;
-	    __entry->tx_waited			= tx->tx_waited;
+	    __entry->tx_waited			= tx->tx_dirty_delayed;


Yes, please update all the instances of tx_waited in this macro to tx_dirty_delayed.

Fabian-Gruenbichler · 2018-01-31T07:59:37Z

include/sys/trace_dmu.h

@@ -73,7 +73,7 @@ DECLARE_EVENT_CLASS(zfs_delay_mintime_class,
 	    "anyobj %d waited %d start %llu wait_dirty %d err %i "


waited should be replaced with dirty_delayed in the format string as well.

Fabian-Gruenbichler

one more nit, and IMHO both commits should be squashed into one ;)

This change was missed in 0735ecb. Signed-off-by: András Korn <[email protected]>

Fabian-Gruenbichler · 2018-01-31T11:24:22Z

@behlendorf (or whoever is responsible for the buildbot worker configs ;)) - it might make sense to configure one of the builders to have the appropriate kernel config/headers to compile with tracing support (see config/kernel-declare-event-class.m4), to prevent future changes that break it from slipping through like this one did?

behlendorf · 2018-01-31T23:59:01Z

@Fabian-Gruenbichler actually we did. Unfortunately, we accidentally broke that builder in such a way that it was reporting success even though this was broken. @dinatale2 has opened openzfs/zfs-buildbot#128 to resolve that infrastructure issue. We'll get the bots updates once we sort out this issue and verify there aren't other issues which snuck in. Sorry about that.

This change was missed in 0735ecb. Reviewed-by: Fabian Grünbichler <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Giuseppe Di Natale <[email protected]> Signed-off-by: András Korn <[email protected]> Closes openzfs#7096

This change was missed in 0735ecb. Reviewed-by: Fabian Grünbichler <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Giuseppe Di Natale <[email protected]> Signed-off-by: András Korn <[email protected]> Closes #7096

dinatale2 reviewed Jan 30, 2018

View reviewed changes

behlendorf requested changes Jan 30, 2018

View reviewed changes

Fabian-Gruenbichler reviewed Jan 31, 2018

View reviewed changes

Fabian-Gruenbichler suggested changes Jan 31, 2018

View reviewed changes

akorn force-pushed the master branch from 8a4046a to 43fa584 Compare January 31, 2018 08:21

Fabian-Gruenbichler approved these changes Jan 31, 2018

View reviewed changes

tx_waited -> tx_dirty_delayed in trace_dmu.h

c3c45ff

This change was missed in 0735ecb. Signed-off-by: András Korn <[email protected]>

akorn force-pushed the master branch from 43fa584 to c3c45ff Compare January 31, 2018 08:30

behlendorf approved these changes Feb 1, 2018

View reviewed changes

dinatale2 approved these changes Feb 1, 2018

View reviewed changes

behlendorf closed this in 4c46b99 Feb 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accommodate 0735ecb33 in trace_dmu.h #7096

Accommodate 0735ecb33 in trace_dmu.h #7096

akorn commented Jan 30, 2018 •

edited

Loading

akorn commented Jan 30, 2018

codecov bot commented Jan 30, 2018 •

edited

Loading

dinatale2 left a comment

dinatale2 Jan 30, 2018 •

edited

Loading

behlendorf Jan 30, 2018

behlendorf Jan 30, 2018

Fabian-Gruenbichler Jan 31, 2018

Fabian-Gruenbichler left a comment

Fabian-Gruenbichler commented Jan 31, 2018

behlendorf commented Jan 31, 2018

		@@ -73,7 +73,7 @@ DECLARE_EVENT_CLASS(zfs_delay_mintime_class,
		"anyobj %d waited %d start %llu wait_dirty %d err %i "

Accommodate 0735ecb33 in trace_dmu.h #7096

Accommodate 0735ecb33 in trace_dmu.h #7096

Conversation

akorn commented Jan 30, 2018 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

akorn commented Jan 30, 2018

codecov bot commented Jan 30, 2018 • edited Loading

Codecov Report

dinatale2 left a comment

Choose a reason for hiding this comment

dinatale2 Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

behlendorf Jan 30, 2018

Choose a reason for hiding this comment

behlendorf Jan 30, 2018

Choose a reason for hiding this comment

Fabian-Gruenbichler Jan 31, 2018

Choose a reason for hiding this comment

Fabian-Gruenbichler left a comment

Choose a reason for hiding this comment

Fabian-Gruenbichler commented Jan 31, 2018

behlendorf commented Jan 31, 2018

akorn commented Jan 30, 2018 •

edited

Loading

codecov bot commented Jan 30, 2018 •

edited

Loading

dinatale2 Jan 30, 2018 •

edited

Loading