lkml.org 
[lkml]   [2008]   [Dec]   [19]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
FromDavid Howells <>
SubjectRe: [PATCH 01/45] Create a dynamically sized pool of threads for doing very slow work items [ver #41]
DateFri, 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


\
 
 \ /
  Last update: 2008-12-19 13:57    [from the cache]
©2003-2008