[PATCH v2 1/2] generic/553: fix test description

[PATCH v2 1/2] generic/553: fix test description

[Amir Goldstein]

The test only checks copy to immutable file.
Note the kernel fix commit.

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

Changes from v1:
- Document kernel fix commit

 tests/generic/553 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/generic/553 b/tests/generic/553
index 98ef77cc..58e0d5ef 100755
--- a/tests/generic/553
+++ b/tests/generic/553
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 553
 #
-# Check that we cannot copy_file_range() to/from an immutable file
+# Check that we cannot copy_file_range() to an immutable file
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
-- 
2.17.1

[PATCH v2 2/2] generic/554: test only copy to active swap file

[Amir Goldstein]

Depending on filesystem, copying from active swapfile may be allowed,
just as read from swapfile may be allowed.

Note the kernel fix commit in test description.

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

Eryu,

Per your and Ted's request, I've documented the kernel fix commit
in the new copy_range tests. Those commits are now on Darrick's
copy-file-range-fixes branch, which is on its way to linux-next
and to kernel 5.3.

Thanks,
Amir.


Changes from v1:
- Document kernel fix commit

 tests/generic/554     | 6 ++++--
 tests/generic/554.out | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/generic/554 b/tests/generic/554
index 10ae4035..fa19d580 100755
--- a/tests/generic/554
+++ b/tests/generic/554
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 554
 #
-# Check that we cannot copy_file_range() to/from a swapfile
+# Check that we cannot copy_file_range() to a swapfile
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
@@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
 _format_swapfile $SCRATCH_MNT/swapfile 16m
 swapon $SCRATCH_MNT/swapfile
 $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
-$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
 swapoff $SCRATCH_MNT/swapfile
 
 # success, all done
diff --git a/tests/generic/554.out b/tests/generic/554.out
index ffaa7b0a..19385a05 100644
--- a/tests/generic/554.out
+++ b/tests/generic/554.out
@@ -1,4 +1,3 @@
 QA output created by 554
 swap files return ETXTBUSY
 copy_range: Text file busy
-copy_range: Text file busy
-- 
2.17.1

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Amir Goldstein]

On Tue, Jun 11, 2019 at 6:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Depending on filesystem, copying from active swapfile may be allowed,
> just as read from swapfile may be allowed.
>
> Note the kernel fix commit in test description.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Forgot to add:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>
> Eryu,
>
> Per your and Ted's request, I've documented the kernel fix commit
> in the new copy_range tests. Those commits are now on Darrick's
> copy-file-range-fixes branch, which is on its way to linux-next
> and to kernel 5.3.
>
> Thanks,
> Amir.
>
>
> Changes from v1:
> - Document kernel fix commit
>
>  tests/generic/554     | 6 ++++--
>  tests/generic/554.out | 1 -
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tests/generic/554 b/tests/generic/554
> index 10ae4035..fa19d580 100755
> --- a/tests/generic/554
> +++ b/tests/generic/554
> @@ -4,7 +4,10 @@
>  #
>  # FS QA Test No. 554
>  #
> -# Check that we cannot copy_file_range() to/from a swapfile
> +# Check that we cannot copy_file_range() to a swapfile
> +#
> +# This is a regression test for kernel commit:
> +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
>  #
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> @@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
>  _format_swapfile $SCRATCH_MNT/swapfile 16m
>  swapon $SCRATCH_MNT/swapfile
>  $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
> -$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
>  swapoff $SCRATCH_MNT/swapfile
>
>  # success, all done
> diff --git a/tests/generic/554.out b/tests/generic/554.out
> index ffaa7b0a..19385a05 100644
> --- a/tests/generic/554.out
> +++ b/tests/generic/554.out
> @@ -1,4 +1,3 @@
>  QA output created by 554
>  swap files return ETXTBUSY
>  copy_range: Text file busy
> -copy_range: Text file busy
> --
> 2.17.1
>

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Theodore Ts'o]

On Tue, Jun 11, 2019 at 06:39:16PM +0300, Amir Goldstein wrote:
> Depending on filesystem, copying from active swapfile may be allowed,
> just as read from swapfile may be allowed.
> 
> Note the kernel fix commit in test description.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> 
> Per your and Ted's request, I've documented the kernel fix commit
> in the new copy_range tests. Those commits are now on Darrick's
> copy-file-range-fixes branch, which is on its way to linux-next
> and to kernel 5.3.

Thanks!  Are we sure at this point that the commit won't need to be
modified / rebased in Darrick's tree?

> +# This is a regression test for kernel commit:
> +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")

					 - Ted

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Amir Goldstein]

On Tue, Jun 11, 2019 at 6:59 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Tue, Jun 11, 2019 at 06:39:16PM +0300, Amir Goldstein wrote:
> > Depending on filesystem, copying from active swapfile may be allowed,
> > just as read from swapfile may be allowed.
> >
> > Note the kernel fix commit in test description.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Per your and Ted's request, I've documented the kernel fix commit
> > in the new copy_range tests. Those commits are now on Darrick's
> > copy-file-range-fixes branch, which is on its way to linux-next
> > and to kernel 5.3.
>
> Thanks!  Are we sure at this point that the commit won't need to be
> modified / rebased in Darrick's tree?
>

Yes. The intention is that fs maintainers can now rebase their
v5.3 development branch on this non-rewindable branch.

Note that you do still need to merge the fixes from Darrick's branch
for ext4 to pass this test despite relaxing the "copy from swapfile" test case.

Thanks,
Amir.

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Murphy Zhou]

On Tue, Jun 11, 2019 at 06:39:16PM +0300, Amir Goldstein wrote:
> Depending on filesystem, copying from active swapfile may be allowed,
> just as read from swapfile may be allowed.

...snip..

> +# This is a regression test for kernel commit:
> +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")

Would you mind updating sha1 after it get merged to Linus tree?

That would be helpful for people tracking this issue.

Thanks,
Murphy

>  #
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> @@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
>  _format_swapfile $SCRATCH_MNT/swapfile 16m
>  swapon $SCRATCH_MNT/swapfile
>  $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
> -$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
>  swapoff $SCRATCH_MNT/swapfile
>  
>  # success, all done
> diff --git a/tests/generic/554.out b/tests/generic/554.out
> index ffaa7b0a..19385a05 100644
> --- a/tests/generic/554.out
> +++ b/tests/generic/554.out
> @@ -1,4 +1,3 @@
>  QA output created by 554
>  swap files return ETXTBUSY
>  copy_range: Text file busy
> -copy_range: Text file busy
> -- 
> 2.17.1
> 

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Amir Goldstein]

On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> On Tue, Jun 11, 2019 at 06:39:16PM +0300, Amir Goldstein wrote:
> > Depending on filesystem, copying from active swapfile may be allowed,
> > just as read from swapfile may be allowed.
>
> ...snip..
>
> > +# This is a regression test for kernel commit:
> > +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
>
> Would you mind updating sha1 after it get merged to Linus tree?
>
> That would be helpful for people tracking this issue.
>

This is the commit id in linux-next and expected to stay the same
when the fix is merged to Linus tree for 5.3.

Thanks,
Amir.

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Murphy Zhou]

On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
> On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> >
> > On Tue, Jun 11, 2019 at 06:39:16PM +0300, Amir Goldstein wrote:
> > > Depending on filesystem, copying from active swapfile may be allowed,
> > > just as read from swapfile may be allowed.
> >
> > ...snip..
> >
> > > +# This is a regression test for kernel commit:
> > > +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
> >
> > Would you mind updating sha1 after it get merged to Linus tree?
> >
> > That would be helpful for people tracking this issue.
> >
> 
> This is the commit id in linux-next and expected to stay the same
> when the fix is merged to Linus tree for 5.3.

That's perfect. :)

> 
> Thanks,
> Amir.

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Theodore Ts'o]

On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
> On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> >
> > Would you mind updating sha1 after it get merged to Linus tree?
> >
> > That would be helpful for people tracking this issue.
> >
> 
> This is the commit id in linux-next and expected to stay the same
> when the fix is merged to Linus tree for 5.3.

When I talked to Darrick last week, that was *not* the sense I got
from him.  It's not necessarily guaranteed to be stable just yet...

     	   	    			   - Ted

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Darrick J. Wong]

On Tue, Jun 18, 2019 at 11:02:42AM -0400, Theodore Ts'o wrote:
> On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
> > On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> > >
> > > Would you mind updating sha1 after it get merged to Linus tree?
> > >
> > > That would be helpful for people tracking this issue.
> > >
> > 
> > This is the commit id in linux-next and expected to stay the same
> > when the fix is merged to Linus tree for 5.3.
> 
> When I talked to Darrick last week, that was *not* the sense I got
> from him.  It's not necessarily guaranteed to be stable just yet...

Darrick hasn't gotten any complaints about the copy-file-range-fixes
branch (which has been in for-next for a week now), so he thinks that
commit id (a31713517dac) should be stable from here on out.

(Note that doesn't guarantee that Linus will pull said branch...)

--D

>      	   	    			   - Ted

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Yang Xu]

on 2019/06/18 23:11, Darrick J. Wong  wrote:

> On Tue, Jun 18, 2019 at 11:02:42AM -0400, Theodore Ts'o wrote:
>> On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
>>> On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou<jencce.kernel@gmail.com>  wrote:
>>>> Would you mind updating sha1 after it get merged to Linus tree?
>>>>
>>>> That would be helpful for people tracking this issue.
>>>>
>>> This is the commit id in linux-next and expected to stay the same
>>> when the fix is merged to Linus tree for 5.3.
>> When I talked to Darrick last week, that was *not* the sense I got
>> from him.  It's not necessarily guaranteed to be stable just yet...
> Darrick hasn't gotten any complaints about the copy-file-range-fixes
> branch (which has been in for-next for a week now), so he thinks that
> commit id (a31713517dac) should be stable from here on out.
>
> (Note that doesn't guarantee that Linus will pull said branch...)
>
> --D
Hi Amir

The kernel fix commit message is right?  :-)  Because when I backport this patch into 5.2.0-rc6+ kernel,
generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range).
By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.

