All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] overlay: another test for dropping nlink below zero
@ 2020-04-09 11:22 Amir Goldstein
  2020-04-19 15:01 ` Eryu Guan
  0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2020-04-09 11:22 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Miklos Szeredi, linux-unionfs, fstests

This is a variant on test overlay/034.

This variant is mangling upper hardlinks instead of lower hardlinks
and does not require the inodes index feature.

This is a regression test for kernel commit 83552eacdfc0
("ovl: fix WARN_ON nlink drop to zero")

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Eryu,

The kernel fix commit just got merged.

Thanks,
Amir.

 tests/overlay/072     | 85 +++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/072.out |  2 +
 tests/overlay/group   |  1 +
 3 files changed, 88 insertions(+)
 create mode 100755 tests/overlay/072
 create mode 100644 tests/overlay/072.out

diff --git a/tests/overlay/072 b/tests/overlay/072
new file mode 100755
index 00000000..e9084e5c
--- /dev/null
+++ b/tests/overlay/072
@@ -0,0 +1,85 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2020 CTERA Networks. All Rights Reserved.
+#
+# FS QA Test 072
+#
+# Test overlay nlink when adding upper hardlinks.
+#
+# nlink of overlay inode could be dropped indefinitely by adding
+# unaccounted upper hardlinks underneath a mounted overlay and
+# trying to remove them.
+#
+# This is a variant of test overlay/034 with mangling of upper instead
+# of lower hardlinks. Unlike overlay/034, this test does not require the
+# inode index feature and will pass whether is it enabled or disabled
+# by default.
+#
+# This is a regression test for kernel commit 83552eacdfc0
+# ("ovl: fix WARN_ON nlink drop to zero").
+# Without the fix, the test triggers
+# WARN_ON(inode->i_nlink == 0) in drop_link().
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+
+# Remove all files from previous tests
+_scratch_mkfs
+
+# Create lower hardlink
+mkdir -p $upperdir
+touch $upperdir/0
+ln $upperdir/0 $upperdir/1
+
+_scratch_mount
+
+# Copy up lower hardlink - overlay inode nlink 2 is copied from lower
+touch $SCRATCH_MNT/0
+
+# Add lower hardlinks while overlay is mounted - overlay inode nlink
+# is not being updated
+ln $upperdir/0 $upperdir/2
+ln $upperdir/0 $upperdir/3
+
+# Unlink the 2 un-accounted lower hardlinks - overlay inode nlinks
+# drops 2 and may reach 0 if the situation is not detected
+rm $SCRATCH_MNT/2
+rm $SCRATCH_MNT/3
+
+# Check if getting ENOENT when trying to link !I_LINKABLE with nlink 0
+ln $SCRATCH_MNT/0 $SCRATCH_MNT/4
+
+# Unlink all hardlinks - if overlay inode nlink is 0, this will trigger
+# WARN_ON() in drop_nlink()
+rm $SCRATCH_MNT/0
+rm $SCRATCH_MNT/1
+rm $SCRATCH_MNT/4
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/overlay/072.out b/tests/overlay/072.out
new file mode 100644
index 00000000..590bbc6c
--- /dev/null
+++ b/tests/overlay/072.out
@@ -0,0 +1,2 @@
+QA output created by 072
+Silence is golden
diff --git a/tests/overlay/group b/tests/overlay/group
index 43ad8a52..82876d09 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -74,3 +74,4 @@
 069 auto quick copyup hardlink exportfs nested nonsamefs
 070 auto quick copyup redirect nested
 071 auto quick copyup redirect nested nonsamefs
+072 auto quick copyup hardlink
-- 
2.17.1


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

* Re: [PATCH] overlay: another test for dropping nlink below zero
  2020-04-09 11:22 [PATCH] overlay: another test for dropping nlink below zero Amir Goldstein
@ 2020-04-19 15:01 ` Eryu Guan
  2020-04-19 15:42   ` Amir Goldstein
  0 siblings, 1 reply; 3+ messages in thread
From: Eryu Guan @ 2020-04-19 15:01 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Eryu Guan, Miklos Szeredi, linux-unionfs, fstests

On Thu, Apr 09, 2020 at 02:22:23PM +0300, Amir Goldstein wrote:
> This is a variant on test overlay/034.
> 
> This variant is mangling upper hardlinks instead of lower hardlinks
> and does not require the inodes index feature.
> 
> This is a regression test for kernel commit 83552eacdfc0
> ("ovl: fix WARN_ON nlink drop to zero")
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Eryu,
> 
> The kernel fix commit just got merged.
> 
> Thanks,
> Amir.
> 
>  tests/overlay/072     | 85 +++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/072.out |  2 +
>  tests/overlay/group   |  1 +
>  3 files changed, 88 insertions(+)
>  create mode 100755 tests/overlay/072
>  create mode 100644 tests/overlay/072.out
> 
> diff --git a/tests/overlay/072 b/tests/overlay/072
> new file mode 100755
> index 00000000..e9084e5c
> --- /dev/null
> +++ b/tests/overlay/072
> @@ -0,0 +1,85 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2020 CTERA Networks. All Rights Reserved.
> +#
> +# FS QA Test 072
> +#
> +# Test overlay nlink when adding upper hardlinks.
> +#
> +# nlink of overlay inode could be dropped indefinitely by adding
> +# unaccounted upper hardlinks underneath a mounted overlay and
> +# trying to remove them.
> +#
> +# This is a variant of test overlay/034 with mangling of upper instead
> +# of lower hardlinks. Unlike overlay/034, this test does not require the
> +# inode index feature and will pass whether is it enabled or disabled
> +# by default.
> +#
> +# This is a regression test for kernel commit 83552eacdfc0
> +# ("ovl: fix WARN_ON nlink drop to zero").
> +# Without the fix, the test triggers
> +# WARN_ON(inode->i_nlink == 0) in drop_link().
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +
> +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> +
> +# Remove all files from previous tests
> +_scratch_mkfs
> +
> +# Create lower hardlink

It seems there're some stale comments that are copied from overlay/034,
above is one of them, should be "Create upper hardlink"

> +mkdir -p $upperdir
> +touch $upperdir/0
> +ln $upperdir/0 $upperdir/1
> +
> +_scratch_mount
> +
> +# Copy up lower hardlink - overlay inode nlink 2 is copied from lower
> +touch $SCRATCH_MNT/0

There's no copyup, then do we need this touch at all?

> +
> +# Add lower hardlinks while overlay is mounted - overlay inode nlink

Add upper hardlinks ...

> +# is not being updated
> +ln $upperdir/0 $upperdir/2
> +ln $upperdir/0 $upperdir/3
> +
> +# Unlink the 2 un-accounted lower hardlinks - overlay inode nlinks
                               ^^^^^ upper?

Thanks,
Eryu

> +# drops 2 and may reach 0 if the situation is not detected
> +rm $SCRATCH_MNT/2
> +rm $SCRATCH_MNT/3
> +
> +# Check if getting ENOENT when trying to link !I_LINKABLE with nlink 0
> +ln $SCRATCH_MNT/0 $SCRATCH_MNT/4
> +
> +# Unlink all hardlinks - if overlay inode nlink is 0, this will trigger
> +# WARN_ON() in drop_nlink()
> +rm $SCRATCH_MNT/0
> +rm $SCRATCH_MNT/1
> +rm $SCRATCH_MNT/4
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/overlay/072.out b/tests/overlay/072.out
> new file mode 100644
> index 00000000..590bbc6c
> --- /dev/null
> +++ b/tests/overlay/072.out
> @@ -0,0 +1,2 @@
> +QA output created by 072
> +Silence is golden
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 43ad8a52..82876d09 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -74,3 +74,4 @@
>  069 auto quick copyup hardlink exportfs nested nonsamefs
>  070 auto quick copyup redirect nested
>  071 auto quick copyup redirect nested nonsamefs
> +072 auto quick copyup hardlink
> -- 
> 2.17.1
> 

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

* Re: [PATCH] overlay: another test for dropping nlink below zero
  2020-04-19 15:01 ` Eryu Guan
@ 2020-04-19 15:42   ` Amir Goldstein
  0 siblings, 0 replies; 3+ messages in thread
From: Amir Goldstein @ 2020-04-19 15:42 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Eryu Guan, Miklos Szeredi, overlayfs, fstests

On Sun, Apr 19, 2020 at 6:00 PM Eryu Guan <guan@eryu.me> wrote:
>
> On Thu, Apr 09, 2020 at 02:22:23PM +0300, Amir Goldstein wrote:
> > This is a variant on test overlay/034.
> >
> > This variant is mangling upper hardlinks instead of lower hardlinks
> > and does not require the inodes index feature.
> >
> > This is a regression test for kernel commit 83552eacdfc0
> > ("ovl: fix WARN_ON nlink drop to zero")
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Eryu,
> >
> > The kernel fix commit just got merged.
> >
> > Thanks,
> > Amir.
> >
> >  tests/overlay/072     | 85 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/overlay/072.out |  2 +
> >  tests/overlay/group   |  1 +
> >  3 files changed, 88 insertions(+)
> >  create mode 100755 tests/overlay/072
> >  create mode 100644 tests/overlay/072.out
> >
> > diff --git a/tests/overlay/072 b/tests/overlay/072
> > new file mode 100755
> > index 00000000..e9084e5c
> > --- /dev/null
> > +++ b/tests/overlay/072
> > @@ -0,0 +1,85 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2020 CTERA Networks. All Rights Reserved.
> > +#
> > +# FS QA Test 072
> > +#
> > +# Test overlay nlink when adding upper hardlinks.
> > +#
> > +# nlink of overlay inode could be dropped indefinitely by adding
> > +# unaccounted upper hardlinks underneath a mounted overlay and
> > +# trying to remove them.
> > +#
> > +# This is a variant of test overlay/034 with mangling of upper instead
> > +# of lower hardlinks. Unlike overlay/034, this test does not require the
> > +# inode index feature and will pass whether is it enabled or disabled
> > +# by default.
> > +#
> > +# This is a regression test for kernel commit 83552eacdfc0
> > +# ("ovl: fix WARN_ON nlink drop to zero").
> > +# Without the fix, the test triggers
> > +# WARN_ON(inode->i_nlink == 0) in drop_link().
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +tmp=/tmp/$$
> > +status=1     # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +     cd /
> > +     rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +_supported_fs overlay
> > +_supported_os Linux
> > +_require_scratch
> > +
> > +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> > +
> > +# Remove all files from previous tests
> > +_scratch_mkfs
> > +
> > +# Create lower hardlink
>
> It seems there're some stale comments that are copied from overlay/034,
> above is one of them, should be "Create upper hardlink"
>

Yep. I'll fix those up.

> > +mkdir -p $upperdir
> > +touch $upperdir/0
> > +ln $upperdir/0 $upperdir/1
> > +
> > +_scratch_mount
> > +
> > +# Copy up lower hardlink - overlay inode nlink 2 is copied from lower
> > +touch $SCRATCH_MNT/0
>
> There's no copyup, then do we need this touch at all?

We do not need touch, but we do need something to do the
lookup and read upper nlink before modifying it underneath.

Good points.
I'll send fixed v2.

Thanks,
Amir.

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

end of thread, other threads:[~2020-04-19 15:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 11:22 [PATCH] overlay: another test for dropping nlink below zero Amir Goldstein
2020-04-19 15:01 ` Eryu Guan
2020-04-19 15:42   ` Amir Goldstein

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.