[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 3/8] xen/hypfs: add new enter() and exit() per node callbacks
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@xxxxxxxx> >>> --- >>> V2: >>> - new patch >>> >>> V3: >>> - add ASSERT(entry); (Jan Beulich) >>> >>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx> >>> --- >>> 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@xxxxxxxx> ? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |