Re: [LTP] [PATCH v1] libswap.c: Improve caculate swap dev number

[Petr Vorel <pvorel@suse.cz>]

> On Mon, Mar 04, 2024 at 06:04:08PM +0800, Li Wang wrote:
> > On Mon, Mar 4, 2024 at 5:19 PM Yang Xu (Fujitsu) <xuyang2018.jy@fujitsu.com>
> > wrote:

> > > Hi  Li，



> > > > On Sun, Mar 3, 2024 at 9:14 PM Petr Vorel <pvorel@suse.cz
> > > > <mailto:pvorel@suse.cz>> wrote:

> > > >     Hi Wei, Xu,

> > > >      > Hi  Wei

> > > >      > > I encounter a dead loop on following code in our test on ppc64
> > > >     machine:
> > > >      > > while ((c = fgetc(fp)) != EOF)

> > > >      > > I think "/proc/swaps" is not normal file and can not get EOF in
> > > >     some situation,
> > > >      > > so i use fgets a line and caculate swap dev number.

> > > >     I guess the problem has been fixed by another patch [1], thus
> > > >     closing it in
> > > >     patchwork. Please let me know if not.


> > > > Seems not, the patch [1] below is mainly to count the free disk size.

> > > > But Wei's work here is to calculate swap-dev numbers correctly
> > > > (especially get rid of EOF affection to some degree).

> > > > Xu Yang, what do you think? or did I miss anything here?



> > > I still think we can use the same way in ipc libs to get rid of the EOF
> > > problem instead of
> > > searching "dev" keyword. We just don't need to  calcualte "/proc/swaps"
> > > header.


> > Sounds good to me. At least it counts right lines.

> > It'd be great to have a patch by that way.




> > > int get_used_sysvipc(const char *file, const int lineno, const char
> > > *sysvipc_file)
> > > {
> > >         FILE *fp;
> > >         int used = -1;
> > >         char buf[BUFSIZE];

> > >         fp = safe_fopen(file, lineno, NULL, sysvipc_file, "r");

> > >         while (fgets(buf, BUFSIZE, fp) != NULL)
> > >                 used++;

> > >         fclose(fp);

> > >         if (used < 0) {
> > >                 tst_brk(TBROK, "can't read %s to get used sysvipc resource
> > > total at "
> > >                         "%s:%d", sysvipc_file, file, lineno);
> > >         }

> > >         return used;
> > > }

> > > But we don't get the actual reason, I still wonder why this deadloop
> > > exists o ppcc64 instead of
> > > other architecture(ie x86_64).

FYI also other architectures fails (eg.g least aarch64, s390x and ppc64le which
was in the original report; it looks to me it works on x86_64).

Problem is somewhere in
c1b8c011e447088b08787462e0c2ed50cd8c43f3..8fd941649afeecc5f87508c9f94e9a840a84e44d
which contains 319693d0b ("libltpswap: alter tst_count_swaps API"), which is Wei
trying to fix.

> > Ok, I think we can just apply your suggested method to see if that deadloop
> > disappears.

> Hi Petr, Xu, Li 
> Deadloop will disappear if you use fgets, fgetc can not get EOF on ppc64.(Suspect an bug)
> So either use my patch or Xu's suggestion both can work.

I would vote for Xu's suggestion (any line except the first header is valid),
actually this implementation is not correct when it counts only /dev:

# dd if=/dev/zero of=/root/swap bs=100M count=1
# mkswap -f /root/swap
# swapon /root/swap

# cat /proc/swaps
Filename				Type		Size		Used		Priority
/dev/dm-0                               partition	192508		4048		-2
/root/swap                              file		102396		0		-3

(second line without /dev)

Also typo in the subject: s/caculate/calculate/

Wei, please send new version and Cc us.

Kind regards,
Petr

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