On 16.12.20 17:36, Jan Beulich wrote:
> On 16.12.2020 17:24, Jürgen Groß wrote:
>> On 16.12.20 17:16, Jan Beulich wrote:
>>> On 09.12.2020 17:09, Juergen Gross wrote:
>>>> In order to better support resource allocation and locking for dynamic
>>>> hypfs nodes add enter() and exit() callbacks to struct hypfs_funcs.
>>>>
>>>> The enter() callback is called when entering a node during hypfs user
>>>> actions (traversing, reading or writing it), while the exit() callback
>>>> is called when leaving a node (accessing another node at the same or a
>>>> higher directory level, or when returning to the user).
>>>>
>>>> For avoiding recursion this requires a parent pointer in each node.
>>>> Let the enter() callback return the entry address which is stored as
>>>> the last accessed node in order to be able to use a template entry for
>>>> that purpose in case of dynamic entries.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> V2:
>>>> - new patch
>>>>
>>>> V3:
>>>> - add ASSERT(entry); (Jan Beulich)
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>    xen/common/hypfs.c      | 80 +++++++++++++++++++++++++++++++++++++++++
>>>>    xen/include/xen/hypfs.h |  5 +++
>>>>    2 files changed, 85 insertions(+)
>>>>
>>>> diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
>>>> index 6f822ae097..f04934db10 100644
>>>> --- a/xen/common/hypfs.c
>>>> +++ b/xen/common/hypfs.c
>>>> @@ -25,30 +25,40 @@ CHECK_hypfs_dirlistentry;
>>>>         ROUNDUP((name_len) + 1, alignof(struct xen_hypfs_direntry)))
>>>>    
>>>>    const struct hypfs_funcs hypfs_dir_funcs = {
>>>> +    .enter = hypfs_node_enter,
>>>> +    .exit = hypfs_node_exit,
>>>>        .read = hypfs_read_dir,
>>>>        .write = hypfs_write_deny,
>>>>        .getsize = hypfs_getsize,
>>>>        .findentry = hypfs_dir_findentry,
>>>>    };
>>>>    const struct hypfs_funcs hypfs_leaf_ro_funcs = {
>>>> +    .enter = hypfs_node_enter,
>>>> +    .exit = hypfs_node_exit,
>>>>        .read = hypfs_read_leaf,
>>>>        .write = hypfs_write_deny,
>>>>        .getsize = hypfs_getsize,
>>>>        .findentry = hypfs_leaf_findentry,
>>>>    };
>>>>    const struct hypfs_funcs hypfs_leaf_wr_funcs = {
>>>> +    .enter = hypfs_node_enter,
>>>> +    .exit = hypfs_node_exit,
>>>>        .read = hypfs_read_leaf,
>>>>        .write = hypfs_write_leaf,
>>>>        .getsize = hypfs_getsize,
>>>>        .findentry = hypfs_leaf_findentry,
>>>>    };
>>>>    const struct hypfs_funcs hypfs_bool_wr_funcs = {
>>>> +    .enter = hypfs_node_enter,
>>>> +    .exit = hypfs_node_exit,
>>>>        .read = hypfs_read_leaf,
>>>>        .write = hypfs_write_bool,
>>>>        .getsize = hypfs_getsize,
>>>>        .findentry = hypfs_leaf_findentry,
>>>>    };
>>>>    const struct hypfs_funcs hypfs_custom_wr_funcs = {
>>>> +    .enter = hypfs_node_enter,
>>>> +    .exit = hypfs_node_exit,
>>>>        .read = hypfs_read_leaf,
>>>>        .write = hypfs_write_custom,
>>>>        .getsize = hypfs_getsize,
>>>> @@ -63,6 +73,8 @@ enum hypfs_lock_state {
>>>>    };
>>>>    static DEFINE_PER_CPU(enum hypfs_lock_state, hypfs_locked);
>>>>    
>>>> +static DEFINE_PER_CPU(const struct hypfs_entry *, hypfs_last_node_entered);
>>>> +
>>>>    HYPFS_DIR_INIT(hypfs_root, "");
>>>>    
>>>>    static void hypfs_read_lock(void)
>>>> @@ -100,11 +112,59 @@ static void hypfs_unlock(void)
>>>>        }
>>>>    }
>>>>    
>>>> +const struct hypfs_entry *hypfs_node_enter(const struct hypfs_entry *entry)
>>>> +{
>>>> +    return entry;
>>>> +}
>>>> +
>>>> +void hypfs_node_exit(const struct hypfs_entry *entry)
>>>> +{
>>>> +}
>>>> +
>>>> +static int node_enter(const struct hypfs_entry *entry)
>>>> +{
>>>> +    const struct hypfs_entry **last = &this_cpu(hypfs_last_node_entered);
>>>> +
>>>> +    entry = entry->funcs->enter(entry);
>>>> +    if ( IS_ERR(entry) )
>>>> +        return PTR_ERR(entry);
>>>> +
>>>> +    ASSERT(entry);
>>>> +    ASSERT(!*last || *last == entry->parent);
>>>> +
>>>> +    *last = entry;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void node_exit(const struct hypfs_entry *entry)
>>>> +{
>>>> +    const struct hypfs_entry **last = &this_cpu(hypfs_last_node_entered);
>>>> +
>>>> +    if ( !*last )
>>>> +        return;
>>>
>>> To my question regarding this in v2 you replied
>>>
>>> "I rechecked and have found that this was a remnant from an earlier
>>>    variant. *last won't ever be NULL, so the if can be dropped (a NULL
>>>    will be catched by the following ASSERT())."
>>>
>>> Now this if() is still there. Why?
>>
>> I really thought I did remove the if(). Seems as if I did that on
>> my test machine only and not in my git tree. Sorry for that.
> 
> So should I drop it while committing and adding
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ?

Yes, please.


Juergen