| Message ID | 20200603140115.30392-1-yangx.jy@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX | expand |
Oh, this wasn't the cover letter. ;) On Wed, Jun 03, 2020 at 10:01:12PM +0800, Xiao Yang wrote: > 1) _require_scratch_dax_mountopt() checks both old and new DAX mount option > 2) _require_scratch_daX_iflag() checks FS_XFLAG_DAX > > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > --- > common/rc | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/common/rc b/common/rc > index a6967831..ec7c19e4 100644 > --- a/common/rc > +++ b/common/rc > @@ -3188,6 +3188,41 @@ _require_scratch_dax() > _scratch_unmount > } > > +_require_scratch_dax_mountopt() > +{ > + local mountopt=$1 > + local output > + > + _require_scratch > + _scratch_mkfs > /dev/null 2>&1 > + _try_scratch_mount -o "$mountopt" || \ > + _notrun "mount $SCRATCH_DEV with $mountopt failed" What happens if MOUNT_OPTS already contains a dax option? Should we clear it out ala _qmount_option, on the assumption that a test that cares about specific options probably wants to override whatever the test runner passed in? --D > + > + output=$(_fs_options $SCRATCH_DEV) > + > + # For new dax mount option, /proc/mounts shows different outputs if we > + # mount with -o dax=inode on ext4 and xfs so skip checking it. > + # /proc/mounts shows 'dax=inode' on ext4 but shows nothing on xfs. > + if [ "$mountopt" != "dax=inode" ]; then > + echo $output | grep -qw "$mountopt" || \ > + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > + fi > + > + # For new dax mount option, /proc/mounts shows "dax=never" if we > + # mount with -o dax on xfs and underlying device doesn't support dax. > + if [ "$mountopt" = "dax" ]; then > + echo $output | grep -qw "dax=never" && \ > + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > + fi > + > + _scratch_unmount > +} > + > +_require_scratch_dax_iflag() > +{ > + _require_xfs_io_command "chattr" "x" > +} > + > # Does norecovery support by this fs? > _require_norecovery() > { > -- > 2.21.0 > > >
Hi Darrick, Thanks a lot for your comments. On 2020/6/10 0:39, Darrick J. Wong wrote: > Oh, this wasn't the cover letter. ;) I will add the cover letter. > > On Wed, Jun 03, 2020 at 10:01:12PM +0800, Xiao Yang wrote: >> 1) _require_scratch_dax_mountopt() checks both old and new DAX mount option >> 2) _require_scratch_daX_iflag() checks FS_XFLAG_DAX >> >> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com> >> --- >> common/rc | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/common/rc b/common/rc >> index a6967831..ec7c19e4 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -3188,6 +3188,41 @@ _require_scratch_dax() >> _scratch_unmount >> } >> >> +_require_scratch_dax_mountopt() >> +{ >> + local mountopt=$1 >> + local output >> + >> + _require_scratch >> + _scratch_mkfs> /dev/null 2>&1 >> + _try_scratch_mount -o "$mountopt" || \ >> + _notrun "mount $SCRATCH_DEV with $mountopt failed" > > What happens if MOUNT_OPTS already contains a dax option? Should we > clear it out ala _qmount_option, on the assumption that a test that > cares about specific options probably wants to override whatever the > test runner passed in? Good point, but it seems that the last dax option is actually used if we mount with multiple dax options, as below: ---------------------------------------------- ext4: # blkid /dev/pmem1 /dev/pmem1: UUID="cd2eb9f0-af2a-4c89-a381-4d2d9d2e8054" TYPE="ext4" # mount -o dax -odax=inode /dev/pmem1 /mnt/xfstests/scratch/ # mount | grep pmem1 /dev/pmem1 on /mnt/xfstests/scratch type ext4 (rw,relatime,seclabel,dax=inode) # mount -o dax=never -odax=inode -odax=always /dev/pmem1 /mnt/xfstests/scratch/ # mount | grep pmem1 /dev/pmem1 on /mnt/xfstests/scratch type ext4 (rw,relatime,seclabel,dax=always) # mount -o dax=never -odax /dev/pmem1 /mnt/xfstests/scratch/ # mount | grep pmem1 /dev/pmem1 on /mnt/xfstests/scratch type ext4 (rw,relatime,seclabel,dax=always) xfs: # blkid /dev/pmem0 /dev/pmem0: UUID="bc830790-1ea8-48fb-9cda-7d5bb96b8961" TYPE="xfs" # mount -o dax=never -o dax=always /dev/pmem0 /mnt/xfstests/test/ # mount | grep pmem0 /dev/pmem0 on /mnt/xfstests/test type xfs (rw,relatime,seclabel,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota) # mount -o dax=never -o dax=inode /dev/pmem0 /mnt/xfstests/test/ # mount | grep pmem0 /dev/pmem0 on /mnt/xfstests/test type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota) # mount -o dax=always -o dax=inode -o dax=never /dev/pmem0 /mnt/xfstests/test/ # mount | grep pmem0 /dev/pmem0 on /mnt/xfstests/test type xfs (rw,relatime,seclabel,attr2,dax=never,inode64,logbufs=8,logbsize=32k,noquota) ---------------------------------------------- all dax options are exclusive, so do you think is it necessary to implement a function as _qmount_option? Best Regards, Xiao Yang > > --D > >> + >> + output=$(_fs_options $SCRATCH_DEV) >> + >> + # For new dax mount option, /proc/mounts shows different outputs if we >> + # mount with -o dax=inode on ext4 and xfs so skip checking it. >> + # /proc/mounts shows 'dax=inode' on ext4 but shows nothing on xfs. >> + if [ "$mountopt" != "dax=inode" ]; then >> + echo $output | grep -qw "$mountopt" || \ >> + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" >> + fi >> + >> + # For new dax mount option, /proc/mounts shows "dax=never" if we >> + # mount with -o dax on xfs and underlying device doesn't support dax. >> + if [ "$mountopt" = "dax" ]; then >> + echo $output | grep -qw "dax=never"&& \ >> + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" >> + fi >> + >> + _scratch_unmount >> +} >> + >> +_require_scratch_dax_iflag() >> +{ >> + _require_xfs_io_command "chattr" "x" >> +} >> + >> # Does norecovery support by this fs? >> _require_norecovery() >> { >> -- >> 2.21.0 >> >> >> > > > . >
On Wed, Jun 10, 2020 at 10:01:58AM +0800, Xiao Yang wrote: > Hi Darrick, > > Thanks a lot for your comments. > > On 2020/6/10 0:39, Darrick J. Wong wrote: > > Oh, this wasn't the cover letter. ;) > > I will add the cover letter. > > > > > On Wed, Jun 03, 2020 at 10:01:12PM +0800, Xiao Yang wrote: > > > 1) _require_scratch_dax_mountopt() checks both old and new DAX mount option > > > 2) _require_scratch_daX_iflag() checks FS_XFLAG_DAX > > > > > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com> > > > --- > > > common/rc | 35 +++++++++++++++++++++++++++++++++++ > > > 1 file changed, 35 insertions(+) > > > > > > diff --git a/common/rc b/common/rc > > > index a6967831..ec7c19e4 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -3188,6 +3188,41 @@ _require_scratch_dax() > > > _scratch_unmount > > > } > > > > > > +_require_scratch_dax_mountopt() > > > +{ > > > + local mountopt=$1 > > > + local output > > > + > > > + _require_scratch > > > + _scratch_mkfs> /dev/null 2>&1 > > > + _try_scratch_mount -o "$mountopt" || \ > > > + _notrun "mount $SCRATCH_DEV with $mountopt failed" > > > > What happens if MOUNT_OPTS already contains a dax option? Should we > > clear it out ala _qmount_option, on the assumption that a test that > > cares about specific options probably wants to override whatever the > > test runner passed in? > > Good point, but it seems that the last dax option is actually used if we > mount with multiple dax options, as below: > ---------------------------------------------- > ext4: > # blkid /dev/pmem1 > /dev/pmem1: UUID="cd2eb9f0-af2a-4c89-a381-4d2d9d2e8054" TYPE="ext4" > # mount -o dax -odax=inode /dev/pmem1 /mnt/xfstests/scratch/ > # mount | grep pmem1 > /dev/pmem1 on /mnt/xfstests/scratch type ext4 > (rw,relatime,seclabel,dax=inode) > > # mount -o dax=never -odax=inode -odax=always /dev/pmem1 > /mnt/xfstests/scratch/ > # mount | grep pmem1 > /dev/pmem1 on /mnt/xfstests/scratch type ext4 > (rw,relatime,seclabel,dax=always) > > # mount -o dax=never -odax /dev/pmem1 /mnt/xfstests/scratch/ > # mount | grep pmem1 > /dev/pmem1 on /mnt/xfstests/scratch type ext4 > (rw,relatime,seclabel,dax=always) > > > xfs: > # blkid /dev/pmem0 > /dev/pmem0: UUID="bc830790-1ea8-48fb-9cda-7d5bb96b8961" TYPE="xfs" > # mount -o dax=never -o dax=always /dev/pmem0 /mnt/xfstests/test/ > # mount | grep pmem0 > /dev/pmem0 on /mnt/xfstests/test type xfs (rw,relatime,seclabel,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota) > > # mount -o dax=never -o dax=inode /dev/pmem0 /mnt/xfstests/test/ > # mount | grep pmem0 > /dev/pmem0 on /mnt/xfstests/test type xfs > (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota) > > # mount -o dax=always -o dax=inode -o dax=never /dev/pmem0 > /mnt/xfstests/test/ > # mount | grep pmem0 > /dev/pmem0 on /mnt/xfstests/test type xfs > (rw,relatime,seclabel,attr2,dax=never,inode64,logbufs=8,logbsize=32k,noquota) > ---------------------------------------------- > all dax options are exclusive, so do you think is it necessary to implement > a function as _qmount_option? Heh, ok, I guess we don't complain about respecified mount options, so (at least for fstests) the current behavior is ok. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > Best Regards, > Xiao Yang > > > > --D > > > > > + > > > + output=$(_fs_options $SCRATCH_DEV) > > > + > > > + # For new dax mount option, /proc/mounts shows different outputs if we > > > + # mount with -o dax=inode on ext4 and xfs so skip checking it. > > > + # /proc/mounts shows 'dax=inode' on ext4 but shows nothing on xfs. > > > + if [ "$mountopt" != "dax=inode" ]; then > > > + echo $output | grep -qw "$mountopt" || \ > > > + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > > > + fi > > > + > > > + # For new dax mount option, /proc/mounts shows "dax=never" if we > > > + # mount with -o dax on xfs and underlying device doesn't support dax. > > > + if [ "$mountopt" = "dax" ]; then > > > + echo $output | grep -qw "dax=never"&& \ > > > + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" > > > + fi > > > + > > > + _scratch_unmount > > > +} > > > + > > > +_require_scratch_dax_iflag() > > > +{ > > > + _require_xfs_io_command "chattr" "x" > > > +} > > > + > > > # Does norecovery support by this fs? > > > _require_norecovery() > > > { > > > -- > > > 2.21.0 > > > > > > > > > > > > > > > . > > > > >
diff --git a/common/rc b/common/rc index a6967831..ec7c19e4 100644 --- a/common/rc +++ b/common/rc @@ -3188,6 +3188,41 @@ _require_scratch_dax() _scratch_unmount } +_require_scratch_dax_mountopt() +{ + local mountopt=$1 + local output + + _require_scratch + _scratch_mkfs > /dev/null 2>&1 + _try_scratch_mount -o "$mountopt" || \ + _notrun "mount $SCRATCH_DEV with $mountopt failed" + + output=$(_fs_options $SCRATCH_DEV) + + # For new dax mount option, /proc/mounts shows different outputs if we + # mount with -o dax=inode on ext4 and xfs so skip checking it. + # /proc/mounts shows 'dax=inode' on ext4 but shows nothing on xfs. + if [ "$mountopt" != "dax=inode" ]; then + echo $output | grep -qw "$mountopt" || \ + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" + fi + + # For new dax mount option, /proc/mounts shows "dax=never" if we + # mount with -o dax on xfs and underlying device doesn't support dax. + if [ "$mountopt" = "dax" ]; then + echo $output | grep -qw "dax=never" && \ + _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt" + fi + + _scratch_unmount +} + +_require_scratch_dax_iflag() +{ + _require_xfs_io_command "chattr" "x" +} + # Does norecovery support by this fs? _require_norecovery() {
1) _require_scratch_dax_mountopt() checks both old and new DAX mount option 2) _require_scratch_daX_iflag() checks FS_XFLAG_DAX Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- common/rc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)