lkml.org 
[lkml]   [2020]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7 01/13] tools/libperf: introduce notion of static polled file descriptors
    From
    Date

    On 05.06.2020 14:38, Jiri Olsa wrote:
    > 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@linux.intel.com>
    >>> ---
    >>> 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

    and that is why the return value of fdarray__add() should be converted
    to bool (added/not added). Currently the return value is used as bool
    only allover the calling code.

    fdarray__add_fixed() brings the notion of fd with fixed pos which is
    valid after fdarray__add_fixed() call so the pos could be used to access
    pos fd poll status after poll() call.

    pos = fdarray__add_fixed(array, fd);
    fdarray_poll(array);
    revents = fdarray_fixed_revents(array, pos);
    fdarray__del(array, pos);

    or fdarray__add() could be extended with fixed attribute to avoid separate call:
    int fdarray__add(struct fdarray *fda, int fd, short revents, bool fixed)

    ~Alexey

    >>
    >> 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;
    >

    \
     
     \ /
      Last update: 2020-06-05 18:17    [W:2.498 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site