Re: [LTP] [PATCH] Add hugemmap41(Migrating the libhugetlbfs/testcases/slbpacaflush.c test)

Shirisha ganta
2024-04-16T08:12:11Z

On Mon, 2024-02-19 at 23:18 +0100, Petr Vorel wrote:
> Hi Shirisha,
>
> > +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> > +{
> > + char cpu_state, path_buf[64];
> > + int total_cpus, cpu_idx, fd, ret, i;
> > +
> > + total_cpus = get_nprocs_conf();
> > + cpu_idx = 0;
> > +
> > + if (get_nprocs() < nr_cpus_needed)
> > + tst_res(TFAIL, "Atleast online %d cpus are required",
> > nr_cpus_needed);
> s/Atleast/At least/
> (or better: "minimum %d online cpus" ..., but I'm not a native
> speaker.)
will take care in V2
>
> Also test should exit when not enough online CPU, right? (it's a test
> requirement not a subject of testing). Therefore tst_brk(TCONF, ...)
> should be
> used instead of tst_res(TFAIL).
will take care in V2
>
> And shouldn't this be a setup function (i.e. run only once, before
> test starts -
> test itself can be run more times, eg 3x with -i3).
I think not because of successive iterations the online cpus state
might change
>
> > +
> > + for (i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
> > + errno = 0;
> > + sprintf(path_buf, SYSFS_CPU_ONLINE_FMT, i);
> > + fd = open(path_buf, O_RDONLY);
> > + if (fd < 0) {
> > + /* If 'online' is absent, the cpu cannot be
> > offlined */
> > + if (errno == ENOENT) {
> > + online_cpus[cpu_idx] = i;
> > + cpu_idx++;
> > + continue;
> > + } else {
> > + tst_res(TFAIL, "Unable to open %s: %s",
> > path_buf,
> > + strerror(errno));
> > + }
> > + }
> > +
> > + ret = read(fd, &cpu_state, 1);
> > + if (ret < 1)
> > + tst_res(TFAIL, "Unable to read %s: %s",
> > path_buf,
> > + strerror(errno));
> > +
> > + if (cpu_state == '1') {
> > + online_cpus[cpu_idx] = i;
> > + cpu_idx++;
> > + }
> > +
> > + if (fd >= 0)
> > + SAFE_CLOSE(fd);
> > + }
> > +
> > + if (cpu_idx < nr_cpus_needed)
> > + tst_res(TFAIL, "Atleast %d online cpus were not found",
> > nr_cpus_needed);
> Also here tst_brk(TCONF, ...)
will take care in V2
> > +}
>
> Kind regards,
> Petr