Re: [LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Petr Vorel <pvorel@suse.cz>]

> Hi Cyril,

> > Hi!
> > >  	is_swap_supported(TEST_FILE);
> > > +
> > > +	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
> > > +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > > +	*swapfiles = 0;
> > > +
> > >  	setup_swap();
> > >  }

> > >  static void cleanup(void)
> > >  {
> > > -	clean_swap();
> > > +	if (swapfiles) {
> > > +		clean_swap();
> > > +		SAFE_MUNMAP(swapfiles, sizeof(*swapfiles));
> > > +	}
> > >  }

> First, thank you for your review!

> > This gets complicated for no good reason since we run the setup code in the
> > child.

> If swapfiles is not mapped for some reason (can happen or "*swapfiles = 0" is
> not run (that should not happen), then using *swapfiles leads to segfault.

... but I'm not happy with the resulting code either. Therefore I'll send
another version where I cleanup unneeded fork() leftover from previous
implementation as you suggested [1]. That will simplify test a bit
(and mmap() will not be needed).

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/aRM0Z-IlZSLdB5ho@yuki.lan/


> *swapfiles is being used not only in SAFE_MUNMAP() here in cleanup(), but
> also in clean_swap().

> Also, this saves trying to deactivate swap files if they weren't activated.

> And, unless "swapon03: Remove grep dependency" (the last commit) is accepted,
> there is a corner case I found during debugging:

...

Kind regards,
Petr

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