Hi Ammar,
On 6/9/23 21:07, Ammar Faizi wrote:
On Fri, Jun 09, 2023 at 08:20:27PM +0800, Hao Xu wrote:
+ rcu_read_lock();
+
+ for (i = 0; i < IO_WQ_ACCT_NR; i++) {
+ unsigned int nr = count[i].nr_workers;
+
+ acct = &wq->acct[i];
+ acct->fixed_nr = nr;
+ acct->fixed_workers = kcalloc(nr, sizeof(struct io_worker *),
+ GFP_KERNEL);
+ if (!acct->fixed_workers) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ for (j = 0; j < nr; j++) {
+ struct io_worker *worker =
+ io_wq_create_worker(wq, acct, true);
+ if (IS_ERR(worker)) {
+ ret = PTR_ERR(worker);
+ break;
+ }
+ acct->fixed_workers[j] = worker;
+ }
+ if (j < nr)
+ break;
+ }
+ rcu_read_unlock();
This looks wrong. kcalloc() with GFP_KERNEL may sleep. Note that you're
not allowed to sleep inside the RCU read lock critical section.
Using GFP_KERNEL implies GFP_RECLAIM, which means that direct reclaim
may be triggered under memory pressure; the calling context must be
allowed to sleep.
I think you are right, I'll fix it in v2.
Hi Jens, ask a question about this: I saw same rcu_read_lock() in
io_wq_max_workers(), but what is it really protect?
Regards,
Hao