| Message ID | 20171129010937.GB31372@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Nov 28, 2017 at 05:09:37PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > If kmemleak is enabled, scan and report memory leaks after every test. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > check | 2 ++ > common/rc | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/check b/check > index b2d251a..469188e 100755 > --- a/check > +++ b/check > @@ -497,6 +497,7 @@ _check_filesystems() > fi > } > > +_init_kmemleak > _prepare_test_list > > if $OPTIONS_HAVE_SECTIONS; then > @@ -793,6 +794,7 @@ for section in $HOST_OPTIONS_SECTIONS; do > n_try=`expr $n_try + 1` > _check_filesystems > _check_dmesg || err=true > + _check_kmemleak || err=true > fi > > fi > diff --git a/common/rc b/common/rc > index cb83918..6dc77d5 100644 > --- a/common/rc > +++ b/common/rc > @@ -3339,6 +3339,57 @@ _check_dmesg() > fi > } > > +# set up kmemleak > +_init_kmemleak() > +{ > + local _kern_knob="/sys/kernel/debug/kmemleak" Use $DEBUGFS_MNT? e.g. $DEBUGFS_MNT/kmemleak > + > + if [ ! -w "${_kern_knob}" ]; then > + return 0 > + fi > + > + # Scan for all the memory leaks that have happened to date and > + # clear them so that we can pinpoint leaks to tests accurately. > + # Scan twice because the first write finishes before the scan > + # does.... > + echo "scan" > "${_kern_knob}" > + cat "${_kern_knob}" > /dev/null > + echo "scan" > "${_kern_knob}" > + cat "${_kern_knob}" > /dev/null > + echo "clear" > "${_kern_knob}" So this leaves the automatic background scan on, seems this is not a problem. But how about disabling automatic scan (echo "scan=off" > $DEBUGFS_MNT/kmemleak) and only depend on the manual scan in _check_kmemleak? > +} > + > +# check kmemleak log > +_check_kmemleak() > +{ > + local _kern_knob="/sys/kernel/debug/kmemleak" > + local _leak_file="${RESULT_DIR}/check_kmemleak" > + > + if [ ! -w "${_kern_knob}" ]; then > + return 0 > + fi > + > + # Tell the kernel to scan for memory leaks. Apparently the write > + # returns before the scan is complete, so do it twice in the hopes > + # that twice is enough to capture all the leaks. > + echo "scan" > "${_kern_knob}" > + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" > + echo "scan" > "${_kern_knob}" > + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" > + > + diff -u -N "${_leak_file}" "${_leak_file}.new" > $seqres.kmemleak Hmm, I think this reports false failure, e.g. let's say generic/001 leaks memory and generic/002 doesn't ./check generic/001 # this fails kmemleak check and populates check_kmemleak file ./check generic/002 # diff against the stale check_kmemleak file and fails the check too How about just check if there's anything reported from $DEBUGFS_MNT/kmemleak and clearing the kmemleak buffer after each test? e.g. echo "scan" > $DEBUGFS_MNT/kmemleak cat $DEBUGFS_MNT/kmemleak > $seqres.kmemleak echo "clear" > $DEBUGFS_MNT/kmemleak if [ -s $seqres.kmemleak ]; then _dump_err "_check_kmemleak: something found in kmemleak (see $seqres.kmemleak)" return 1 else rm -f $seqres.kmemleak return 0 fi Thanks, Eryu > + res=$? > + mv "${_leak_file}.new" "${_leak_file}" > + > + if [ $res -ne 0 ]; then > + _dump_err "_check_kmemleak: something found in kmemleak (see $seqres.kmemleak)" > + return 1 > + else > + rm -f $seqres.kmemleak > + return 0 > + fi > +} > + > # don't check dmesg log after test > _disable_dmesg_check() > { -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 04, 2017 at 12:55:13PM +0800, Eryu Guan wrote: > On Tue, Nov 28, 2017 at 05:09:37PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > If kmemleak is enabled, scan and report memory leaks after every test. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > check | 2 ++ > > common/rc | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 53 insertions(+) > > > > diff --git a/check b/check > > index b2d251a..469188e 100755 > > --- a/check > > +++ b/check > > @@ -497,6 +497,7 @@ _check_filesystems() > > fi > > } > > > > +_init_kmemleak > > _prepare_test_list > > > > if $OPTIONS_HAVE_SECTIONS; then > > @@ -793,6 +794,7 @@ for section in $HOST_OPTIONS_SECTIONS; do > > n_try=`expr $n_try + 1` > > _check_filesystems > > _check_dmesg || err=true > > + _check_kmemleak || err=true > > fi > > > > fi > > diff --git a/common/rc b/common/rc > > index cb83918..6dc77d5 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -3339,6 +3339,57 @@ _check_dmesg() > > fi > > } > > > > +# set up kmemleak > > +_init_kmemleak() > > +{ > > + local _kern_knob="/sys/kernel/debug/kmemleak" > > Use $DEBUGFS_MNT? e.g. $DEBUGFS_MNT/kmemleak Ok. > > + > > + if [ ! -w "${_kern_knob}" ]; then > > + return 0 > > + fi > > + > > + # Scan for all the memory leaks that have happened to date and > > + # clear them so that we can pinpoint leaks to tests accurately. > > + # Scan twice because the first write finishes before the scan > > + # does.... > > + echo "scan" > "${_kern_knob}" > > + cat "${_kern_knob}" > /dev/null > > + echo "scan" > "${_kern_knob}" > > + cat "${_kern_knob}" > /dev/null > > + echo "clear" > "${_kern_knob}" > > So this leaves the automatic background scan on, seems this is not a > problem. But how about disabling automatic scan (echo "scan=off" > > $DEBUGFS_MNT/kmemleak) and only depend on the manual scan in > _check_kmemleak? Sure? Seeing as we trigger it manually after each test there's little reason to leave the autoscan enabled. > > +} > > + > > +# check kmemleak log > > +_check_kmemleak() > > +{ > > + local _kern_knob="/sys/kernel/debug/kmemleak" > > + local _leak_file="${RESULT_DIR}/check_kmemleak" > > + > > + if [ ! -w "${_kern_knob}" ]; then > > + return 0 > > + fi > > + > > + # Tell the kernel to scan for memory leaks. Apparently the write > > + # returns before the scan is complete, so do it twice in the hopes > > + # that twice is enough to capture all the leaks. > > + echo "scan" > "${_kern_knob}" > > + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" > > + echo "scan" > "${_kern_knob}" > > + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" > > + > > + diff -u -N "${_leak_file}" "${_leak_file}.new" > $seqres.kmemleak > > Hmm, I think this reports false failure, e.g. let's say generic/001 > leaks memory and generic/002 doesn't > > ./check generic/001 # this fails kmemleak check and populates check_kmemleak file > ./check generic/002 # diff against the stale check_kmemleak file and fails the check too > > How about just check if there's anything reported from > $DEBUGFS_MNT/kmemleak and clearing the kmemleak buffer after each test? e.g. > > echo "scan" > $DEBUGFS_MNT/kmemleak > cat $DEBUGFS_MNT/kmemleak > $seqres.kmemleak > echo "clear" > $DEBUGFS_MNT/kmemleak > if [ -s $seqres.kmemleak ]; then > _dump_err "_check_kmemleak: something found in kmemleak (see $seqres.kmemleak)" > return 1 > else > rm -f $seqres.kmemleak > return 0 > fi That does seem simpler, though even with the manual scan we still have to read kmemleak twice to ensure that we capture all the leak information. (The memory leak scanner seems to have some kind of race condition with the sysfs write method...) --D > Thanks, > Eryu > > > + res=$? > > + mv "${_leak_file}.new" "${_leak_file}" > > + > > + if [ $res -ne 0 ]; then > > + _dump_err "_check_kmemleak: something found in kmemleak (see $seqres.kmemleak)" > > + return 1 > > + else > > + rm -f $seqres.kmemleak > > + return 0 > > + fi > > +} > > + > > # don't check dmesg log after test > > _disable_dmesg_check() > > { > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/check b/check index b2d251a..469188e 100755 --- a/check +++ b/check @@ -497,6 +497,7 @@ _check_filesystems() fi } +_init_kmemleak _prepare_test_list if $OPTIONS_HAVE_SECTIONS; then @@ -793,6 +794,7 @@ for section in $HOST_OPTIONS_SECTIONS; do n_try=`expr $n_try + 1` _check_filesystems _check_dmesg || err=true + _check_kmemleak || err=true fi fi diff --git a/common/rc b/common/rc index cb83918..6dc77d5 100644 --- a/common/rc +++ b/common/rc @@ -3339,6 +3339,57 @@ _check_dmesg() fi } +# set up kmemleak +_init_kmemleak() +{ + local _kern_knob="/sys/kernel/debug/kmemleak" + + if [ ! -w "${_kern_knob}" ]; then + return 0 + fi + + # Scan for all the memory leaks that have happened to date and + # clear them so that we can pinpoint leaks to tests accurately. + # Scan twice because the first write finishes before the scan + # does.... + echo "scan" > "${_kern_knob}" + cat "${_kern_knob}" > /dev/null + echo "scan" > "${_kern_knob}" + cat "${_kern_knob}" > /dev/null + echo "clear" > "${_kern_knob}" +} + +# check kmemleak log +_check_kmemleak() +{ + local _kern_knob="/sys/kernel/debug/kmemleak" + local _leak_file="${RESULT_DIR}/check_kmemleak" + + if [ ! -w "${_kern_knob}" ]; then + return 0 + fi + + # Tell the kernel to scan for memory leaks. Apparently the write + # returns before the scan is complete, so do it twice in the hopes + # that twice is enough to capture all the leaks. + echo "scan" > "${_kern_knob}" + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" + echo "scan" > "${_kern_knob}" + sed -e 's/age [0-9\.]*s/age XXXX/g' < "${_kern_knob}" > "${_leak_file}.new" + + diff -u -N "${_leak_file}" "${_leak_file}.new" > $seqres.kmemleak + res=$? + mv "${_leak_file}.new" "${_leak_file}" + + if [ $res -ne 0 ]; then + _dump_err "_check_kmemleak: something found in kmemleak (see $seqres.kmemleak)" + return 1 + else + rm -f $seqres.kmemleak + return 0 + fi +} + # don't check dmesg log after test _disable_dmesg_check() {