On 7/28/21 3:31 AM, Dan Carpenter wrote:
Hi GFS2 devs,
This is 10 year old code, but it looks suspicious and hopefully the
recovery code doesn't get testing very often in runtime.
The patch 75549186edf1: "GFS2: Fix bug-trap in ail flush code" from
Aug 2, 2011, leads to the following static checker warning:
fs/gfs2/glock.c:1487 gfs2_glock_dq()
warn: sleeping in atomic context
fs/gfs2/glops.c
57 static void __gfs2_ail_flush(struct gfs2_glock *gl, bool fsync,
58 unsigned int nr_revokes)
59 {
60 struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
61 struct list_head *head = &gl->gl_ail_list;
62 struct gfs2_bufdata *bd, *tmp;
63 struct buffer_head *bh;
64 const unsigned long b_state = (1UL << BH_Dirty)|(1UL <<
BH_Pinned)|(1UL << BH_Lock);
65
66 gfs2_log_lock(sdp);
^^^^^^^^^^^^^^^^^^
67 spin_lock(&sdp->sd_ail_lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We're holding a spinlock here
68 list_for_each_entry_safe_reverse(bd, tmp, head,
bd_ail_gl_list) {
69 if (nr_revokes == 0)
70 break;
71 bh = bd->bd_bh;
72 if (bh->b_state & b_state) {
73 if (fsync)
74 continue;
75 gfs2_ail_error(gl, bh);
^^^^^^^^^^^^^^^^^^^^^^
The gfs2_ail_error() function calls gfs2_withdraw() which can sleep or
the call tree that this is complains about is:
--> gfs2_ail_error()
--> gfs2_withdraw()
--> signal_our_withdraw()
-->gfs2_glock_dq()
It's also very possible that this is a false positive... Smatch doesn't
understand bit tests very well and especially across function
boundaries.
76 }
77 gfs2_trans_add_revoke(sdp, bd);
78 nr_revokes--;
79 }
80 GLOCK_BUG_ON(gl, !fsync && atomic_read(&gl->gl_ail_count));
81 spin_unlock(&sdp->sd_ail_lock);
82 gfs2_log_unlock(sdp);
83 }
regards,
dan carpenter
Hi Dan,
Thanks. I don't think it's a false positive. I think we should be using
a delayed withdraw rather than an actual withdraw. I'll send a patch out
to fix it shortly.
Regards,
Bob Peterson
