Re: [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api

[Petr Vorel <pvorel@suse.cz>]

Hi Xu,

> Also add hot_add/hot_remove in setup/cleanup, so this case can adapt the situation that
> zram module is being used by zram-generator or zram module is builtin.
Very nice. Again, I like you added both CONFIG_ZRAM=y support and simultaneous
run.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
...
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> - * 02110-1301, USA.
> + * This case check whether data read from zram device is consistent with
> + * thoese are written.
>   */

> +
nit: I'd remove this extra line
>  #include <sys/types.h>
>  #include <sys/stat.h>

...
>  	if (i != SIZE - 1) {
> -		tst_resm(TFAIL, "expect size: %ld, actual size: %ld.",
> +		tst_res(TFAIL, "expect size: %ld, actual size: %ld.",
>  			 SIZE - 1, i);
>  	} else if (s[i] != '\0') {
> -		tst_resm(TFAIL, "zram device seems not null terminated");
> +		tst_res(TFAIL, "zram device seems not null terminated");
>  	} else if (fail) {
> -		tst_resm(TFAIL, "%ld failed bytes found.", fail);
> +		tst_res(TFAIL, "%ld failed bytes found", fail);
>  	} else {
> -		tst_resm(TPASS, "data read from zram device is consistent "
> +		tst_res(TPASS, "data read from zram device is consistent "
>  			 "with those are written");
nit: I'd join this line (less than over 100)
>  	}

...
> +static void setup(void)
> +{
> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
> +	int hot_add_fd;
> +
> +	if (!access(ZRAM_CONTROL_PATH, F_OK)) {
> +		hot_add_fd = SAFE_OPEN(HOT_ADD_PATH, O_RDONLY);
> +		SAFE_READ(0, hot_add_fd, &buf, 20);
> +		dev_num = atoi(buf);
> +		SAFE_CLOSE(hot_add_fd);
> +		hot_add_flag = 1;
We have SAFE_FILE_SCANF(), you can use just:
		SAFE_FILE_SCANF(HOT_ADD_PATH, "%d", &dev_num);

> +	} else {
> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
> +		modprobe = 1;
> +	}
> +	sprintf(zram_block_path, "/sys/block/zram%d", dev_num);
> +	sprintf(zram_dev_path, "/dev/zram%d", dev_num);
> +}
> +
> +static void cleanup(void)
> +{
> +	const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
> +	int hot_remove_fd;
> +
> +	if (hot_add_flag) {
> +		hot_remove_fd = SAFE_OPEN(HOT_REMOVE_PATH, O_WRONLY);
> +		SAFE_WRITE(0, hot_remove_fd, buf, 20);
> +		SAFE_CLOSE(hot_remove_fd);
> +	}
Ad here
	if (hot_add_flag)
		SAFE_FILE_PRINTF(HOT_REMOVE_PATH, "%d", dev_num);

Kind regards,
Petr

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