[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/6] xenbus: process be_watch events in xenwatch multithreading
Hi Juergen, On 09/14/2018 10:44 PM, Juergen Gross wrote: > On 14/09/18 16:29, Dongli Zhang wrote: >> Hi Juergen, >> >> On 09/14/2018 10:26 PM, Juergen Gross wrote: >>> On 14/09/18 16:18, Dongli Zhang wrote: >>>> Hi Juergen, >>>> >>>> On 09/14/2018 05:12 PM, Juergen Gross wrote: >>>>> On 14/09/18 09:34, Dongli Zhang wrote: >>>>>> This is the 5th patch of a (6-patch) patch set. >>>>>> >>>>>> With this patch, watch event in relative path pattern >>>>>> 'backend/<pvdev>/<domid>i/...' can be processed in per-domU xenwatch >>>>> >>>>> superfluous "i" ----------^ >>>>> >>>>>> thread. >>>>>> >>>>>> Signed-off-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx> >>>>>> --- >>>>>> drivers/xen/xenbus/xenbus_probe.c | 2 +- >>>>>> drivers/xen/xenbus/xenbus_probe_backend.c | 32 >>>>>> +++++++++++++++++++++++++++++++ >>>>>> include/xen/xenbus.h | 2 ++ >>>>>> 3 files changed, 35 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c >>>>>> b/drivers/xen/xenbus/xenbus_probe.c >>>>>> index ba0644c..aa1b15a 100644 >>>>>> --- a/drivers/xen/xenbus/xenbus_probe.c >>>>>> +++ b/drivers/xen/xenbus/xenbus_probe.c >>>>>> @@ -552,7 +552,7 @@ int xenbus_probe_devices(struct xen_bus_type *bus) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(xenbus_probe_devices); >>>>>> >>>>>> -static unsigned int char_count(const char *str, char c) >>>>>> +unsigned int char_count(const char *str, char c) >>>>> >>>>> Please change the name of the function when making it globally >>>>> visible, e.g. by prefixing "xenbus_". >>>>> >>>>> Generally I think you don't need to use it below. >>>>> >>>>>> { >>>>>> unsigned int i, ret = 0; >>>>>> >>>>>> diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c >>>>>> b/drivers/xen/xenbus/xenbus_probe_backend.c >>>>>> index b0bed4f..50df86a 100644 >>>>>> --- a/drivers/xen/xenbus/xenbus_probe_backend.c >>>>>> +++ b/drivers/xen/xenbus/xenbus_probe_backend.c >>>>>> @@ -211,9 +211,41 @@ static void backend_changed(struct xenbus_watch >>>>>> *watch, >>>>>> xenbus_dev_changed(path, &xenbus_backend); >>>>>> } >>>>>> >>>>>> +static domid_t path_to_domid(const char *path) >>>>>> +{ >>>>>> + const char *p = path; >>>>>> + domid_t domid = 0; >>>>>> + >>>>>> + while (*p) { >>>>>> + if (*p < '0' || *p > '9') >>>>>> + break; >>>>>> + domid = (domid << 3) + (domid << 1) + (*p - '0'); >>>>> >>>>> reinventing atoi()? >>>>> >>>>> Please don't do that. kstrtou16() seems to be a perfect fit. >>>> >>>> I did use kstrtou*() in the early prototype and realized kstrtou16() >>>> returns 0 >>>> if the input string contains non-numerical characters. >>>> >>>> E.g., the example of input can be "1/0/state", where 1 is fotherend_id >>>> (frontend_id) and 0 is handle. >>>> >>>> When "1/0/state" is used at input, kstrtou16() returns 0 (returned >>>> integer) and >>>> -22 (error). >>> >>> Aah, okay. Then simple_strtoul()? >> >> I did consider simple_strtoul() initially. Unfortunately, it is obsolete >> (below >> line 81). AFAIR, the patch would not be able to pass the check_patch script >> when >> this function is used. > > Better use that than open coding a new instance of it. > > Another variant would be to use sscanf() or similar. Then you could even > drop using strchr() by adding that in the format string: > > return (sscanf(path, "%*u/%u/", &domid) == 1) ? domid : DOMID_SELF; I recall what was happened. Suppose one sample of path is "backend/vif/19/3/state". (we would like to obtain domid=19) Initially I would like to use sscanf(path, "backend/%*[a-z]/%hu/%*u") to obtain the domid from xenstore path in one call. Unfortunately, unlike userspace sscanf(), the version in linux kernel does not support '[' so that I would not be able to use "%*[a-z]" in sscanf() in linux kernel. Finally, I decided to skip 2 '/' to reach at "19/3/state" and I forgot to continue using sscanf and implemented my own one :( With sscanf, I think I still need to skip 2 '/' to skip "backend/<pv device>" in order to reach at "<domid>/xxxxxxx...": 1. return if the number of '/' in the path is less than 2 2. skip 2 '/' (e.g., backend/vif/) 3. process the rest of patch via sscanf. Dongli Zhang _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |