[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH V2] Switch from select() to poll() in xenconsoled's IO loop.
On 04/01/13 16:38, Wei Liu wrote:
On Fri, 2013-01-04 at 16:08 +0000, Ian Campbell wrote:
+static int initialize_pollfd_arrays(void)
+{
+ fds = (struct pollfd *)
+ malloc(sizeof(struct pollfd) * DEFAULT_ARRAY_SIZE);
+ if (!fds)
+ goto fail;
+ fd_to_pollfd = (struct pollfd **)
+ malloc(sizeof(struct pollfd *) * DEFAULT_ARRAY_SIZE);
I believe it's considered "bad" to cast the result of malloc... Unless
you are using a C++ compiler to compile C - in which case it's
necessary. But then it should really be converted to "new" and "delete"
where relevant. [Although that does cause problems with realloc!]
I do notice you are not casting realloc, so I expect it's safe to remove
the casts here too.
Of course, if you remove these calls to malloc and just use realloc in
the first place, you can ignore this comment! ;)
+ if (!fd_to_pollfd)
+ goto fail;
+ memset(fds, 0, sizeof(struct pollfd) * DEFAULT_ARRAY_SIZE);
+ memset(fd_to_pollfd, 0, sizeof(struct pollfd *) * DEFAULT_ARRAY_SIZE);
+ current_array_size = DEFAULT_ARRAY_SIZE;
+ return 0;
+fail:
+ free(fds);
+ free(fd_to_pollfd);
+ return -ENOMEM;
+}
+
+static void destroy_pollfd_arrays(void)
+{
+ free(fds);
+ free(fd_to_pollfd);
+ current_array_size = 0;
+}
+
+static void set_fds(int fd, short events)
+{
+ if (current_array_size < fd+1) {
+ struct pollfd *p1 = NULL;
+ struct pollfd **p2 = NULL;
+ unsigned int newsize = current_array_size;
+
+ do { newsize += GROWTH_LENGTH; } while (newsize < fd+1);
Steal #define ROUNDUP from tools/libxc/xc_linux_osdep.c.
+
+ p1 = realloc(fds, sizeof(struct pollfd)*newsize);
+ if (!p1)
+ goto fail;
+ fds = p1;
+
+ p2 = realloc(fd_to_pollfd, sizeof(struct pollfd *)*newsize);
realloc(NULL, ...) is the same as malloc() so I think you can initialise
current_array_size to 0 and the various pointers to NULL and avoid the
need for initialize_pollfd_arrays -- i.e. it will just grow from 0 on
the first use here.
Huh, good idea.
for (;;) {
struct domain *d, *n;
- int max_fd = -1;
- struct timeval timeout;
+ int poll_timeout; /* timeout in milliseconds */
struct timespec ts;
long long now, next_timeout = 0;
- FD_ZERO(&readfds);
- FD_ZERO(&writefds);
+ reset_fds();
- FD_SET(xs_fileno(xs), &readfds);
- max_fd = MAX(xs_fileno(xs), max_fd);
+ set_fds(xs_fileno(xs), POLLIN);
Do you know, or can you track, the maximum fd over time?
If you can then you could likely make use of automatic stack allocations
(struct pollfd fds[max_fd]) and therefore avoid the pain of manual
memory management.
Stack size is subject to system setting. Typically it is limited to 8MB
in Linux as shown by `ulimit -s`. This is actually very small compared
to heap space.
Not to mention that if you run out of heapspace, you can "do something
about it" (even if it's just print an error message and exit, it's
better than "Stack overflow crash with no message at all").
I prefer allocations for that reason.
On the other hand, the number of fds we can fit in 8MB (or 4MB) is
probably more than we'd ever get from running many guests with
consoles.... And it wouldn't be hard to add a "set stack size to 16MB"
or something like that as part of "start xenconsoled".
--
Mats
Of course we can make xenconsoled special among all the processes... But
that's not ideal.
Wei.
Not sure what the semantics of those are inside a for loop where max_fd
can change but worst case you could put the content of the loop into a
function.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
