On 8/11/24 01:44, Kamlesh Gurudasani wrote:
Waiman Long <longman@xxxxxxxxxx> writes:
On 8/10/24 13:44, Kamlesh Gurudasani wrote:
Waiman Long <longman@xxxxxxxxxx> writes:
...
diff --git a/kernel/padata.c b/kernel/padata.c
index 53f4bc912712..0fa6c2895460 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -517,6 +517,13 @@ void __init padata_do_multithreaded(struct padata_mt_job *job)
ps.chunk_size = max(ps.chunk_size, job->min_chunk);
ps.chunk_size = roundup(ps.chunk_size, job->align);
+ /*
+ * chunk_size can be 0 if the caller sets min_chunk to 0. So force it
+ * to at least 1 to prevent divide-by-0 panic in padata_mt_helper().`
+ */
Thanks for the patch and detailed comment.
+ if (!ps.chunk_size)
+ ps.chunk_size = 1U;
+
could it be
ps.chunk_size = max(ps.chunk_size, 1U);
or can be merged with earlier max()
ps.chunk_size = max(ps.chunk_size, max(job->min_chunk, 1U));
ps.chunk_size = roundup(ps.chunk_size, job->align);
sits well with how entire file is written and compiler is optimizing
them to same level.
I had actually thought about doing that as an alternative. I used the
current patch to avoid putting too many max() calls there. I can go this
route if you guys prefer this.
Just curious, what is your reason for avoiding too many max() calls? Both
if (!ps.chunk_size)
ps.chunk_size = 1U;
and
ps.chunk_size = max(ps.chunk_size, 1U);
are having same number of instructions [1].
[1] https://godbolt.org/z/ajrK59c67
We can avoid nested max(), though following would make it easier to understand.
ps.chunk_size = max(ps.chunk_size, 1U);
That will certainly work. My current patch has been merged into the
Linus tree. You are welcome to post another patch to clean it up if you
want.
Cheers,
Longman
Cheers,
Kamlesh
Cheers,
Longman