[Cluster-devel] [PATCH 3/8] gfs2: Clean up glock work enqueuing

[Andreas Gruenbacher <agruenba@redhat.com>]

On Thu, Jun 1, 2017 at 5:21 PM, Steven Whitehouse <swhiteho@redhat.com> wrote:
> On 31/05/17 16:03, Andreas Gruenbacher wrote:
> Looks generally like a nice clean up, but...
> [...]
>> @@ -623,16 +643,25 @@ static void glock_work_func(struct work_struct *work)
>>                 }
>>         }
>>         run_queue(gl, 0);
>> -       spin_unlock(&gl->gl_lockref.lock);
>> -       if (!delay)
>> -               gfs2_glock_put(gl);
>> -       else {
>> +       if (delay) {
>> +               /* Keep one glock reference for the work we requeue. */
>> +               drop_refs--;
>>                 if (gl->gl_name.ln_type != LM_TYPE_INODE)
>>                         delay = 0;
>> -               if (queue_delayed_work(glock_workqueue, &gl->gl_work, delay) == 0)
>> -                       gfs2_glock_put(gl);
>> +               __gfs2_glock_queue_work(gl, delay);
>> +       }
>> +       if (drop_refs > 1) {
>> +               gl->gl_lockref.count -= drop_refs - 1;
>> +               drop_refs = 1;
>>         }
>
> This looks very confusing... is there no cleaner way to write this? After
> all the effort of cleaning up the queuing of the work, this could also do
> with tidying up too. Otherwise I think the patch looks good,

You mean like below, removing the refcount optimization?

Andreas

---
 fs/gfs2/glock.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 51ec6de..f833f1b 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -732,10 +732,6 @@ static void glock_work_func(struct work_struct *work)
 			delay = 0;
 		__gfs2_glock_queue_work(gl, delay);
 	}
-	if (drop_refs > 1) {
-		gl->gl_lockref.count -= drop_refs - 1;
-		drop_refs = 1;
-	}
 	spin_unlock(&gl->gl_lockref.lock);
 
 	/*
@@ -743,7 +739,7 @@ static void glock_work_func(struct work_struct *work)
 	 * the lockref spinlock: this allows tasks to safely drop glock
 	 * references under that spinlock (see __gfs2_glock_queue_work).
 	 */
-	if (drop_refs)
+	while (drop_refs--)
 		gfs2_glock_put(gl);
 }
 
-- 
2.7.4