Re: [PATCH] common/rc: report kmemleak errors

["Darrick J. Wong" <darrick.wong@oracle.com>]

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