linux-kernel - Re: [PATCH 1/5] percpu_ida: Make percpu_ida

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 19 Jan 2014 18:38:55 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
Cc:	target-devel <target-devel@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Kent Overstreet <kmo@...erainc.com>,
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH 1/5] percpu_ida: Make percpu_ida_alloc accept task state bitmask

On Sun, Jan 19, 2014 at 2:16 AM, Nicholas A. Bellinger
<nab@...ux-iscsi.org> wrote:
>
> This patch changes percpu_ida_alloc() to accept task state bitmask
> for prepare_to_wait() to support interruptible sleep for callers
> that require it.

This patch-series is not bisectable. Afaik, the first patch will break
the build (or at least cause the end result to not actually work).

This kind of "split up one large patch into many small patches THAT
DON'T ACTUALLY WORK INDIVIDUALLY" model is pure and utter garbage.

So a big NAK on this series as being completely broken.

To fix it, I would suggest:

 - make the first patch change all *existing* users (that only have
the atomic vs uninterruptible semantics) pass in either
TASK_UNINTERRUPTIBLE or TASK_RUNNING depending on whether they had
__GFP_WAIT or not.

   So the first patch would not change *any* semantics or behavior, it
would only change the calling convention.

 - do the cleanup patches to block/blk-mq-tag.c to not have those
"gfp" calling convention, and instead passing in the state natively

 - add the TASK_INTERRUPTIBLE case last (which includes the new
"signal_pending_state()" logic in percpu_ida_alloc())

that way, all patches compile cleanly and should each work
individually, and they all do clearly just one thing. And the biggest
patch in the series (the first one) doesn't actually make any semantic
changes.

                Linus
--
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/