| From | David Howells <> | Subject | Re: [PATCH 01/45] Create a dynamically sized pool of threads for doing very slow work items [ver #41] | Date | Fri, 19 Dec 2008 12:54:02 +0000 |
| |
Andrew Morton <akpm@linux-foundation.org> wrote:
> This may be the most skilfully commented kernel code I've ever seen.
Thanks. I like useful comments as I may need to re-understand the code I've written should I have to fix it in five years time.
> > +static unsigned slow_work_min_threads = 2; > > +static unsigned slow_work_max_threads = (NR_CPUS > 4) ? NR_CPUS : 4; > > I suspect there will be a requirement to tune this at runtime.
Yes. See:
[PATCH 03/45] Make the slow work pool configurable [ver #41]
> Using num_possible_cpus() would be more accurate here. One could > easily envisage NR_CPUS=1024 on a 2-way machine. Generally any use > of NR_CPUS is a red flag. In fact there's a checkpatch warning about > it now.
This has been fixed in ver #42, which is what is in the GIT trees. I can post that as patches if you like. Well #43, I've just noticed a bug:-(.
> > +#define slow_work_available(vsmax) \ > > + (!list_empty(&slow_work_queue) || \ > > + (!list_empty(&vslow_work_queue) && \ > > + atomic_read(&vslow_work_executing_count) < (vsmax))) > > This could be a regular C function?
I suppose so.
> > + p = kthread_create(slow_work_thread, NULL, > > + "kslow%Xd", loop); > > + if (!p) > > + goto error; > > + wake_up_process(p); > > The above reimplements kthread_run().
And wrongly. I should use IS_ERR(p) not !p.
David
|