Messages in this thread | | | From | Jeff Moyer <> | Subject | Re: [patch] bdi: add a user-tunable cpu_list for the bdi flusher threads | Date | Mon, 03 Dec 2012 10:49:25 -0500 |
| |
Zach Brown <zab@redhat.com> writes:
>> + ret = cpulist_parse(buf, newmask); >> + if (!ret) { >> + spin_lock(&bdi->wb_lock); >> + task = wb->task; >> + get_task_struct(task); >> + spin_unlock(&bdi->wb_lock); >> + if (task) >> + ret = set_cpus_allowed_ptr(task, newmask); >> + put_task_struct(task); > > If that test for a non-null task is needed then surely the get and put > need to be similarly protected :).
How embarrassing.
>> + bdi->flusher_cpumask = kmalloc(sizeof(cpumask_t), GFP_KERNEL); >> + if (!bdi->flusher_cpumask) >> + return -ENOMEM; > > The bare GFP_KERNEL raises an eyebrow. Some bdi_init() callers like > blk_alloc_queue_node() look like they'll want to pass in a gfp_t for the > allocation.
I'd be surprised if that was necessary, seeing how every single caller of blk_alloc_queue_node passes in GFP_KERNEL. I'll make the change, though, there aren't too many callers of bdi_init out there.
> And shouldn't this be freed in the error path of bdi_init()?
Yes. ;-)
Thanks! Jeff
| |