Re: [GIT PULL] target fixes for v3.13

[Nicholas A. Bellinger]

On Fri, 2014-01-17 at 17:29 -0800, Linus Torvalds wrote:
> On Thu, Jan 16, 2014 at 4:09 PM, Nicholas A. Bellinger
> <nab@xxxxxxxxxxxxxxx> wrote:
> >
> > This change allows the percpu_ida tag allocator to optionally use
> > interruptible sleep that iscsi-target expects, while still leaving the
> > functionality + interface for existing percpu_ida consumers unchanged.
> 
> I'm not pulling this. Passing in TASK_RUNNING to prepare_to_wait() is
> insane (because if it ever were to actually wait, that would be a
> bug), and afaik this would be the first time anybody ever does that.
> 
> Yes, yes, it may "work", but I'm not pulling that kind of hack just
> before a release.
> 
> Quite frankly, it looks like you want to have a tristate argument ("no
> wait", "wait interruptibly", "wait uninterruptibly") to that
> percpu_ida_alloc() function. Fine. But dammit, using this kind of
> hackery, and then having two *different* calling conventions (one
> mis-using the gfp_t for legacy reasons, and one now using the task
> state flags in odd ways) is just not acceptable.
> 
> Now, neither of those two is perfect, but I can see why you want to
> use the task state ones to say which kind of interruptible you want..
> But I really don't like suddenly having a
> prepare_to_wait(TASK_RUNNING) caller without any discussion, and I
> *really* don't like having two completely different models for this
> hack.
> 
> So quite frankly, I'd much prefer:
> 
>  - talk to the scheduler people, and make them aware of the fact that
> you are going to pass in TASK_RUNNING to prepare_to_wait(). It works
> with the code as-is, as long as you don't actually then wait.
> 
>  - Don't do that wrapper function with a totally different calling
> convention logic. Instead, just change all the callers explicitly.
> From a quick look, you really only have a couple of cases:
> 
>   (a) target/iscsi, which wants the new ternary argument
> 
>   (b) vhost/scsi.c, which uses GFP_ATOMIC and can be changed to TASK_RUNNABLE
> 
>   (c) block/blk-mq-tag.c, which already hates the current insane
> thing, and uses __GFP_WAIT and (gfp & ~__GFP_WAIT) and other hacks,
> and is obviously *very* aware of the internal hackery in the current
> percpu_ida_alloc() argument. So I'm getting the feeling that that
> whole thing might actually be *happier* with the TASK_xyz flags.
> 

Sure.

> 
> So I really think this needs cleanup, and that hacky "passing in
> TASK_RUNNING to prepare_to_wait()" needs to be made official. And yes,
> that implies that it's too late to try to push this through for 3.13,
> this goes into the next merge window and can be backported.
> 
> Added the appropriate people to the Cc..
> 

Just sent out a series as requested for review with CC's for Ingo +
Peter.

Jens, please review the blk-mq changes.

Thanks!

--nab

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