All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] lib: limit the size of tmpfs in LTP
Date: Fri, 2 Jul 2021 11:21:00 +0200	[thread overview]
Message-ID: <YN7afAs3Mup5UbIf@yuki> (raw)
In-Reply-To: <20210701055208.715395-1-liwang@redhat.com>

Hi!
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  include/tst_device.h |  7 +++++++
>  lib/tst_device.c     | 15 +++++++++++++++
>  lib/tst_test.c       | 10 ++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 1d1246e82..51bde4190 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -31,6 +31,13 @@ int tst_umount(const char *path);
>  int tst_is_mounted(const char *path);
>  int tst_is_mounted_at_tmpdir(const char *path);
>  
> +/*
> + * Limit the tmpfs mount size for LTP test
> + * @mnt_data: mount options from tst_test->mnt_data
> + * @size: tmpfs size to be mounted
> + */
> +char *limit_tmpfs_mount_size(const char *mnt_data, unsigned int size);

If we want this function to be public it has to be prefixed with 'tst_'.

Also do we really need this to be public?

>  /*
>   * Clears a first few blocks of the device. This is needed when device has
>   * already been formatted with a filesystems, subset of mkfs.foo utils aborts
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index c096b418b..66a830b7b 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -45,6 +45,7 @@
>  #define DEV_FILE "test_dev.img"
>  #define DEV_SIZE_MB 256u
>  
> +static char tmpfs_buf[1024];

Can we please, instead of adding a global variable, pass the buffer and
it's size to the limit_tmpfs_mount size, and then create the path on the
stack in the prepare device function?

>  static char dev_path[1024];
>  static int device_acquired;
>  static unsigned long prev_dev_sec_write;
> @@ -368,6 +369,20 @@ int tst_clear_device(const char *dev)
>  	return 0;
>  }
>  
> +char *limit_tmpfs_mount_size(const char *mnt_data, unsigned int size)
> +{
> +	unsigned int dev_size = MAX(size, DEV_SIZE_MB);
> +
> +	if (mnt_data)
> +		snprintf(tmpfs_buf, sizeof(tmpfs_buf), "%s,size=%uM", mnt_data, dev_size);
> +	else
> +		snprintf(tmpfs_buf, sizeof(tmpfs_buf), "size=%uM", dev_size);
> +
> +	tst_resm(TINFO, "Limiting tmpfs size to %uMB", dev_size);
> +
> +	return tmpfs_buf;
> +}

If we passed the filesystem type to this function here as well we could
do:

	if (!strcmp(fs_type, "tmpfs"))
		return mnt_data;

As a first thing in this function and then we could pass the return
value from this function to the SAFE_MOUNT() unconditionally.

>  int tst_umount(const char *path)
>  {
>  	int err, ret, i;
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 55449c80b..27766fbfd 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -896,9 +896,19 @@ static void prepare_device(void)
>  	}
>  
>  	if (tst_test->mount_device) {
> +		char *mnt_data = tst_test->mnt_data;
> +
> +		if (!strcmp(tdev.fs_type, "tmpfs")) {
> +			tst_test->mnt_data = limit_tmpfs_mount_size(tst_test->mnt_data,
> +					tst_test->dev_min_size);
> +		}
> +
>  		SAFE_MOUNT(tdev.dev, tst_test->mntpoint, tdev.fs_type,
>  			   tst_test->mnt_flags, tst_test->mnt_data);
>  		mntpoint_mounted = 1;
> +
> +		if (!strcmp(tdev.fs_type, "tmpfs"))
> +			tst_test->mnt_data = mnt_data;

I guess that we are doing this in order to export the changes in the
mnt_data to the test, right?

Is that needed for something or are you doing this just in a case that
somebody will use that?

>  	}
>  }
>  
> -- 
> 2.31.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

  parent reply	other threads:[~2021-07-02  9:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01  5:52 [LTP] [PATCH 1/2] lib: limit the size of tmpfs in LTP Li Wang
2021-07-01  5:52 ` [LTP] [PATCH 2/2] lib: mount tmpfs name as ltp-tmpfs Li Wang
2021-07-02  9:23   ` Cyril Hrubis
2021-07-02  9:21 ` Cyril Hrubis [this message]
2021-07-02 12:01   ` [LTP] [PATCH 1/2] lib: limit the size of tmpfs in LTP Li Wang
2021-07-02 11:40     ` Cyril Hrubis
2021-07-02 12:20       ` Li Wang

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=YN7afAs3Mup5UbIf@yuki \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /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.