Re: [LTP] [PATCH v3 2/2] swapon03: Try to swapon() as many files until it fails

[Petr Vorel <pvorel@suse.cz>]

...
> > +/*
> > + * MAX_SWAPFILES from the internal kernel implementation is currently <23, 29>,
> > + * depending on kernel configuration (see man swapon(2). Chose small enough
> > + * value for future changes.
> > + */
> > +#define NUM_SWAP_FILES 15

> It would be slightly better if we renamed this to MIN_SWAP_FILES since
> this is the minimal number of swapfiles we expect to be able to create.

> >  #define MNTPOINT	"mntpoint"
> >  #define TEST_FILE	MNTPOINT"/testswap"

> > @@ -27,31 +37,33 @@ static int swapfiles;

> >  static int setup_swap(void)
> >  {
> > -	int j, max_swapfiles, used_swapfiles;
> > +	int used_swapfiles, expected_swapfiles;

> Here as well, s/expected_swapfiles/min_swapfiles/

> >  	char filename[FILENAME_MAX];

> > -	/* Determine how many more files are to be created */
> > -	max_swapfiles = tst_max_swapfiles();
> >  	used_swapfiles = tst_count_swaps();
> > -	swapfiles = max_swapfiles - used_swapfiles;
> > -	if (swapfiles > max_swapfiles)
> > -		swapfiles = max_swapfiles;
> > +	expected_swapfiles = NUM_SWAP_FILES - used_swapfiles;

> > -	/*create and turn on remaining swapfiles */
> > -	for (j = 0; j < swapfiles; j++) {
> > +	if (expected_swapfiles < 0)
> > +		tst_brk(TCONF, "too many used swap files (%d)", used_swapfiles);

> Do we really have to quit here? If all swap slots are already used we
> will not create any additioinal swaps and just go directly to the test.
> If there are some slot left, we will fill them.

We already have v4, which does not contain it.  But just for a record (I'm sorry
to discuss in the old version):
the original suggestion was to have at least few swapon() runs. This is not
checked in the code you suggested. Of course, the reason why swapon() fails
should be due already used swaps. But we don't check that. We have options:
* count used swaps (i.e. count lines in /proc/swaps)
* ignore the check
* require at least one swap can be created (code in v4).

Kind regards,
Petr

> The only difference is taht we shouldn't expect a failure until this
> number of swaps has been reached.

> So we should do:

> 	if (swapon(filename, 0) == -1) {
> 		if (errno == EPERM && swapfiles > min_swapfiles)
> 			break;

> 		tst_res(TFAIL | TERRNO, "swapon(%s, 0)", filename);
> 	}

> > -		/* Create the swapfile */
> > -		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j + 2);
> > -		SAFE_MAKE_SMALL_SWAPFILE(filename);
> > +	SAFE_MAKE_SMALL_SWAPFILE(TEST_FILE);
> > +	swapfiles++;

> Why do we increase the number of swapfiles here? We only created the
> file. Isn't the swapfiles counter for swaps that were enabled by
> swapon()?

> > -		/* turn on the swap file */
> > -		TST_EXP_PASS_SILENT(swapon(filename, 0));
> > -		if (!TST_PASS)
> > -			tst_brk(TFAIL, "Failed to setup swap files");
> > +	while (true) {
> > +		/* Create the swapfile */
> > +		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, swapfiles);
> > +		MAKE_SMALL_SWAPFILE(filename);
> > +
> > +		/* Quit on a first swap file over max, check for EPERM */
> > +		if (swapon(filename, 0) == -1) {
> > +			if (errno != EPERM)
> > +				tst_res(TFAIL | TERRNO, "swapon(%s, 0)", filename);
> > +			break;
> > +		}
> > +		swapfiles++;
> >  	}

> >  	tst_res(TINFO, "Successfully created %d swap files", swapfiles);
> > -	MAKE_SMALL_SWAPFILE(TEST_FILE);

> >  	return 0;
> >  }
> > @@ -61,7 +73,7 @@ static int setup_swap(void)
> >   */
> >  static int check_and_swapoff(const char *filename)
> >  {
> > -	char cmd_buffer[256];
> > +	char cmd_buffer[FILENAME_MAX+28];
> >  	int rc = -1;

> >  	snprintf(cmd_buffer, sizeof(cmd_buffer), "grep -q '%s.*file' /proc/swaps", filename);
> > @@ -82,8 +94,8 @@ static void clean_swap(void)
> >  	int j;
> >  	char filename[FILENAME_MAX];

> > -	for (j = 0; j < swapfiles; j++) {
> > -		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j + 2);
> > +	for (j = 1; j < swapfiles; j++) {
> > +		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j);
> >  		check_and_swapoff(filename);
> >  	}

> > -- 
> > 2.51.0

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp