| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 25 Oct 2023 15:15:10 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Nick Terrell <terrelln@...com>,
Kan Liang <kan.liang@...ux.intel.com>,
Andi Kleen <ak@...ux.intel.com>, Leo Yan <leo.yan@...aro.org>,
Song Liu <song@...nel.org>,
Sandipan Das <sandipan.das@....com>,
James Clark <james.clark@....com>,
Anshuman Khandual <anshuman.khandual@....com>,
Miguel Ojeda <ojeda@...nel.org>,
Liam Howlett <liam.howlett@...cle.com>,
Yang Jihong <yangjihong1@...wei.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
Kajol Jain <kjain@...ux.ibm.com>,
K Prateek Nayak <kprateek.nayak@....com>,
Sean Christopherson <seanjc@...gle.com>,
Yanteng Si <siyanteng@...ngson.cn>,
Ravi Bangoria <ravi.bangoria@....com>,
German Gomez <german.gomez@....com>,
Changbin Du <changbin.du@...wei.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
liuwenyu <liuwenyu7@...wei.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v3 18/50] tools lib api: Add io_dir an allocation free
readdir alternative
On Wed, Oct 25, 2023 at 11:43 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Tue, Oct 24, 2023 at 3:24 PM Ian Rogers <irogers@...gle.com> wrote:
> >
> > glibc's opendir allocates a minimum of 32kb, when called recursively
> > for a directory tree the memory consumption can add up - nearly 300kb
> > during perf start-up when processing modules. Add a stack allocated
> > variant of readdir sized a little more than 1kb.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/lib/api/Makefile | 2 +-
> > tools/lib/api/io_dir.h | 72 ++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 73 insertions(+), 1 deletion(-)
> > create mode 100644 tools/lib/api/io_dir.h
> >
> > diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
> > index 044860ac1ed1..186aa407de8c 100644
> > --- a/tools/lib/api/Makefile
> > +++ b/tools/lib/api/Makefile
> > @@ -99,7 +99,7 @@ install_lib: $(LIBFILE)
> > $(call do_install_mkdir,$(libdir_SQ)); \
> > cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
> >
> > -HDRS := cpu.h debug.h io.h
> > +HDRS := cpu.h debug.h io.h io_dir.h
> > FD_HDRS := fd/array.h
> > FS_HDRS := fs/fs.h fs/tracing_path.h
> > INSTALL_HDRS_PFX := $(DESTDIR)$(prefix)/include/api
> > diff --git a/tools/lib/api/io_dir.h b/tools/lib/api/io_dir.h
> > new file mode 100644
> > index 000000000000..8a70c0718df5
> > --- /dev/null
> > +++ b/tools/lib/api/io_dir.h
> > @@ -0,0 +1,72 @@
> > +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> > +/*
> > + * Lightweight directory reading library.
> > + */
> > +#ifndef __API_IO_DIR__
> > +#define __API_IO_DIR__
> > +
> > +#include <dirent.h>
> > +#include <fcntl.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <sys/stat.h>
> > +
> > +struct io_dirent64 {
> > + ino64_t d_ino; /* 64-bit inode number */
> > + off64_t d_off; /* 64-bit offset to next structure */
> > + unsigned short d_reclen; /* Size of this dirent */
> > + unsigned char d_type; /* File type */
> > + char d_name[NAME_MAX + 1]; /* Filename (null-terminated) */
> > +};
> > +
> > +struct io_dir {
> > + int dirfd;
> > + ssize_t available_bytes;
> > + struct io_dirent64 *next;
> > + struct io_dirent64 buff[4];
> > +};
> > +
> > +static inline void io_dir__init(struct io_dir *iod, int dirfd)
> > +{
> > + iod->dirfd = dirfd;
> > + iod->available_bytes = 0;
> > +}
> > +
> > +static inline void io_dir__rewinddir(struct io_dir *iod)
> > +{
> > + lseek(iod->dirfd, 0, SEEK_SET);
> > + iod->available_bytes = 0;
> > +}
> > +
> > +static inline struct io_dirent64 *io_dir__readdir(struct io_dir *iod)
> > +{
> > + struct io_dirent64 *entry;
> > +
> > + if (iod->available_bytes <= 0) {
> > + ssize_t rc = getdents64(iod->dirfd, iod->buff, sizeof(iod->buff));
> > +
> > + if (rc <= 0)
> > + return NULL;
> > + iod->available_bytes = rc;
> > + iod->next = iod->buff;
> > + }
> > + entry = iod->next;
> > + iod->next = (struct io_dirent64 *)((char *)entry + entry->d_reclen);
>
> Any chance of unaligned access?
It's a funny one. io_dirent64 has a fixed size but then the reclen can
be within that size as 256 bytes may not be used for the name. As the
name is chars then unalignment is possible, but libc doesn't worry
about unaligned access, so my take is the kernel is padding the reclen
to always be 8 byte aligned.
>
> > + iod->available_bytes -= entry->d_reclen;
> > + return entry;
> > +}
> > +
> > +static inline bool io_dir__is_dir(const struct io_dir *iod, const struct io_dirent64 *dent)
> > +{
> > + if (dent->d_type == DT_UNKNOWN) {
> > + struct stat st;
> > +
> > + if (fstatat(iod->dirfd, dent->d_name, &st, /*flags=*/0))
> > + return false;
> > +
> > + return S_ISDIR(st.st_mode);
>
> You may want to save the type if it's a DIR.
Sure. Fwiw, we shouldn't take this code path as the DT_UNKNOWN is only
used by some fairly obscure file systems.
Thanks,
Ian
> Thanks,
> Namhyung
>
>
> > + }
> > + return dent->d_type == DT_DIR;
> > +}
> > +
> > +#endif
> > --
> > 2.42.0.758.gaed0368e0e-goog
> >
Powered by blists - more mailing lists