Hello Bob Peterson,
The patch dc732906c245: "gfs2: Introduce flag for glock holder
auto-demotion" from Aug 19, 2021, leads to the following Smatch
static checker warning:
fs/gfs2/glock.c:421 demote_incompat_holders()
warn: iterator 'gh->gh_list.next' changed during iteration
fs/gfs2/glock.c
411 static void demote_incompat_holders(struct gfs2_glock *gl,
412 struct gfs2_holder *new_gh)
413 {
414 struct gfs2_holder *gh;
415
416 /*
417 * Demote incompatible holders before we make ourselves
eligible.
418 * (This holder may or may not allow auto-demoting, but we
don't want
419 * to demote the new holder before it's even granted.)
420 */
--> 421 list_for_each_entry(gh, &gl->gl_holders, gh_list) {
422 /*
423 * Since holders are at the front of the list, we stop
when we
424 * find the first non-holder.
425 */
426 if (!test_bit(HIF_HOLDER, &gh->gh_iflags))
427 return;
428 if (test_bit(HIF_MAY_DEMOTE, &gh->gh_iflags) &&
429 !may_grant(gl, new_gh, gh)) {
430 /*
431 * We should not recurse into do_promote because
432 * __gfs2_glock_dq only calls handle_callback,
433 * gfs2_glock_add_to_lru and
__gfs2_glock_queue_work.
434 */
435 __gfs2_glock_dq(gh);
^^^^^^^^^^^^^^^^^^^^
This calls list_del_init(&gh->gh_list); which sets the ->next pointer.
So I think that means we could hit a forever loop situation looking for
the original &gl->gl_holders list head.
Should it use list_for_each_entry_safe()?
436 }
437 }
438 }
regards,
dan carpenter
