diff mbox

common/rc: report kmemleak errors

Message ID 20171129010937.GB31372@magnolia (mailing list archive)
State New, archived
Headers show

Commit Message

Darrick J. Wong Nov. 29, 2017, 1:09 a.m. UTC
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(+)

--
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

Comments

Eryu Guan Dec. 4, 2017, 4:55 a.m. UTC | #1
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 fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Dec. 4, 2017, 6:45 p.m. UTC | #2
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 fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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()
 {