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.

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

Reintroduce IO accounting on zvols on Linux 3.19+ #3746

Closed
wants to merge 1 commit into from

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Sep 7, 2015

e20cd6f caused us to
lose IO accounting on zvols. When I originally wrote that last year, the
symbols we needed to maintain IO accounting were GPL exported, but
torvalds/linux@394ffa5 provided
suitable symbols for restoring this functionality 4 months later. We
can call them to restore the IO accounting on Linux 3.19 and later as
well as any older kernels where that patch is backported.

Closes #3741

Signed-off-by: Richard Yao ryao@gentoo.org

@ryao ryao force-pushed the zvol_accounting branch 2 times, most recently from 3d51dff to cb94cf1 Compare September 7, 2015 16:10
@sempervictus
Copy link
Contributor

This seems to break AC's generation of the configure file:

checking for -Wno-unused-but-set-variable support... yes
checking for -Wno-bool-compare support... no
./configure: line 20741: syntax error near unexpected token `;'
./configure: line 20741: `  ; do'

That region ends up looking like this:

20721 
20722         if test $rc -ne 0; then :
20723 
20724         else
20725                 if test "x$enable_linux_builtin" != xyes; then
20726 
20727         grep -q -E '[[:space:]]
20728                 { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
20729 $as_echo "yes" >&6; }
20730 
20731 $as_echo "#define HAVE_GENERIC_IO_ACCT 1" >>confdefs.h
20732 
20733         [[:space:]]' \
20734                 $LINUX_OBJ/$LINUX_SYMBOLS 2>/dev/null
20735         rc=$?
20736         if test $rc -ne 0; then
20737                 export=0
20738                 for file in
20739                 { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
20740 $as_echo "no" >&6; }
20741         ; do
20742                         grep -q -E "EXPORT_SYMBOL.*(
20743                 { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
20744 $as_echo "yes" >&6; }
20745 
20746 $as_echo "#define HAVE_GENERIC_IO_ACCT 1" >>confdefs.h
20747 
20748         )" \
20749                                 "$LINUX/$file" 2>/dev/null
20750                         rc=$?
20751                         if test $rc -eq 0; then
20752                                 export=1
20753                                 break;
20754                         fi
20755                 done
20756                 if test $export -eq 0; then :
20757                         rc=1
20758                 else :
20759                         rc=0
20760                 fi
20761         else :

Seems autoconf is doing something strange here. Any thoughts on what this might be? The exact same issue is visible around line 29934, with the surrounding 40 lines appearing identical at first pass.

@ryao
Copy link
Contributor Author

ryao commented Sep 8, 2015

@sempervictus I had not looked at ZFS_LINUX_TRY_COMPILE_SYMBOL's arguments carefully enough to notice that it added a couple for explicitly specifying a symbol and file containing it and autoconf was happy with it. I had sent it to the buildbot to see if I had made any mistakes, so I had not known about this until I looked at the result. The mistake is fixed now.

@ryao
Copy link
Contributor Author

ryao commented Sep 8, 2015

Hmm... this doesn't seem to be working like I expected it to work. I will debug it when I find time.

@ryao ryao force-pushed the zvol_accounting branch 3 times, most recently from 978609a to 87cd58a Compare September 8, 2015 16:56
@ryao
Copy link
Contributor Author

ryao commented Sep 8, 2015

The latest version works properly.

@behlendorf
Copy link
Contributor

This patch looks good, however there are two minor style issue to address.

./include/linux/blkdev_compat.h: 347: #define followed by space instead of tab
./include/linux/blkdev_compat.h: 348: #define followed by space instead of tab
make: *** [cstyle] Error 1

openzfs/zfs@e20cd6f caused us to
lose IO accounting on zvols. When I originally wrote that last year, the
symbols we needed to maintain IO accounting were GPL exported, but
torvalds/linux@394ffa5 provided
suitable symbols for restoring this functionality 4 months later.  We
can call them to restore the IO accounting on Linux 3.19 and later as
well as any older kernels where that patch is backported.

Closes openzfs#3741

Signed-off-by: Richard Yao <ryao@gentoo.org>
@ryao
Copy link
Contributor Author

ryao commented Sep 8, 2015

@behlendorf The cstyle issue should be fixed.

@sempervictus
Copy link
Contributor

Seems to work now, slowly pushing out to other testbeds, if all goes well will hit prod tomorrow.

@behlendorf behlendorf added this to the 0.6.5 milestone Sep 9, 2015
@behlendorf
Copy link
Contributor

The updated version looks good to me. The test failures we all due to known issues or the VMs running out of free space in the case of the ztest seg faults. Merged as:

8198d18 Reintroduce IO accounting on zvols on Linux 3.19+

@behlendorf behlendorf closed this Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants