Re: [PATCH v7 01/13] tools/libperf: introduce notion of static polled file descriptors
From: Jiri Olsa
Date: Fri Jun 05 2020 - 07:38:49 EST
On Fri, Jun 05, 2020 at 12:50:54PM +0200, Jiri Olsa wrote:
> On Wed, Jun 03, 2020 at 06:52:59PM +0300, Alexey Budankov wrote:
> >
> > Implement adding of file descriptors by fdarray__add_stat() to
> > fix-sized (currently 1) stat_entries array located at struct fdarray.
> > Append added file descriptors to the array used by poll() syscall
> > during fdarray__poll() call. Copy poll() result of the added
> > descriptors from the array back to the storage for analysis.
> >
> > Signed-off-by: Alexey Budankov <alexey.budankov@xxxxxxxxxxxxxxx>
> > ---
> > tools/lib/api/fd/array.c | 42 +++++++++++++++++++++++-
> > tools/lib/api/fd/array.h | 7 ++++
> > tools/lib/perf/evlist.c | 11 +++++++
> > tools/lib/perf/include/internal/evlist.h | 2 ++
> > 4 files changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> > index 58d44d5eee31..b0027f2169c7 100644
> > --- a/tools/lib/api/fd/array.c
> > +++ b/tools/lib/api/fd/array.c
> > @@ -11,10 +11,16 @@
> >
> > void fdarray__init(struct fdarray *fda, int nr_autogrow)
> > {
> > + int i;
> > +
> > fda->entries = NULL;
> > fda->priv = NULL;
> > fda->nr = fda->nr_alloc = 0;
> > fda->nr_autogrow = nr_autogrow;
> > +
> > + fda->nr_stat = 0;
> > + for (i = 0; i < FDARRAY__STAT_ENTRIES_MAX; i++)
> > + fda->stat_entries[i].fd = -1;
> > }
> >
> > int fdarray__grow(struct fdarray *fda, int nr)
> > @@ -83,6 +89,20 @@ int fdarray__add(struct fdarray *fda, int fd, short revents)
> > return pos;
> > }
> >
> > +int fdarray__add_stat(struct fdarray *fda, int fd, short revents)
> > +{
> > + int pos = fda->nr_stat;
> > +
> > + if (pos >= FDARRAY__STAT_ENTRIES_MAX)
> > + return -1;
> > +
> > + fda->stat_entries[pos].fd = fd;
> > + fda->stat_entries[pos].events = revents;
> > + fda->nr_stat++;
> > +
> > + return pos;
> > +}
> > +
> > int fdarray__filter(struct fdarray *fda, short revents,
> > void (*entry_destructor)(struct fdarray *fda, int fd, void *arg),
> > void *arg)
> > @@ -113,7 +133,27 @@ int fdarray__filter(struct fdarray *fda, short revents,
> >
> > int fdarray__poll(struct fdarray *fda, int timeout)
> > {
> > - return poll(fda->entries, fda->nr, timeout);
> > + int nr, i, pos, res;
> > +
> > + nr = fda->nr;
> > +
> > + for (i = 0; i < fda->nr_stat; i++) {
> > + if (fda->stat_entries[i].fd != -1) {
> > + pos = fdarray__add(fda, fda->stat_entries[i].fd,
> > + fda->stat_entries[i].events);
>
> so every call to fdarray__poll will add whatever is
> in stat_entries to entries? how is it removed?
>
> I think you should either follow what Adrian said
> and put 'static' descriptors early and check for
> filter number to match it as an 'quick fix'
>
> or we should fix it for real and make it generic
>
> so currently the interface is like this:
>
> pos1 = fdarray__add(a, fd1 ... );
> pos2 = fdarray__add(a, fd2 ... );
> pos3 = fdarray__add(a, fd2 ... );
>
> fdarray__poll(a);
>
> num = fdarray__filter(a, revents, destructor, arg);
>
> when fdarray__filter removes some of the fds the 'pos1,pos2,pos3'
> indexes are not relevant anymore
>
> how about we make the 'pos indexes' being stable by allocating
> separate object for each added descriptor and each poll call
> would create pollfd array from current objects, and entries
> would keep pointer to its pollfd entry
>
> struct fdentry *entry {
> int fd;
> int events;
> struct pollfd *pollfd;
> }
>
> entry1 = fdarray__add(a, fd1 ...);
> entry2 = fdarray__add(a, fd2 ...);
> entry3 = fdarray__add(a, fd3 ...);
>
> fdarray__poll(a);
>
> struct pollfd *fdarray__entry_pollfd(a, entry1);
>
> or smoething like that ;-)
maybe something like below (only compile tested)
jirka
---
diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index 58d44d5eee31..f1effed3dde1 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -22,8 +22,8 @@ int fdarray__grow(struct fdarray *fda, int nr)
void *priv;
int nr_alloc = fda->nr_alloc + nr;
size_t psize = sizeof(fda->priv[0]) * nr_alloc;
- size_t size = sizeof(struct pollfd) * nr_alloc;
- struct pollfd *entries = realloc(fda->entries, size);
+ size_t size = sizeof(struct fdentry *) * nr_alloc;
+ struct fdentry **entries = realloc(fda->entries, size);
if (entries == NULL)
return -ENOMEM;
@@ -58,7 +58,12 @@ struct fdarray *fdarray__new(int nr_alloc, int nr_autogrow)
void fdarray__exit(struct fdarray *fda)
{
+ int i;
+
+ for (i = 0; i < fda->nr; i++)
+ free(fda->entries[i]);
free(fda->entries);
+ free(fda->pollfd);
free(fda->priv);
fdarray__init(fda, 0);
}
@@ -69,18 +74,25 @@ void fdarray__delete(struct fdarray *fda)
free(fda);
}
-int fdarray__add(struct fdarray *fda, int fd, short revents)
+struct fdentry *fdarray__add(struct fdarray *fda, int fd, short revents)
{
- int pos = fda->nr;
+ struct fdentry *entry;
if (fda->nr == fda->nr_alloc &&
fdarray__grow(fda, fda->nr_autogrow) < 0)
- return -ENOMEM;
+ return NULL;
+
+ entry = malloc(sizeof(*entry));
+ if (!entry)
+ return NULL;
+
+ entry->fd = fd;
+ entry->revents = revents;
+ entry->pollfd = NULL;
- fda->entries[fda->nr].fd = fd;
- fda->entries[fda->nr].events = revents;
+ fda->entries[fda->nr] = entry;
fda->nr++;
- return pos;
+ return entry;
}
int fdarray__filter(struct fdarray *fda, short revents,
@@ -93,7 +105,7 @@ int fdarray__filter(struct fdarray *fda, short revents,
return 0;
for (fd = 0; fd < fda->nr; ++fd) {
- if (fda->entries[fd].revents & revents) {
+ if (fda->entries[fd]->revents & revents) {
if (entry_destructor)
entry_destructor(fda, fd, arg);
@@ -113,7 +125,22 @@ int fdarray__filter(struct fdarray *fda, short revents,
int fdarray__poll(struct fdarray *fda, int timeout)
{
- return poll(fda->entries, fda->nr, timeout);
+ struct pollfd *pollfd = fda->pollfd;
+ int i;
+
+ pollfd = realloc(pollfd, sizeof(*pollfd) * fda->nr);
+ if (!pollfd)
+ return -ENOMEM;
+
+ fda->pollfd = pollfd;
+
+ for (i = 0; i < fda->nr; i++) {
+ pollfd[i].fd = fda->entries[i]->fd;
+ pollfd[i].revents = fda->entries[i]->revents;
+ fda->entries[i]->pollfd = &pollfd[i];
+ }
+
+ return poll(pollfd, fda->nr, timeout);
}
int fdarray__fprintf(struct fdarray *fda, FILE *fp)
@@ -121,7 +148,12 @@ int fdarray__fprintf(struct fdarray *fda, FILE *fp)
int fd, printed = fprintf(fp, "%d [ ", fda->nr);
for (fd = 0; fd < fda->nr; ++fd)
- printed += fprintf(fp, "%s%d", fd ? ", " : "", fda->entries[fd].fd);
+ printed += fprintf(fp, "%s%d", fd ? ", " : "", fda->entries[fd]->fd);
return printed + fprintf(fp, " ]");
}
+
+int fdentry__events(struct fdentry *entry)
+{
+ return entry->pollfd->revents;
+}
diff --git a/tools/lib/api/fd/array.h b/tools/lib/api/fd/array.h
index b39557d1a88f..5231ce047f2e 100644
--- a/tools/lib/api/fd/array.h
+++ b/tools/lib/api/fd/array.h
@@ -6,6 +6,12 @@
struct pollfd;
+struct fdentry {
+ int fd;
+ int revents;
+ struct pollfd *pollfd;
+};
+
/**
* struct fdarray: Array of file descriptors
*
@@ -20,7 +26,10 @@ struct fdarray {
int nr;
int nr_alloc;
int nr_autogrow;
- struct pollfd *entries;
+
+ struct fdentry **entries;
+ struct pollfd *pollfd;
+
union {
int idx;
void *ptr;
@@ -33,7 +42,7 @@ void fdarray__exit(struct fdarray *fda);
struct fdarray *fdarray__new(int nr_alloc, int nr_autogrow);
void fdarray__delete(struct fdarray *fda);
-int fdarray__add(struct fdarray *fda, int fd, short revents);
+struct fdentry *fdarray__add(struct fdarray *fda, int fd, short revents);
int fdarray__poll(struct fdarray *fda, int timeout);
int fdarray__filter(struct fdarray *fda, short revents,
void (*entry_destructor)(struct fdarray *fda, int fd, void *arg),
@@ -41,6 +50,8 @@ int fdarray__filter(struct fdarray *fda, short revents,
int fdarray__grow(struct fdarray *fda, int extra);
int fdarray__fprintf(struct fdarray *fda, FILE *fp);
+int fdentry__events(struct fdentry *entry);
+
static inline int fdarray__available_entries(struct fdarray *fda)
{
return fda->nr_alloc - fda->nr;