Xen project Mailing List

[xen master] tools/ocaml/xenstored: drop select based socket watching

Date: Sat, 22 Aug 2020 05:57:54 +0000

Delivery-date: Sat, 22 Aug 2020 05:57:56 +0000

List-id: "Change log for Mercurial $receive only$" <xen-changelog.lists.xenproject.org>

commit 80a868f0f6ccffee5843cbf1ad880ffcb76267a2 Author: Edwin TÃ¶rÃ¶k <edvin.torok@xxxxxxxxxx> AuthorDate: Mon Aug 17 19:45:47 2020 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Tue Aug 18 17:34:50 2020 +0100 tools/ocaml/xenstored: drop select based socket watching Poll has been the default since 2014, I think we can safely say by now that poll() works and we don't need to fall back to select(). This will allow fixing up the way we call poll to be more efficient (and pave the way for introducing epoll support): currently poll wraps the select API, which is inefficient. Signed-off-by: Edwin TÃ¶rÃ¶k <edvin.torok@xxxxxxxxxx> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx> --- tools/ocaml/xenstored/Makefile | 12 +++--- tools/ocaml/xenstored/parse_arg.ml | 7 +--- tools/ocaml/xenstored/poll.ml | 67 +++++++++++++++++++++++++++++++++ tools/ocaml/xenstored/poll.mli | 19 ++++++++++ tools/ocaml/xenstored/select.ml | 77 -------------------------------------- tools/ocaml/xenstored/select.mli | 27 ------------- tools/ocaml/xenstored/xenstored.ml | 4 +- 7 files changed, 95 insertions(+), 118 deletions(-) diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile index 68d35c483a..692a62584e 100644 --- a/tools/ocaml/xenstored/Makefile +++ b/tools/ocaml/xenstored/Makefile @@ -18,12 +18,12 @@ OCAMLINCLUDE += \ -I $(OCAML_TOPLEVEL)/libs/xc \ -I $(OCAML_TOPLEVEL)/libs/eventchn -LIBS = syslog.cma syslog.cmxa select.cma select.cmxa +LIBS = syslog.cma syslog.cmxa poll.cma poll.cmxa syslog_OBJS = syslog syslog_C_OBJS = syslog_stubs -select_OBJS = select -select_C_OBJS = select_stubs -OCAML_LIBRARY = syslog select +poll_OBJS = poll +poll_C_OBJS = select_stubs +OCAML_LIBRARY = syslog poll LIBS += systemd.cma systemd.cmxa systemd_OBJS = systemd @@ -58,13 +58,13 @@ OBJS = paths \ process \ xenstored -INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi select.cmi +INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi poll.cmi XENSTOREDLIBS = \ unix.cmxa \ -ccopt -L -ccopt . syslog.cmxa \ -ccopt -L -ccopt . systemd.cmxa \ - -ccopt -L -ccopt . select.cmxa \ + -ccopt -L -ccopt . poll.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/mmap $(OCAML_TOPLEVEL)/libs/mmap/xenmmap.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/eventchn $(OCAML_TOPLEVEL)/libs/eventchn/xeneventchn.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xc $(OCAML_TOPLEVEL)/libs/xc/xenctrl.cmxa \ diff --git a/tools/ocaml/xenstored/parse_arg.ml b/tools/ocaml/xenstored/parse_arg.ml index 1803c3eda0..2c4b5a8528 100644 --- a/tools/ocaml/xenstored/parse_arg.ml +++ b/tools/ocaml/xenstored/parse_arg.ml @@ -25,7 +25,6 @@ type config = tracefile: string option; (* old xenstored compatibility *) restart: bool; disable_socket: bool; - use_select: bool; } let do_argv = @@ -37,7 +36,7 @@ let do_argv = and config_file = ref "" and restart = ref false and disable_socket = ref false - and use_select = ref false in + in let speclist = [ ("--no-domain-init", Arg.Unit (fun () -> domain_init := false), @@ -54,9 +53,8 @@ let do_argv = ("-T", Arg.Set_string tracefile, ""); (* for compatibility *) ("--restart", Arg.Set restart, "Read database on starting"); ("--disable-socket", Arg.Unit (fun () -> disable_socket := true), "Disable socket"); - ("--use-select", Arg.Unit (fun () -> use_select := true), "Use select instead of poll"); (* for backward compatibility and testing *) ] in - let usage_msg = "usage : xenstored [--config-file <filename>] [--no-domain-init] [--help] [--no-fork] [--reraise-top-level] [--restart] [--disable-socket] [--use-select]" in + let usage_msg = "usage : xenstored [--config-file <filename>] [--no-domain-init] [--help] [--no-fork] [--reraise-top-level] [--restart] [--disable-socket]" in Arg.parse speclist (fun _ -> ()) usage_msg; { domain_init = !domain_init; @@ -68,5 +66,4 @@ let do_argv = tracefile = if !tracefile <> "" then Some !tracefile else None; restart = !restart; disable_socket = !disable_socket; - use_select = !use_select; } diff --git a/tools/ocaml/xenstored/poll.ml b/tools/ocaml/xenstored/poll.ml new file mode 100644 index 0000000000..26f8620dfc --- /dev/null +++ b/tools/ocaml/xenstored/poll.ml @@ -0,0 +1,67 @@ +(* + * Copyright (C) 2014 Zheng Li <dev@xxxxxxxx> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + + +(* The [read], [write], [except] are fields mapped to the POLLIN/OUT/PRI + subscription flags used by poll, which have a correspondence to the + readfds, writefds, exceptfds concept as in select. *) +type event = { + mutable read: bool; + mutable write: bool; + mutable except: bool; +} + +external select_on_poll: (Unix.file_descr * event) array -> int -> int = "stub_select_on_poll" +external set_fd_limit: int -> unit = "stub_set_fd_limit" + +(* The rlim_max given to setrlimit must not go above the system level nr_open, + which we can read from /proc/sys. *) +let get_sys_fs_nr_open () = + try + let ch = open_in "/proc/sys/fs/nr_open" in + let v = int_of_string (input_line ch) in + close_in_noerr ch; v + with _ -> 1024 * 1024 + +let init_event () = {read = false; write = false; except = false} + +let poll_select in_fds out_fds exc_fds timeout = + let h = Hashtbl.create 57 in + let add_event event_set fd = + let e = + try Hashtbl.find h fd + with Not_found -> + let e = init_event () in + Hashtbl.add h fd e; e in + event_set e in + List.iter (add_event (fun x -> x.read <- true)) in_fds; + List.iter (add_event (fun x -> x.write <- true)) out_fds; + List.iter (add_event (fun x -> x.except <- true)) exc_fds; + (* Unix.stdin and init_event are dummy input as stubs, which will + always be overwritten later on. *) + let a = Array.make (Hashtbl.length h) (Unix.stdin, init_event ()) in + let i = ref (-1) in + Hashtbl.iter (fun fd event -> incr i; Array.set a !i (fd, event)) h; + let n = select_on_poll a (int_of_float (timeout *. 1000.)) in + let r = [], [], [] in + if n = 0 then r else + Array.fold_right + (fun (fd, event) (r, w, x) -> + (if event.read then fd :: r else r), + (if event.write then fd :: w else w), + (if event.except then fd :: x else x)) + a r + +let () = + set_fd_limit (get_sys_fs_nr_open ()) diff --git a/tools/ocaml/xenstored/poll.mli b/tools/ocaml/xenstored/poll.mli new file mode 100644 index 0000000000..f73465b99f --- /dev/null +++ b/tools/ocaml/xenstored/poll.mli @@ -0,0 +1,19 @@ +(* + * Copyright (C) 2014 Zheng Li <dev@xxxxxxxx> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + + +(** Same interface and semantics as [Unix.select], implemented using poll(3). *) +val poll_select: + Unix.file_descr list -> Unix.file_descr list -> Unix.file_descr list -> float + -> Unix.file_descr list * Unix.file_descr list * Unix.file_descr list diff --git a/tools/ocaml/xenstored/select.ml b/tools/ocaml/xenstored/select.ml deleted file mode 100644 index 0455e163e3..0000000000 --- a/tools/ocaml/xenstored/select.ml +++ /dev/null @@ -1,77 +0,0 @@ -(* - * Copyright (C) 2014 Zheng Li <dev@xxxxxxxx> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published - * by the Free Software Foundation; version 2.1 only. with the special - * exception on linking described in file LICENSE. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - *) - - -(* The [read], [write], [except] are fields mapped to the POLLIN/OUT/PRI - subscription flags used by poll, which have a correspondence to the - readfds, writefds, exceptfds concept as in select. *) -type event = { - mutable read: bool; - mutable write: bool; - mutable except: bool; -} - -external select_on_poll: (Unix.file_descr * event) array -> int -> int = "stub_select_on_poll" -external set_fd_limit: int -> unit = "stub_set_fd_limit" - -(* The rlim_max given to setrlimit must not go above the system level nr_open, - which we can read from /proc/sys. *) -let get_sys_fs_nr_open () = - try - let ch = open_in "/proc/sys/fs/nr_open" in - let v = int_of_string (input_line ch) in - close_in_noerr ch; v - with _ -> 1024 * 1024 - -let init_event () = {read = false; write = false; except = false} - -let poll_select in_fds out_fds exc_fds timeout = - let h = Hashtbl.create 57 in - let add_event event_set fd = - let e = - try Hashtbl.find h fd - with Not_found -> - let e = init_event () in - Hashtbl.add h fd e; e in - event_set e in - List.iter (add_event (fun x -> x.read <- true)) in_fds; - List.iter (add_event (fun x -> x.write <- true)) out_fds; - List.iter (add_event (fun x -> x.except <- true)) exc_fds; - (* Unix.stdin and init_event are dummy input as stubs, which will - always be overwritten later on. *) - let a = Array.make (Hashtbl.length h) (Unix.stdin, init_event ()) in - let i = ref (-1) in - Hashtbl.iter (fun fd event -> incr i; Array.set a !i (fd, event)) h; - let n = select_on_poll a (int_of_float (timeout *. 1000.)) in - let r = [], [], [] in - if n = 0 then r else - Array.fold_right - (fun (fd, event) (r, w, x) -> - (if event.read then fd :: r else r), - (if event.write then fd :: w else w), - (if event.except then fd :: x else x)) - a r - -(* If the use_poll function is not called at all, we default to the original Unix.select behavior *) -let select_fun = ref Unix.select - -let use_poll yes = - let sel_fun, max_fd = - if yes then poll_select, get_sys_fs_nr_open () - else Unix.select, 1024 in - select_fun := sel_fun; - set_fd_limit max_fd - -let select in_fds out_fds exc_fds timeout = - (!select_fun) in_fds out_fds exc_fds timeout diff --git a/tools/ocaml/xenstored/select.mli b/tools/ocaml/xenstored/select.mli deleted file mode 100644 index 3912779172..0000000000 --- a/tools/ocaml/xenstored/select.mli +++ /dev/null @@ -1,27 +0,0 @@ -(* - * Copyright (C) 2014 Zheng Li <dev@xxxxxxxx> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published - * by the Free Software Foundation; version 2.1 only. with the special - * exception on linking described in file LICENSE. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - *) - - -(** Same interface and semantics as [Unix.select] but with an extra alternative - implementation based on poll. Switching implementations is done by calling - the [use_poll] function. *) -val select: - Unix.file_descr list -> Unix.file_descr list -> Unix.file_descr list -> float - -> Unix.file_descr list * Unix.file_descr list * Unix.file_descr list - -(** [use_poll true] will use poll based select with max fds number limitation - eliminated; [use_poll false] will use standard [Unix.select] with max fd - number set to 1024; not calling this function at all equals to use the - standard [Unix.select] with max fd number setting untouched. *) -val use_poll: bool -> unit diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml index a4466c5b5c..5b96f1852a 100644 --- a/tools/ocaml/xenstored/xenstored.ml +++ b/tools/ocaml/xenstored/xenstored.ml @@ -308,8 +308,6 @@ let _ = ); ); - Select.use_poll (not cf.use_select); - Sys.set_signal Sys.sighup (Sys.Signal_handle sighup_handler); Sys.set_signal Sys.sigterm (Sys.Signal_handle (fun _ -> quit := true)); Sys.set_signal Sys.sigusr1 (Sys.Signal_handle (fun _ -> sigusr1_handler store)); @@ -453,7 +451,7 @@ let _ = let inset, outset = Connections.select ~only_if:is_peaceful cons in let rset, wset, _ = try - Select.select (spec_fds @ inset) outset [] timeout + Poll.poll_select (spec_fds @ inset) outset [] timeout with Unix.Unix_error(Unix.EINTR, _, _) -> [], [], [] in let sfds, cfds = -- generated by git-patchbot for /home/xen/git/xen.git#master

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.