On Sat, Jan 11, 2025 at 8:16 PM Zorro Lang <zlang@xxxxxxxxxx> wrote:
>
> On Fri, Dec 27, 2024 at 11:33:10AM -0800, Joanne Koong wrote:
> > Add support for reads/writes from buffers backed by hugepages.
> > This can be enabled through the '-h' flag. This flag should only be used
> > on systems where THP capabilities are enabled.
> >
> > This is motivated by a recent bug that was due to faulty handling of
> > userspace buffers backed by hugepages. This patch is a mitigation
> > against problems like this in the future.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > ---
> > ltp/fsx.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 97 insertions(+), 11 deletions(-)
> >
>
> [snip]
>
> > +static void *
> > +init_hugepages_buf(unsigned len, int hugepage_size, int alignment)
> > +{
> > + void *buf;
> > + long buf_size = roundup(len, hugepage_size) + alignment;
> > +
> > + if (posix_memalign(&buf, hugepage_size, buf_size)) {
> > + prterr("posix_memalign for buf");
> > + return NULL;
> > + }
> > + memset(buf, '\0', buf_size);
> > + if (madvise(buf, buf_size, MADV_COLLAPSE)) {
>
> Hi Joanne,
>
> Sorry I have to drop this patchset from the "upcoming" release v2025.01.12. Due to
> it cause a regression build error on older system, e.g. RHEL-9:
>
> [CC] fsx
> fsx.c: In function 'init_hugepages_buf':
> fsx.c:2935:36: error: 'MADV_COLLAPSE' undeclared (first use in this function); did you mean 'MADV_COLD'?
> 2935 | if (madvise(buf, buf_size, MADV_COLLAPSE)) {
> | ^~~~~~~~~~~~~
> | MADV_COLD
> fsx.c:2935:36: note: each undeclared identifier is reported only once for each function it appears in
> gmake[4]: *** [Makefile:51: fsx] Error 1
> gmake[4]: *** Waiting for unfinished jobs....
> gmake[3]: *** [include/buildrules:30: ltp] Error 2
>
> It might cause xfstests totally can't be used on downstream systems, so it can't
> catch up the release of this weekend. Sorry about that, let's try to have it
> in next release :)
Hi Zorro,
Thanks for the update. I'll submit a v3 of this patch that gates this
function behind #ifdef MADV_COLLAPSE, and hopefully that should fix
this issue.
Thanks,
Joanne
>
> Thanks,
> Zorro
>
>
> > + prterr("madvise collapse for buf");
> > + free(buf);
> > + return NULL;
> > + }
> > +
> > + return buf;
> > +}
> > +
> > +static void
> > +init_buffers(void)
> > +{
> > + int i;
> > +
> > + original_buf = (char *) malloc(maxfilelen);
> > + for (i = 0; i < maxfilelen; i++)
> > + original_buf[i] = random() % 256;
> > + if (hugepages) {
> > + long hugepage_size = get_hugepage_size();
> > + if (hugepage_size == -1) {
> > + prterr("get_hugepage_size()");
> > + exit(100);
> > + }
> > + good_buf = init_hugepages_buf(maxfilelen, hugepage_size, writebdy);
> > + if (!good_buf) {
> > + prterr("init_hugepages_buf failed for good_buf");
> > + exit(101);
> > + }
> > +
> > + temp_buf = init_hugepages_buf(maxoplen, hugepage_size, readbdy);
> > + if (!temp_buf) {
> > + prterr("init_hugepages_buf failed for temp_buf");
> > + exit(101);
> > + }
> > + } else {
> > + unsigned long good_buf_len = maxfilelen + writebdy;
> > + unsigned long temp_buf_len = maxoplen + readbdy;
> > +
> > + good_buf = (char *) malloc(good_buf_len);
> > + memset(good_buf, '\0', good_buf_len);
> > + temp_buf = (char *) malloc(temp_buf_len);
> > + memset(temp_buf, '\0', temp_buf_len);
> > + }
> > + good_buf = round_ptr_up(good_buf, writebdy, 0);
> > + temp_buf = round_ptr_up(temp_buf, readbdy, 0);
> > +}
> > +
> > static struct option longopts[] = {
> > {"replay-ops", required_argument, 0, 256},
> > {"record-ops", optional_argument, 0, 255},
> > @@ -2883,7 +2974,7 @@ main(int argc, char **argv)
> > setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
> >
> > while ((ch = getopt_long(argc, argv,
> > - "0b:c:de:fg:i:j:kl:m:no:p:qr:s:t:uw:xyABD:EFJKHzCILN:OP:RS:UWXZ",
> > + "0b:c:de:fg:hi:j:kl:m:no:p:qr:s:t:uw:xyABD:EFJKHzCILN:OP:RS:UWXZ",
> > longopts, NULL)) != EOF)
> > switch (ch) {
> > case 'b':
> > @@ -2916,6 +3007,9 @@ main(int argc, char **argv)
> > case 'g':
> > filldata = *optarg;
> > break;
> > + case 'h':
> > + hugepages = 1;
> > + break;
> > case 'i':
> > integrity = 1;
> > logdev = strdup(optarg);
> > @@ -3229,15 +3323,7 @@ main(int argc, char **argv)
> > exit(95);
> > }
> > }
> > - original_buf = (char *) malloc(maxfilelen);
> > - for (i = 0; i < maxfilelen; i++)
> > - original_buf[i] = random() % 256;
> > - good_buf = (char *) malloc(maxfilelen + writebdy);
> > - good_buf = round_ptr_up(good_buf, writebdy, 0);
> > - memset(good_buf, '\0', maxfilelen);
> > - temp_buf = (char *) malloc(maxoplen + readbdy);
> > - temp_buf = round_ptr_up(temp_buf, readbdy, 0);
> > - memset(temp_buf, '\0', maxoplen);
> > + init_buffers();
> > if (lite) { /* zero entire existing file */
> > ssize_t written;
> >
> > --
> > 2.47.1
> >
> >
>
