linux-kernel - Re: [PATCH 01/45] Create a dynamically sized pool of threads for doing very slow work items [ver #41]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10865.1229691242@redhat.com>
Date:	Fri, 19 Dec 2008 12:54:02 +0000
From:	David Howells <dhowells@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	dhowells@...hat.com, trond.myklebust@....uio.no,
	viro@...IV.linux.org.uk, nfsv4@...ux-nfs.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 01/45] Create a dynamically sized pool of threads for doing very slow work items [ver #41]

Andrew Morton <akpm@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/