On Mon, Jan 22, 2024 at 05:51:32PM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
>
> The patch ff35f7b3d0cf: "bcachefs: Better journal tracepoints" from
> Jan 15, 2024 (linux-next), leads to the following Smatch static
> checker warning:
>
> fs/bcachefs/journal.c:236 __journal_entry_close()
> warn: sleeping in atomic context
>
> fs/bcachefs/journal.c
> 200 static void __journal_entry_close(struct journal *j, unsigned
> closed_val, bool trace)
> 201 {
> 202 struct bch_fs *c = container_of(j, struct bch_fs, journal);
> 203 struct journal_buf *buf = journal_cur_buf(j);
> 204 union journal_res_state old, new;
> 205 u64 v = atomic64_read(&j->reservations.counter);
> 206 unsigned sectors;
> 207
> 208 BUG_ON(closed_val != JOURNAL_ENTRY_CLOSED_VAL &&
> 209 closed_val != JOURNAL_ENTRY_ERROR_VAL);
> 210
> 211 lockdep_assert_held(&j->lock);
> 212
> 213 do {
> 214 old.v = new.v = v;
> 215 new.cur_entry_offset = closed_val;
> 216
> 217 if (old.cur_entry_offset == JOURNAL_ENTRY_ERROR_VAL ||
> 218 old.cur_entry_offset == new.cur_entry_offset)
> 219 return;
> 220 } while ((v = atomic64_cmpxchg(&j->reservations.counter,
> 221 old.v, new.v)) != old.v);
> 222
> 223 if (!__journal_entry_is_open(old))
> 224 return;
> 225
> 226 /* Close out old buffer: */
> 227 buf->data->u64s =
> cpu_to_le32(old.cur_entry_offset);
> 228
> 229 if (trace_journal_entry_close_enabled() && trace) {
> 230 struct printbuf pbuf = PRINTBUF;
> 231 pbuf.atomic++;
> 232
> 233 prt_str(&pbuf, "entry size: ");
> 234 prt_human_readable_u64(&pbuf,
> vstruct_bytes(buf->data));
> 235 prt_newline(&pbuf);
> --> 236 bch2_prt_task_backtrace(&pbuf, current, 1);
> ^^^^^^^^^^^^^^^^^^^^^^^
> It looks like bch2_prt_task_backtrace() calls kvmalloc() in
> __bch2_darray_resize() which is a sleeping function. A lot of of the
> callers are holding spinlocks.
>
> bch2_journal_halt() <- disables preempt
> bch2_journal_entry_close() <- disables preempt
> bch2_journal_flush_seq_async() <- disables preempt
> -> journal_entry_want_write()
> journal_write_work() <- disables preempt
> __journal_res_get() <- disables preempt
> bch2_journal_entry_res_resize() <- disables preempt
> bch2_journal_flush_seq_async() <- disables preempt <duplicate>
> __bch2_next_write_buffer_flush_journal_buf() <- disables preempt
> -> __journal_entry_close()
>
> regards,
> dan carpenter
Thanks - that just needs gfp flags plumbed.
