Re: [PATCH-v2 1/3] percpu_ida: Make percpu_ida_alloc + callersaccept task state bitmask

From: Peter Zijlstra
Date: Mon Jan 20 2014 - 06:34:45 EST


On Mon, Jan 20, 2014 at 03:44:44AM +0000, Nicholas A. Bellinger wrote:
> From: Kent Overstreet <kmo@xxxxxxxxxxxxx>
>
> This patch changes percpu_ida_alloc() + callers to accept task state
> bitmask for prepare_to_wait() for code like target/iscsi that needs
> it for interruptible sleep, that is provided in a subsequent patch.
>
> It now expects TASK_UNINTERRUPTIBLE when the caller is able to sleep
> waiting for a new tag, or TASK_RUNNING when the caller cannot sleep,
> and is forced to return a negative value when no tags are available.
>
> v2 changes:
> - Include blk-mq + tcm_fc + vhost/scsi + target/iscsi changes
> - Drop signal_pending_state() call

Urgh, you made me look at percpu_ida... steal_tags() does a
for_each_cpus() with IRQs disabled. This mean you'll disable IRQs for
multiple ticks on SGI class hardware. That is a _very_ long time indeed.

Then there's alloc_global_tags() vs alloc_local_tags(), one gets an
actual tag, while the other only moves tags about -- semantic mismatch.

I do not get the comment near prepare to wait -- why does it matter if
percpu_ida_free() flips a cpus_have_tags bit?

Given I don't understand this comment, its hard for me to properly
review the proposed patch series.

Help?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/