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

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

Hi Petr
> 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
OK
>>   #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)
ok
>>   	}
>
> ...
>> +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);
Oh, yes, I forgot it...
>
>> +	} 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