Re: [PATCH RFC 11/12] RDMA/rtrs-srv: fix several issues in rtrs_srv_destroy_path_files

Haris Iqbal
2022-11-14T16:23:25Z

On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> There are several issues in the function which is supposed to be paired
> with rtrs_srv_create_path_files.
>
> 1. rtrs_srv_stats_attr_group is not removed though it is created in
> rtrs_srv_create_stats_files.
>
> 2. it makes more sense to check kobj_stats.state_in_sysfs before destroy
> kobj_stats instead of rely on kobj.state_in_sysfs.
>
> 3. kobject_init_and_add is used for both kobjs (srv_path->kobj and
> srv_path->stats->kobj_stats), however we missed to call kobject_del
> for srv_path->kobj which was called in free_path.
>
> 4. rtrs_srv_destroy_once_sysfs_root_folders is independant of either
> kobj or kobj_stats.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>

Thanks

> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> index 2a3c9ac64a42..da8e205ce331 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> @@ -304,12 +304,18 @@ int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
>
> void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
> {
> - if (srv_path->kobj.state_in_sysfs) {
> + if (srv_path->stats->kobj_stats.state_in_sysfs) {
> + sysfs_remove_group(&srv_path->stats->kobj_stats,
> + &rtrs_srv_stats_attr_group);
> kobject_del(&srv_path->stats->kobj_stats);
> kobject_put(&srv_path->stats->kobj_stats);
> + }
> +
> + if (srv_path->kobj.state_in_sysfs) {
> sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
> + kobject_del(&srv_path->kobj);
> kobject_put(&srv_path->kobj);
> -
> - rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> }
> +
> + rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> }
> --
> 2.31.1
>