All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ira Weiny <ira.weiny@intel.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-kernel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>,
	"Theodore Y. Ts'o" <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH V2 10/12] fs/xfs: Fix truncate up
Date: Tue, 14 Jan 2020 11:39:36 -0800	[thread overview]
Message-ID: <20200114193934.GA23311@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <20200114190057.GB7871@iweiny-DESK2.sc.intel.com>

[-- Attachment #1: Type: text/plain, Size: 3036 bytes --]

On Tue, Jan 14, 2020 at 11:00:57AM -0800, 'Ira Weiny' wrote:
> On Mon, Jan 13, 2020 at 05:14:07PM -0800, Darrick J. Wong wrote:
> > On Mon, Jan 13, 2020 at 04:40:47PM -0800, Ira Weiny wrote:
> > > On Mon, Jan 13, 2020 at 02:27:55PM -0800, Darrick J. Wong wrote:
> > > > On Fri, Jan 10, 2020 at 11:29:40AM -0800, ira.weiny@intel.com wrote:
> > > > > From: Ira Weiny <ira.weiny@intel.com>

[snip]

> 
> > 
> > > But as I said in the cover I am not 100% sure of
> > > this fix.
> > 
> > > From what I can tell xfs_ioctl_setattr_dax_invalidate() should invalidate the
> > > mappings and the page cache and the traces I have indicate that the DAX mode
> > > is not changing or was properly held off.
> > 
> > Hmm, that implies the invalidation didn't work.  Can you find a way to
> > report the contents of the page cache after the dax mode change
> > invalidation fails?  I wonder if this is something dorky like rounding
> > down such that the EOF page doesn't actually get invalidated?
> > 
> > Hmm, no, xfs_ioctl_setattr_dax_invalidate should be nuking all the
> > pages... do you have a quick reproducer?
> 
> I thought I did...
> 
> What I have done is take this patch:
> 
> https://www.spinics.net/lists/fstests/msg13313.html
> 
> and put [run_fsx ""] in a loop... (diff below) And without this truncate fix
> patch it was failing in about 5 - 10 iterations.  But I'm running it right now
> and it has gone for 30+...  :-(

Ok...  perhaps this is a qemu problem?  In qemu I had a slightly different
script; 'test-suite.sh' (attached).  This was copied to create the xfstest I
sent.  Without this 'fix truncate patch I get something like the following
after in just a few iterations.

...

   *** run 'fsx ' racing with setting/clearing the DAX flag
   *** run 'fsx ' racing with setting/clearing the DAX flag
FAILED: fsx exited with status : 205
        see trace_output
zero_range to largest ever: 0x19867
truncating to largest ever: 0x3fb6d
zero_range to largest ever: 0x40000
Mapped Write: non-zero data past EOF (0x3ab88) page offset 0xb89 is 0xe71c
LOG DUMP (15817 total operations):
15818(202 mod 256): SKIPPED (no operation)
15819(203 mod 256): ZERO     0x2ae9f thru 0x3346c       (0x85ce bytes)
15820(204 mod 256): READ     0x3637 thru 0x91e6 (0x5bb0 bytes)
15821(205 mod 256): MAPREAD  0x38a80 thru 0x3d014       (0x4595 bytes)
15822(206 mod 256): INSERT 0x28000 thru 0x29fff (0x2000 bytes)

Ira

> 
> I am 90% confident that this series works for 100% of the use cases we have.  I
> think this is an existing bug which I've just managed to find.  And again I'm
> not comfortable with this patch either.  So I'm not trying to argue for it but
> I just don't know what could be wrong...
> 
> Ira
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> index 6dd5529dbc65..929c20c6db04 100755
> --- a/tests/generic/999
> +++ b/tests/generic/999
> @@ -274,7 +274,9 @@ function run_fsx {
>         pid=""
>  }
>  
> -run_fsx ""
> +while [ 1 ]; do
> +       run_fsx ""
> +done
>  run_fsx "-A"
>  run_fsx "-Z -r 4096 -w 4096"
> 

[-- Attachment #2: test-suite.sh --]
[-- Type: text/plain, Size: 7344 bytes --]

#!/bin/bash

#
# Run suite of tests for feature
#

mnt_pt=/mnt/pmem

function mount_no_dax {
	umount $mnt_pt
	mount /dev/pmem0 $mnt_pt
}

function mount_dax {
	umount $mnt_pt
	mount -o dax /dev/pmem0 $mnt_pt
}

# test set up
if [ ! -d /mnt/pmem ]; then
	echo ""
	echo "     Setting up FS DAX"
	echo ""

	ndctl create-namespace -e namespace0.0 -f --mode=fsdax
	#mkfs.ext4 /dev/pmem0
	mkfs.xfs -f /dev/pmem0
	mkdir -p /mnt/pmem
	mount_no_dax
fi

#if [ ! -c /dev/dax1.0 ]; then
#	echo ""
#	echo "     Setting up DEV DAX"
#	echo ""
#
#	ndctl create-namespace -f --mode=devdax --align=4096 -e namespace1.0
#fi

if [ "$1" == "--create-fs-only" ]; then
	exit 0
fi


test_file=/mnt/pmem/foo

function start_test {
	rm -f $test_file
	echo ""
	echo "     ***** START ${FUNCNAME[1]} *****"
}

function end_test {
	echo "     ***** END ${FUNCNAME[1]} *****"
	echo ""
	killall -9 rdma-fsdax
}

function expect_pass {
	echo -n "     ***** $2 : "
	if [ "$1" == "0" ]; then
		echo "PASSED"
	else
		echo "FAILED"
		exit 255
	fi
}

function expect_fail {
	echo -n "     ***** $2 : "
	if [ "$1" == "0" ]; then
		echo "FAILED"
		exit 255
	else
		echo "PASSED"
	fi
}


# test below here are not fully written yet.  Just ideas of what we should test
# for.

function check_phys_dax {
	xfs_io -c 'lsattr' $1 | awk -e '{ print $1 }' | grep 'x' &> /dev/null
	if [ "$?" != "0" ]; then
		echo "FAILED: Did NOT find DAX flag on $1"
		exit 255
	fi
}

function check_effective_dax {
	attr=`xfs_io -c 'statx -r' $1 | grep 'stat.attributes' | awk -e '{ print $3 }'`
	masked=$(( $attr & 0x2000 ))
	if [ "$masked" != "8192" ]; then
		echo "FAILED: Did NOT find VFS DAX flag on $1"
		exit 255
	fi
}

function check_phys_no_dax {
	xfs_io -c 'lsattr' $1 | awk -e '{ print $1 }' | grep 'x' &> /dev/null
	if [ "$?" == "0" ]; then
		echo "FAILED: Found DAX flag on $1"
		exit 255
	fi
}

function check_effective_no_dax {
	attr=`xfs_io -c 'statx -r' $1 | grep 'stat.attributes' | awk -e '{ print $3 }'`
	masked=$(( $attr & 0x2000 ))
	if [ "$masked" == "8192" ]; then
		echo "FAILED: Found VFS DAX flag on $1"
		exit 255
	fi
}

XFS_IO_PROG=xfs_io

TEST_DIR=/mnt/pmem

dax_dir=$TEST_DIR/dax-dir
dax_sub_dir=$TEST_DIR/dax-dir/dax-sub-dir
dax_inh_file=$dax_dir/dax-inh-file
dax_non_inh_file=$dax_dir/dax-non-inh-file
non_dax=$TEST_DIR/non-dax
dax_file=$TEST_DIR/dax-file
dax_file_copy=$TEST_DIR/dax-file-copy
dax_file_move=$TEST_DIR/dax-file-move
data_file=$TEST_DIR/data-file

function clean_up {
	echo "cleaning up..."
	rm -rf $TEST_DIR/*
}

clean_up

# Do double mount to have these special options.
mount_no_dax
touch $dax_file

if [ "$1" == "--chattr-only" ]; then
	for i in `seq 1 10000`; do
		xfs_io -c 'chattr +x' $dax_file
		xfs_io -c 'chattr -x' $dax_file
	done
	exit 0
fi

if [ "$1" == "--fsx-only" ]; then
	./fsx -R -W -N 100000 $dax_file
	head $dax_file.fsxlog
	exit 0
fi

if [ "$1" == "--fsx-aio-only" ]; then
	./fsx -R -W -A -N 100000 $dax_file
	head $dax_file.fsxlog
	exit 0
fi

# The below should be kept the same as xfstests

echo "   *** mount w/o dax flag."
mount_no_dax

echo "   *** mark dax-dir as dax enabled"
mkdir $dax_dir
xfs_io -c 'chattr +x' $dax_dir
check_phys_dax $dax_dir
check_effective_dax $dax_dir

echo "   *** check file inheritance"
touch $dax_inh_file
check_phys_dax $dax_inh_file
check_effective_dax $dax_inh_file

echo "   *** check directory inheritance"
mkdir $dax_sub_dir
check_phys_dax $dax_sub_dir
check_effective_dax $dax_sub_dir

echo "   *** check changing directory"
xfs_io -c 'chattr -x' $dax_dir
check_phys_no_dax $dax_dir
check_effective_no_dax $dax_dir

echo "   *** check non file inheritance"
touch $dax_non_inh_file
check_phys_no_dax $dax_non_inh_file
check_effective_no_dax $dax_non_inh_file

echo "   *** check that previous file stays enabled"
check_phys_dax $dax_inh_file
check_effective_dax $dax_inh_file

echo "   *** Reset the directory"
xfs_io -c 'chattr +x' $dax_dir
check_phys_dax $dax_dir
check_effective_dax $dax_dir


# check mount override
# ====================

echo "   *** Remount fs with mount flag"
mount_dax
touch $non_dax
check_phys_no_dax $non_dax
check_effective_dax $non_dax

echo "   *** Check for non-dax files to be dax with mount option"
check_effective_dax $dax_non_inh_file

echo "   *** check for file dax flag 'sticky-ness' after remount"
touch $dax_file
xfs_io -c 'chattr +x' $dax_file
check_phys_dax $dax_file
check_effective_dax $dax_file

echo "   *** remount w/o mount flag"
mount_no_dax
check_phys_dax $dax_file
check_effective_dax $dax_file

check_phys_no_dax $non_dax
check_effective_no_dax $non_dax


# Check non-zero file operations
# ==============================

echo "   *** file should change effective but page cache should be empty"
$XFS_IO_PROG -f -c "pwrite 0 10000" $data_file > /dev/null
xfs_io -c 'chattr +x' $data_file
check_phys_dax $data_file
check_effective_dax $data_file


# Check inheritance on cp, mv
# ===========================

echo "   *** check inheritance on cp, mv"
cp $non_dax $dax_dir/conv-dax
check_phys_dax $dax_dir/conv-dax
check_effective_dax $dax_dir/conv-dax

echo "   *** Moved files 'don't inherit'"
mv $non_dax $dax_dir/move-dax
check_phys_no_dax $dax_dir/move-dax
check_effective_no_dax $dax_dir/move-dax

# Check preservation of phys on cp, mv
# ====================================

mv $dax_file $dax_file_move
check_phys_dax $dax_file_move
check_effective_dax $dax_file_move

cp $dax_file_move $dax_file_copy
check_phys_no_dax $dax_file_copy
check_effective_no_dax $dax_file_copy


# Verify no mode changes on mmap
# ==============================

echo "   *** check no mode change when mmaped"

dd if=/dev/zero of=$dax_file bs=4096 count=10 > $tmp.log 2>&1

# set known state.
xfs_io -c 'chattr -x' $dax_file
check_phys_no_dax $dax_file
check_effective_no_dax $dax_file

python3 - << EOF > $tmp.log 2>&1 &
import mmap
import time
print ('mmaping "$dax_file"')
f=open("$dax_file", "r+b")
mm = mmap.mmap(f.fileno(), 0)
print ('mmaped "$dax_file"')
while True:
	time.sleep(1)
EOF
pid=$!

sleep 1

# attempt to should fail
xfs_io -c 'chattr +x' $dax_file > /dev/null 2>&1
check_phys_no_dax $dax_file
check_effective_no_dax $dax_file

kill -TERM $pid > /dev/null 2>&1
wait $pid > /dev/null 2>&1

# after mmap released should work
xfs_io -c 'chattr +x' $dax_file
check_phys_dax $dax_file
check_effective_dax $dax_file


# Finally run the test stolen from Christoph Hellwig to test changing the mode
# while performing a series of operations
# =============================================================================

function run_fsx {
	options=$1

	echo "   *** run 'fsx $options' racing with setting/clearing the DAX flag"
	./fsx $options -N 20000 $dax_file > $tmp.log 2>&1 &
	pid=$!

	if [ ! -n "$pid" ]; then
		echo "FAILED to start fsx"
		exit 255
	fi

	# NOTE: for some
	for i in `seq 1 500`; do
		xfs_io -c 'chattr +x' $dax_file > /dev/null 2>&1
		xfs_io -c 'chattr -x' $dax_file > /dev/null 2>&1
	done

	wait $pid
	status=$?
	if [ "$status" != "0" ]; then
		cat /sys/kernel/debug/tracing/trace > trace_output
		echo "FAILED: fsx exited with status : $status"
		echo "        see trace_output"
		head $dax_file.fsxlog
		exit $status
	fi
	pid=""
}

while [ 1 ]; do
	run_fsx ""
done
	run_fsx "-A"
	run_fsx "-Z -r 4096 -w 4096"

echo "   *** Check 'dump' and 'xfsdump'"
echo "      TBD"

echo "   *** PASSED!  All tests PASSED ***"
exit 0


  reply	other threads:[~2020-01-14 19:39 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10 19:29 [RFC PATCH V2 00/12] Enable per-file/directory DAX operations V2 ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 01/12] fs/stat: Define DAX statx attribute ira.weiny
2020-01-15 11:37   ` Jan Kara
2020-01-15 17:38     ` Darrick J. Wong
2020-01-15 19:45       ` Ira Weiny
2020-01-15 20:10         ` Dan Williams
2020-01-15 22:38           ` Ira Weiny
2020-01-16  5:39             ` Darrick J. Wong
2020-01-16  6:05               ` Dan Williams
2020-01-16  6:18                 ` Darrick J. Wong
2020-01-16  6:25                   ` Dan Williams
2020-01-18  9:11                 ` Dave Chinner
2020-01-16 17:55               ` Ira Weiny
2020-01-16 18:04                 ` Darrick J. Wong
2020-01-16 18:52                   ` Ira Weiny
2020-01-16 22:19                     ` Darrick J. Wong
2020-01-17 11:58                     ` Jan Kara
2020-01-10 19:29 ` [RFC PATCH V2 02/12] fs/xfs: Isolate the physical DAX flag from effective ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 03/12] fs/xfs: Separate functionality of xfs_inode_supports_dax() ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 04/12] fs/xfs: Clean up DAX support check ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 05/12] fs: remove unneeded IS_DAX() check ira.weiny
2020-01-16  9:38   ` Jan Kara
2020-01-16 18:47     ` Ira Weiny
2020-01-10 19:29 ` [RFC PATCH V2 06/12] fs/xfs: Check if the inode supports DAX under lock ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 07/12] fs: Add locking for a dynamic inode 'mode' ira.weiny
2020-01-13 22:12   ` Darrick J. Wong
2020-01-14  0:20     ` Ira Weiny
2020-01-14  1:03       ` Darrick J. Wong
2020-01-15 19:08         ` Ira Weiny
2020-01-16  5:40           ` Darrick J. Wong
2020-01-16 18:54             ` Ira Weiny
2020-01-10 19:29 ` [RFC PATCH V2 08/12] fs/xfs: Add lock/unlock mode to xfs ira.weiny
2020-01-11 14:49   ` kbuild test robot
2020-01-13 22:19   ` Darrick J. Wong
2020-01-14  0:35     ` Ira Weiny
2020-01-15  0:57       ` Ira Weiny
2020-01-15 23:52     ` Ira Weiny
2020-01-16  9:24   ` Jan Kara
2020-01-16 19:12     ` Ira Weiny
2020-01-10 19:29 ` [RFC PATCH V2 09/12] fs: Prevent mode change if file is mmap'ed ira.weiny
2020-01-13 22:22   ` Darrick J. Wong
2020-01-14  0:46     ` Ira Weiny
2020-01-14  1:30       ` Darrick J. Wong
2020-01-14 17:53         ` Ira Weiny
2020-01-15 11:34           ` Jan Kara
2020-01-15 18:24             ` Ira Weiny
2020-01-15 10:21   ` David Laight
2020-01-15 17:53     ` Ira Weiny
2020-01-10 19:29 ` [RFC PATCH V2 10/12] fs/xfs: Fix truncate up ira.weiny
2020-01-13 22:27   ` Darrick J. Wong
2020-01-14  0:40     ` Ira Weiny
2020-01-14  1:14       ` Darrick J. Wong
2020-01-14 19:00         ` Ira Weiny
2020-01-14 19:39           ` Ira Weiny [this message]
2020-01-10 19:29 ` [RFC PATCH V2 11/12] fs/xfs: Clean up locking in dax invalidate ira.weiny
2020-01-10 19:29 ` [RFC PATCH V2 12/12] fs/xfs: Allow toggle of effective DAX flag ira.weiny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200114193934.GA23311@iweiny-DESK2.sc.intel.com \
    --to=ira.weiny@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.