From: Cyril Hrubis <chrubis@suse.cz>
To: Petr Vorel <pvorel@suse.cz>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [RFC][PATCH 0/2] Add safe_get_nodemap()
Date: Wed, 2 Apr 2025 12:14:28 +0200 [thread overview]
Message-ID: <Z-0OBM_ta3VesFGV@yuki.lan> (raw)
In-Reply-To: <20250331173102.GA268097@pevik>
Hi!
> +1, thanks for suggesting a proper struct :).
>
> BTW why we can use enum tst_numa_types in struct and not as function parameter?
Lazines I suppose.
> -struct tst_nodemap *tst_get_nodemap(int type, size_t min_mem_kb);
> +struct tst_nodemap *tst_get_nodemap(enum tst_numa_types type, size_t min_mem_kb);
>
> Old toolchains? Maybe not needed nowadays? I guess with using enum we could get
> rid of the check for enum validity:
> if (type & ~(TST_NUMA_MEM))
> tst_brk(TBROK, "Invalid type %i\n", type);
No we couldn't because you can stil pass whatever you want even when the
type is enum.
> > And then use that in tst_test structure. That would certainly make
> > sense, but I guess that we would have to move the numa library to the
> > lib/ as well. I'm not sure that we can have a function call from
> > tst_test.c library to something that is not compiled in by default.
>
> Correct, file is in libs/numa/tst_numa.c. But how about use predecessor check?
>
> if (tst_test->numa) {
> #ifdef HAVE_NUMA_V2
> ...
> #else
> tst_brk(TCONF, NUMA_ERROR_MSG);
> #endif
> }
>
> With help we would even get rid of else part of the compile check in the
> tests:
>
> #else
> TST_TEST_TCONF(NUMA_ERROR_MSG);
>
> But we would have modify lib/Makefile to conditionally add "-lnuma -lltpnuma".
> "-lnuma" would be based on NUMA_LIBS (stored in include/mk/config.mk, detected
> in m4/ltp-numa.m4), e.g. something like this could work:
>
> ifneq ($(NUMA_LIBS),)
> LDLIBS += $(NUMA_LIBS) -lltpnuma
> endif
I do no think that this would be enough, because all tests are linking
againts -lltp and if -lltp potentionally calls into the libnuma suddenly
all tests has to be linked againts libnuma. Or libnuma must be moved
into the ltp library.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
prev parent reply other threads:[~2025-04-02 10:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-28 11:43 [LTP] [RFC][PATCH 0/2] Add safe_get_nodemap() Petr Vorel
2025-03-28 11:43 ` [LTP] [RFC][PATCH 1/2] libs: " Petr Vorel
2025-03-31 16:00 ` Cyril Hrubis
2025-03-31 17:55 ` Petr Vorel
2025-03-28 11:43 ` [LTP] [RFC][PATCH 2/2] Use safe_get_nodemap() Petr Vorel
2025-03-31 16:06 ` Cyril Hrubis
2025-03-31 15:52 ` [LTP] [RFC][PATCH 0/2] Add safe_get_nodemap() Cyril Hrubis
2025-03-31 17:31 ` Petr Vorel
2025-04-02 10:14 ` Cyril Hrubis [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z-0OBM_ta3VesFGV@yuki.lan \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
--cc=pvorel@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox