Re: block: don't check request size in blk_cloned_rq_check_limits()

[Hannes Reinecke <hare@suse.de>]

On 06/10/2016 04:18 PM, Mike Snitzer wrote:
> On Fri, Jun 10 2016 at  9:30am -0400,
> Hannes Reinecke <hare@suse.de> wrote:
>
>> On 06/10/2016 03:19 PM, Mike Snitzer wrote:
>>> On Mon, May 30 2016 at  3:24am -0400,
>>> Hannes Reinecke <hare@suse.de> wrote:
>>>
>>>> When checking a cloned request there is no need to check
>>>> the overall request size; this won't have changed even
>>>> when resubmitting to another queue.
>>>> Without this patch ppc64le on ibmvfc fails to boot.
>>>
>>> By simply removing the check aren't you papering over the real problem?
>>> Looking at Martin's commit f31dc1cd490539 (which introduced the current
>>> variant of the limits check) I'm not convinced it is equivalent to what
>>> he replaced.  I'll look closer in a bit.
>>>
>> The check itself is wrong, as we need (at least) to check the
>> max_hw_sectors here; the request is already fully assembled, so there is
>> a really good chance he's going beyond the max_sectors.
>> But trying the error still was found to be present.
>> So I decided to rip it out, as the overall value of this check is zero.
>
> fine, any chance you can improve the header to include these details.
> At least mention that commit f31dc1cd490539 incorrectly removed the
> max_hw_sectors checking.  And then please add these tags to a v2 repost:
>
> Fixes: f31dc1cd490539 ("block: Consolidate command flag and queue limit checks for merges")
> Reported-by: Mark Bergman <mark.bergman@uphs.upenn.edu>
> Acked-by: Mike Snitzer <snitzer@redhat.com>
> Cc: stable@vger.kernel.org # 3.7+
>
Okay, will be doing for a repost.

>>> Also you categorized your fix was for "ppc64le on ibmvfc"; whereas Mark
>>> has reported this issue (off-list) against x86_64.  By making it seem
>>> ppc64le specific I didn't take this patch to be generally applicable.
>>>
>> Well, it has been observed on ppc64. That doesn't mean _only_ ppc64 is
>> affected. If it were ppc64 only it should've been marked as such, right?
>
> If it is a generic problem, being specific about the hardware you saw it
> on leads idiots like me to filter unnecessarily ;)
>
> Though I'm curious what you mean by "it should've been marked as
> such".. "it" being what?  The patch?  And how would it have been marked
> as ppc64 only?
Exactly my point.
I was just trying to figure out what caused you to ignore the patch.

Anyway.

Will be reposting a v2 once Martin is happy.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html