[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
On 19/09/18 08:15, Dongli Zhang wrote: > 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. That is not correct. It doesn't support ranges in [], but it is perfectly fine to use %[^/]. This requires a temporary buffer, as %*[ isn't supported. Why don't you use: char temp[16]; ... /* kernel sscanf() %[] doesn't support '*' modifier and needs length. */ sscanf(path, "backend/%15[^/]%hu/%*u", temp, &domid) Juergen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |