Re: [PATCH 4/5] common/rc: fix scratch mount options for tmpfs

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Jun 14, 2024 at 08:47:28AM GMT, Darrick J. Wong wrote:
> On Fri, Jun 14, 2024 at 06:17:25AM +0000, Daniel Gomez wrote:
> > Make sure tmpfs scratch device inherits the extra tmpfs mount options
> > variable (TMPFS_MOUNT_OPTIONS).
> > 
> > Signed-off-by: Daniel Gomez <da.gomez@xxxxxxxxxxx>
> > ---
> >  common/rc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index 7f995b0fa..a42792c37 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -224,7 +224,7 @@ _test_options()
> >  # hosted on $SCRATCH_DEV, so can't use external scratch devices).
> >  _common_dev_mount_options()
> >  {
> > -	echo $MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS $*
> > +	echo $MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS $TMPFS_MOUNT_OPTIONS $*
> 
> Why is it necessary to include tmpfs mount options for all fs types?
> XFS does not accept tmpfs mount options.

You are right. We should not do this.

However, _scratch_mount_options() calls _common_dev_mount_options() ignoring the
specific mount options based on fstyp. Replacing it with _common_mount_opts()
makes this work. In addition, _common_dev_mount_options() function description
says 'Used for mounting non-scratch devices with the safe set of scratch mount
options'. So, why is it used to mount scratch devices?

This fixes the issue:

diff --git a/common/rc b/common/rc
index 51827119c..627dbaaaa 100644
--- a/common/rc
+++ b/common/rc
@@ -231,8 +231,8 @@ _scratch_mount_options()
 {
        _scratch_options mount

-       echo `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
-                                       $SCRATCH_DEV $SCRATCH_MNT
+       echo `_common_mount_opts` $SCRATCH_OPTIONS \
+                                       $SCRATCH_DEV $SCRATCH_MNT $*
 }

> 
> For that matter, why is TMPFS_MOUNT_OPTIONS necessary at all?  Shouldn't
> that simply be "MOUNT_OPTIONS=<stuff> FSTYP=tmpfs ./check" ?

TMPFS_MOUNT_OPTIONS is used to specify mount options in each section of the
configuration file. For example, the following snippet is part of my conf file:

```
[tmpfs_noswap_huge_always]
TMPFS_MOUNT_OPTIONS="-o noswap,huge=always"

[tmpfs_noswap_huge_within_size]
TMPFS_MOUNT_OPTIONS="-o noswap,huge=within_size"
```

Then I run -s option to switch between profiles, e.g., 'check -s
tmpfs_noswap_huge_within_size -R xunit -g auto'.

I’m not sure if this addresses your question. Please let me know if I
misunderstood.

> 
> --D
> 
> >  }
> >  
> >  _scratch_mount_options()
> > -- 
> > 2.43.0
> > 




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux