All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] common/rc: report kmemleak errors
@ 2017-11-29  1:09 Darrick J. Wong
  2017-12-04  4:55 ` Eryu Guan
  0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2017-11-29  1:09 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, xfs

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] common/rc: report kmemleak errors
  2017-11-29  1:09 [PATCH] common/rc: report kmemleak errors Darrick J. Wong
@ 2017-12-04  4:55 ` Eryu Guan
  2017-12-04 18:45   ` Darrick J. Wong
  0 siblings, 1 reply; 3+ messages in thread
From: Eryu Guan @ 2017-12-04  4:55 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, xfs

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] common/rc: report kmemleak errors
  2017-12-04  4:55 ` Eryu Guan
@ 2017-12-04 18:45   ` Darrick J. Wong
  0 siblings, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2017-12-04 18:45 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, xfs

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-12-05  2:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29  1:09 [PATCH] common/rc: report kmemleak errors Darrick J. Wong
2017-12-04  4:55 ` Eryu Guan
2017-12-04 18:45   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.