Re: [PATCHv2 dlm/next 7/9] dlm: drop scand kthread and use timers

Alexander Aring
2024-04-17T11:40:47Z

Hi,

On Mon, Apr 15, 2024 at 2:39 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Currently the scand kthread acts like a garbage collection for expired
> rsbs on toss list to clean them up after a certain timeout. It triggers
> every couple of seconds and iterates over the toss list while holding
> ls_rsbtbl_lock for the whole hash bucket iteration. To reduce the amount
> of holding the ls_rsbtbl_lock time we handle free of cached rsbs (tossed
> rsbs) on a per rsb timer. If it expires the rsb deletes itself out of
> the rsb toss queue that is an ordered queue of tossed rsbs. First entry
> is the earliest rsb to expire the last is the lastest rsb that will expire.
> The res_toss_time is now an absolute value of when it will be expired
> before it was the time when the rsb was tossed. It was an debug
> functionality for debugfs anyway, we change a little bit the semantic
> here.
>
> Signed-off-by: Alexander Aring <aahringo@redhat.com>
> ---
> fs/dlm/dlm_internal.h | 16 +-
> fs/dlm/lock.c | 382 ++++++++++++++++++++++++++----------------
> fs/dlm/lock.h | 2 +
> fs/dlm/lockspace.c | 105 ++----------
> fs/dlm/member.c | 2 +
> fs/dlm/recover.c | 5 +
> fs/dlm/recoverd.c | 10 ++
> 7 files changed, 283 insertions(+), 239 deletions(-)
>
> diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
> index cf43b97cf3e5..98a0ac511bc8 100644
> --- a/fs/dlm/dlm_internal.h
> +++ b/fs/dlm/dlm_internal.h
> @@ -334,6 +334,7 @@ struct dlm_rsb {
> struct list_head res_root_list; /* used for recovery */
> struct list_head res_masters_list; /* used for recovery */
> struct list_head res_recover_list; /* used for recovery */
> + struct list_head res_toss_q_list;
> int res_recover_locks_count;
>
> char *res_lvbptr;
> @@ -584,13 +585,20 @@ struct dlm_ls {
> spinlock_t ls_lkbidr_spin;
>
> struct rhashtable ls_rsbtbl;
> -#define DLM_RTF_SHRINK_BIT 0
> - unsigned long ls_rsbtbl_flags;
> spinlock_t ls_rsbtbl_lock;
>
> struct list_head ls_toss;
> struct list_head ls_keep;
>
> + struct timer_list ls_timer;
> + /* this queue is ordered according the
> + * absolute res_toss_time jiffies time
> + * to mod_timer() with the first element
> + * if necessary.
> + */
> + struct list_head ls_toss_q;
> + spinlock_t ls_toss_q_lock;
> +
> spinlock_t ls_waiters_lock;
> struct list_head ls_waiters; /* lkbs needing a reply */
>
> @@ -601,9 +609,6 @@ struct dlm_ls {
> int ls_new_rsb_count;
> struct list_head ls_new_rsb; /* new rsb structs */
>
> - char
...
> + spin_unlock_bh(&ls->ls_rsbtbl_lock);
> +
> goto out_unlock;
>
>
> @@ -777,6 +1009,9 @@ static int find_rsb_nodir(struct dlm_ls *ls, const void *name, int len,
> * valid for keep state rsbs
> */
> kref_init(&r->res_ref);
> + rsb_delete_toss_timer(ls, r);
> + spin_unlock_bh(&ls->ls_rsbtbl_lock);
> +
> goto out_unlock;

those spin_unlock_bh() are twice there... just for note I realized it
now, it was fixed in the next patch. Probably because I was doing some
git rebase things and tried to solve conflicts and made this
mistake...

- Alex