diff mbox

generic/127 failure on xfs with hacked fsx

Message ID CAJfpegs=RenwFojwAze_y7WF3hwiNu-+5qDw_wyT=o0M063HkQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Miklos Szeredi March 14, 2018, 3:50 p.m. UTC
While testing overlayfs I found something which appears to be an
unwanted side effect of xfs_release().

Apply the attached patch to xfstests and run generic/127.

Result is:

    +ltp/fsx -q -l 262144 -o 65536 -S 191110531 -N 100000 fsx_std_mmap
    +Mapped Read: non-zero data past EOF (0x8aa1) page offset 0xaa2 is 0xa091
    +LOG DUMP (70315 total operations):

Thanks,
Miklos

Comments

Darrick J. Wong March 14, 2018, 9:18 p.m. UTC | #1
On Wed, Mar 14, 2018 at 04:50:02PM +0100, Miklos Szeredi wrote:
> While testing overlayfs I found something which appears to be an
> unwanted side effect of xfs_release().
> 
> Apply the attached patch to xfstests and run generic/127.
> 
> Result is:
> 
>     +ltp/fsx -q -l 262144 -o 65536 -S 191110531 -N 100000 fsx_std_mmap
>     +Mapped Read: non-zero data past EOF (0x8aa1) page offset 0xaa2 is 0xa091
>     +LOG DUMP (70315 total operations):
> 
> Thanks,
> Miklos

Hmm.... so I applied this and ran generic/127 on xfs:

FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 magnolia-mtr00 4.16.0-rc5-xfsx
MKFS_OPTIONS  -- -f -m reflink=1,rmapbt=1, -i sparse=1, /dev/pmem4
MOUNT_OPTIONS -- -o usrquota,grpquota,prjquota, /dev/sda /opt

generic/127      53s
Ran: generic/127
Passed all 1 tests

Reran a couple of times without seeing any problems, so what am
I missing?  Are you running xfstests atop overlayfs atop xfs?

/me confused. :/

--D

> ---
>  ltp/fsx.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -647,6 +647,14 @@ check_trunc_hack(void)
>  	}
>  }
>  
> +static void openclose(int fd)
> +{
> +	char path[32];
> +
> +	sprintf(path, "/proc/self/fd/%i", fd);
> +	close(open(path, O_RDONLY));
> +}
> +
>  void
>  doflush(unsigned offset, unsigned size)
>  {
> @@ -799,6 +807,7 @@ domapread(unsigned offset, unsigned size
>  	        prterr("domapread: mmap");
>  		report_failure(190);
>  	}
> +	openclose(fd);
>  	memcpy(temp_buf, p + pg_offset, size);
>  
>  	check_eofpage("Read", offset, p, size);

--
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
Dave Chinner March 14, 2018, 9:27 p.m. UTC | #2
On Wed, Mar 14, 2018 at 02:18:51PM -0700, Darrick J. Wong wrote:
> On Wed, Mar 14, 2018 at 04:50:02PM +0100, Miklos Szeredi wrote:
> > While testing overlayfs I found something which appears to be an
> > unwanted side effect of xfs_release().
> > 
> > Apply the attached patch to xfstests and run generic/127.
> > 
> > Result is:
> > 
> >     +ltp/fsx -q -l 262144 -o 65536 -S 191110531 -N 100000 fsx_std_mmap
> >     +Mapped Read: non-zero data past EOF (0x8aa1) page offset 0xaa2 is 0xa091
> >     +LOG DUMP (70315 total operations):
> > 
> > Thanks,
> > Miklos
> 
> Hmm.... so I applied this and ran generic/127 on xfs:
> 
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 magnolia-mtr00 4.16.0-rc5-xfsx
> MKFS_OPTIONS  -- -f -m reflink=1,rmapbt=1, -i sparse=1, /dev/pmem4
> MOUNT_OPTIONS -- -o usrquota,grpquota,prjquota, /dev/sda /opt
> 
> generic/127      53s
> Ran: generic/127
> Passed all 1 tests
> 
> Reran a couple of times without seeing any problems, so what am
> I missing?  Are you running xfstests atop overlayfs atop xfs?
> 
> /me confused. :/

I'm betting it's timing related. the close can run a filemap_flush()
call, so there's every chance the page is under IO during the
mapread operation after the openclose() call. On pmem, it won't
be under IO because the writeback IO is a synchronous memcpy....

CHeers,

Dave.
Miklos Szeredi March 15, 2018, 7:58 a.m. UTC | #3
On Wed, Mar 14, 2018 at 10:27 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Wed, Mar 14, 2018 at 02:18:51PM -0700, Darrick J. Wong wrote:
>> On Wed, Mar 14, 2018 at 04:50:02PM +0100, Miklos Szeredi wrote:
>> > While testing overlayfs I found something which appears to be an
>> > unwanted side effect of xfs_release().
>> >
>> > Apply the attached patch to xfstests and run generic/127.
>> >
>> > Result is:
>> >
>> >     +ltp/fsx -q -l 262144 -o 65536 -S 191110531 -N 100000 fsx_std_mmap
>> >     +Mapped Read: non-zero data past EOF (0x8aa1) page offset 0xaa2 is 0xa091
>> >     +LOG DUMP (70315 total operations):
>> >
>> > Thanks,
>> > Miklos
>>
>> Hmm.... so I applied this and ran generic/127 on xfs:
>>
>> FSTYP         -- xfs (debug)
>> PLATFORM      -- Linux/x86_64 magnolia-mtr00 4.16.0-rc5-xfsx
>> MKFS_OPTIONS  -- -f -m reflink=1,rmapbt=1, -i sparse=1, /dev/pmem4
>> MOUNT_OPTIONS -- -o usrquota,grpquota,prjquota, /dev/sda /opt
>>
>> generic/127      53s
>> Ran: generic/127
>> Passed all 1 tests
>>
>> Reran a couple of times without seeing any problems, so what am
>> I missing?  Are you running xfstests atop overlayfs atop xfs?

No, plain xfs.

>>
>> /me confused. :/
>
> I'm betting it's timing related. the close can run a filemap_flush()
> call, so there's every chance the page is under IO during the
> mapread operation after the openclose() call. On pmem, it won't
> be under IO because the writeback IO is a synchronous memcpy....

Forgot to mention that xfs partition is on virtio_blk device.

Thanks,
Miklos
--
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

---
 ltp/fsx.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -647,6 +647,14 @@  check_trunc_hack(void)
 	}
 }
 
+static void openclose(int fd)
+{
+	char path[32];
+
+	sprintf(path, "/proc/self/fd/%i", fd);
+	close(open(path, O_RDONLY));
+}
+
 void
 doflush(unsigned offset, unsigned size)
 {
@@ -799,6 +807,7 @@  domapread(unsigned offset, unsigned size
 	        prterr("domapread: mmap");
 		report_failure(190);
 	}
+	openclose(fd);
 	memcpy(temp_buf, p + pg_offset, size);
 
 	check_eofpage("Read", offset, p, size);