Change liveupdate_unregister_file_handler and liveupdate_unregister_flb to return void instead of an error code. If they fail to unregister due to dangling references, print a warning instead of aborting. As pointed out by Jason Gunthorpe and Alex Williamson, returning an error from a "destroy" or unregister function during a module unload is a poor API choice since the unload cannot be stopped at that point. Signed-off-by: Pasha Tatashin --- include/linux/liveupdate.h | 13 ++++++------ kernel/liveupdate/luo_file.c | 27 +++++++++++------------- kernel/liveupdate/luo_flb.c | 40 ++++++++++-------------------------- lib/tests/liveupdate.c | 8 ++------ 4 files changed, 31 insertions(+), 57 deletions(-) diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h index 8394fb2d8774..293df26b9e7f 100644 --- a/include/linux/liveupdate.h +++ b/include/linux/liveupdate.h @@ -231,12 +231,12 @@ bool liveupdate_enabled(void); int liveupdate_reboot(void); int liveupdate_register_file_handler(struct liveupdate_file_handler *fh); -int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh); +void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh); int liveupdate_register_flb(struct liveupdate_file_handler *fh, struct liveupdate_flb *flb); -int liveupdate_unregister_flb(struct liveupdate_file_handler *fh, - struct liveupdate_flb *flb); +void liveupdate_unregister_flb(struct liveupdate_file_handler *fh, + struct liveupdate_flb *flb); int liveupdate_flb_get_incoming(struct liveupdate_flb *flb, void **objp); int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb, void **objp); @@ -258,9 +258,8 @@ static inline int liveupdate_register_file_handler(struct liveupdate_file_handle return -EOPNOTSUPP; } -static inline int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) +static inline void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) { - return -EOPNOTSUPP; } static inline int liveupdate_register_flb(struct liveupdate_file_handler *fh, @@ -269,8 +268,8 @@ static inline int liveupdate_register_flb(struct liveupdate_file_handler *fh, return -EOPNOTSUPP; } -static inline int liveupdate_unregister_flb(struct liveupdate_file_handler *fh, - struct liveupdate_flb *flb) +static inline void liveupdate_unregister_flb(struct liveupdate_file_handler *fh, + struct liveupdate_flb *flb) { return -EOPNOTSUPP; } diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c index 6b2b49cb375e..0fe2f8be8bd1 100644 --- a/kernel/liveupdate/luo_file.c +++ b/kernel/liveupdate/luo_file.c @@ -907,21 +907,17 @@ int liveupdate_register_file_handler(struct liveupdate_file_handler *fh) * reverses the operations of liveupdate_register_file_handler(). * * It ensures safe removal by checking that: - * No live update session is currently in progress. * No FLB registered with this file handler. * * If the unregistration fails, the internal test state is reverted. * - * Return: 0 Success. -EOPNOTSUPP when live update is not enabled. -EBUSY A live - * update is in progress, can't quiesce live update or FLB is registred with - * this file handler. */ -int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) +void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) { - int err = -EBUSY; + bool is_empty; if (!liveupdate_enabled()) - return -EOPNOTSUPP; + return; liveupdate_test_unregister(fh); @@ -929,18 +925,19 @@ int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) goto err_register; scoped_guard(rwsem_write, &luo_file_handler_lock) { - if (!list_empty(&ACCESS_PRIVATE(fh, flb_list))) - goto err_resume; - - list_del(&ACCESS_PRIVATE(fh, list)); + is_empty = list_empty(&ACCESS_PRIVATE(fh, flb_list)); + if (is_empty) + list_del(&ACCESS_PRIVATE(fh, list)); } luo_session_resume(); - return 0; + if (!is_empty) { + pr_warn("Failed to unregister file handler '%s': FLB list not empty\n", + fh->compatible); + liveupdate_test_register(fh); + } + return; -err_resume: - luo_session_resume(); err_register: liveupdate_test_register(fh); - return err; } diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c index daa852abdedd..23fa6e0c6083 100644 --- a/kernel/liveupdate/luo_flb.c +++ b/kernel/liveupdate/luo_flb.c @@ -436,31 +436,17 @@ int liveupdate_register_flb(struct liveupdate_file_handler *fh, * the FLB is removed from the global registry and the reference to its * owner module (acquired during registration) is released. * - * Context: This function ensures the session is quiesced (no active FDs - * being created) during the update. It is typically called from a - * subsystem's module exit function. - * Return: 0 on success. - * -EOPNOTSUPP if live update is disabled. - * -EBUSY if the live update session is active and cannot be quiesced. - * -ENOENT if the FLB was not found in the file handler's list. */ -int liveupdate_unregister_flb(struct liveupdate_file_handler *fh, - struct liveupdate_flb *flb) +void liveupdate_unregister_flb(struct liveupdate_file_handler *fh, + struct liveupdate_flb *flb) { struct luo_flb_private *private = luo_flb_get_private(flb); struct list_head *flb_list = &ACCESS_PRIVATE(fh, flb_list); struct luo_flb_link *iter; - int err = -ENOENT; + bool found = false; if (!liveupdate_enabled()) - return -EOPNOTSUPP; - - /* - * Ensure the system is quiescent (no active sessions). - * This acts as a global lock for unregistration. - */ - if (!luo_session_quiesce()) - return -EBUSY; + return; guard(rwsem_write)(&luo_flb_lock); guard(rwsem_write)(&ACCESS_PRIVATE(fh, flb_lock)); @@ -470,15 +456,19 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh, if (iter->flb == flb) { list_del(&iter->list); kfree(iter); - err = 0; + found = true; break; } } - if (err) - goto err_resume; + if (!found) { + pr_warn("Failed to unregister FLB '%s': not found in file handler '%s'\n", + flb->compatible, fh->compatible); + return; + } private->users--; + /* * If this is the last file-handler with which we are registred, remove * from the global list, and relese module reference. @@ -487,14 +477,6 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh, list_del_init(&private->list); luo_flb_global.count--; } - - luo_session_resume(); - - return 0; - -err_resume: - luo_session_resume(); - return err; } /** diff --git a/lib/tests/liveupdate.c b/lib/tests/liveupdate.c index 496d6ef91a30..5b6abf779f87 100644 --- a/lib/tests/liveupdate.c +++ b/lib/tests/liveupdate.c @@ -137,16 +137,12 @@ void liveupdate_test_register(struct liveupdate_file_handler *fh) void liveupdate_test_unregister(struct liveupdate_file_handler *fh) { - int err, i; + int i; for (i = 0; i < TEST_NFLBS; i++) { struct liveupdate_flb *flb = &test_flbs[i]; - err = liveupdate_unregister_flb(fh, flb); - if (err) { - pr_err("Failed to unregister %s %pe\n", - flb->compatible, ERR_PTR(err)); - } + liveupdate_unregister_flb(fh, flb); } pr_info("Unregistered %d FLBs from file handler: [%s]\n", -- 2.53.0.851.ga537e3e6e9-goog