I think we can change this message after the fix is merged to Linus tree for 5.3.  What do you think about it?

Thanks
Yang Xu

>>       	   	    			- Ted
>
>

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Amir Goldstein]

On Tue, Jun 25, 2019 at 1:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:
>
> on 2019/06/18 23:11, Darrick J. Wong  wrote:
>
> > On Tue, Jun 18, 2019 at 11:02:42AM -0400, Theodore Ts'o wrote:
> >> On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
> >>> On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou<jencce.kernel@gmail.com>  wrote:
> >>>> Would you mind updating sha1 after it get merged to Linus tree?
> >>>>
> >>>> That would be helpful for people tracking this issue.
> >>>>
> >>> This is the commit id in linux-next and expected to stay the same
> >>> when the fix is merged to Linus tree for 5.3.
> >> When I talked to Darrick last week, that was *not* the sense I got
> >> from him.  It's not necessarily guaranteed to be stable just yet...
> > Darrick hasn't gotten any complaints about the copy-file-range-fixes
> > branch (which has been in for-next for a week now), so he thinks that
> > commit id (a31713517dac) should be stable from here on out.
> >
> > (Note that doesn't guarantee that Linus will pull said branch...)
> >
> > --D
> Hi Amir
>
> The kernel fix commit message is right?  :-)  Because when I backport this patch into 5.2.0-rc6+ kernel,
> generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range).
> By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.
>
> I think we can change this message after the fix is merged to Linus tree for 5.3.  What do you think about it?

You are right. Documented commit is wrong.
The correct commit in linux-next is:
96e6e8f4a68d ("vfs: add missing checks to copy_file_range")

(Not sure where you got a5544984af4 from?)
Let's fix that after the commit is upstream.

Obviously, you would need to backport the entire series and not just this
one commit to stable kernel.

Thanks,
Amir.

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

[Yang Xu]

On 2019/06/25 18:35, Amir Goldstein wrote:

> On Tue, Jun 25, 2019 at 1:29 PM Yang Xu<xuyang2018.jy@cn.fujitsu.com>  wrote:
>> on 2019/06/18 23:11, Darrick J. Wong  wrote:
>>
>>> On Tue, Jun 18, 2019 at 11:02:42AM -0400, Theodore Ts'o wrote:
>>>> On Tue, Jun 18, 2019 at 12:16:45PM +0300, Amir Goldstein wrote:
>>>>> On Tue, Jun 18, 2019 at 12:02 PM Murphy Zhou<jencce.kernel@gmail.com>   wrote:
>>>>>> Would you mind updating sha1 after it get merged to Linus tree?
>>>>>>
>>>>>> That would be helpful for people tracking this issue.
>>>>>>
>>>>> This is the commit id in linux-next and expected to stay the same
>>>>> when the fix is merged to Linus tree for 5.3.
>>>> When I talked to Darrick last week, that was *not* the sense I got
>>>> from him.  It's not necessarily guaranteed to be stable just yet...
>>> Darrick hasn't gotten any complaints about the copy-file-range-fixes
>>> branch (which has been in for-next for a week now), so he thinks that
>>> commit id (a31713517dac) should be stable from here on out.
>>>
>>> (Note that doesn't guarantee that Linus will pull said branch...)
>>>
>>> --D
>> Hi Amir
>>
>> The kernel fix commit message is right?  :-)  Because when I backport this patch into 5.2.0-rc6+ kernel,
>> generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range).
>> By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.
>>
>> I think we can change this message after the fix is merged to Linus tree for 5.3.  What do you think about it?
> You are right. Documented commit is wrong.
> The correct commit in linux-next is:
> 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
>
> (Not sure where you got a5544984af4 from?)
I get a5544984af4 from Darrick.wong copy-file-range-fixes branch.

> Let's fix that after the commit is upstream.
>
> Obviously, you would need to backport the entire series and not just this
> one commit to stable kernel.
Yes. I got it.

> Thanks,
> Amir.
>
>
>

Re: [PATCH v2 1/2] generic/553: fix test description

[Amir Goldstein]

On Tue, Jun 11, 2019 at 6:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> The test only checks copy to immutable file.
> Note the kernel fix commit.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Forgot to add:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>
> Changes from v1:
> - Document kernel fix commit
>
>  tests/generic/553 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/generic/553 b/tests/generic/553
> index 98ef77cc..58e0d5ef 100755
> --- a/tests/generic/553
> +++ b/tests/generic/553
> @@ -4,7 +4,10 @@
>  #
>  # FS QA Test No. 553
>  #
> -# Check that we cannot copy_file_range() to/from an immutable file
> +# Check that we cannot copy_file_range() to an immutable file
> +#
> +# This is a regression test for kernel commit:
> +#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
>  #
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> --
> 2.17.1
>