Re: patch "[PATCH] [SCSI] mpt2sas: Fix for device scan following host reset" was seriously submitted to be applied to the 3.10-stable tree?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 24, 2013 at 12:18:07PM +0530, Reddy, Sreekanth wrote:
> Replied inline..
> 
> >-----Original Message-----
> >From: gregkh@xxxxxxxxxxxxxxxxxxx [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> >Sent: Tuesday, July 23, 2013 9:46 PM
> >To: Reddy, Sreekanth
> >Cc: stable@xxxxxxxxxxxxxxx; JBottomley@xxxxxxxxxxxxx; Nandigama,
> >Nagalakshmi
> >Subject: Re: patch "[PATCH] [SCSI] mpt2sas: Fix for device scan following host
> >reset" was seriously submitted to be applied to the 3.10-stable tree?
> >
> >On Tue, Jul 23, 2013 at 04:12:21PM +0530, Reddy, Sreekanth wrote:
> >> Hi Greg,
> >>
> >> Here is the summary of this patch.
> >>
> >> After host reset, the driver breaks from the device scan loop only
> >> when ioc_status of config page request equals
> >> MPI2_IOCSTATUS_CONFIG_INVALID_PAGE.  If the firmware returns some
> >> other ioc_status such as MPI2_IOCSTATUS_INVALID_FUNCTION then this
> >> results to infinite loop.
> >>
> >> As this issue is critical since the driver may go in to infinite loop,
> >> so we thought to add this patch in stable kernel also.
> >
> >Why was this text not in the changelog information for the patch itself?
> >That would have help justified why it was submitted to the stable tree.
> >
> >Why would the firmware return some other status values?  Is this a firmware
> >failure, or a driver failure to handle valid firmware errors?
> 
> For firmware returns MPI2_IOCSTATUS_INVALID_FUNCTION ioc_status if the
> config page request is not supported by IOC or the function code is
> undefined. Here driver fails to handle the valid firmware errors.

But you are taking a "valid" firmware error, and spamming the kernel log
with a ton of messages when that happens.  You are also adding a much
more "informational" log messages, none of which are really needed,
right?

> >
> >Note, the patch is bigger than I like to see in stable patches, are you _sure_ it's
> >needed?
> 
> As this issue will occur very rarely and also this patch is bigger in
> stable patches, so it's fine to drop this patch from your stable
> patches queue.  But it will be good if you include this patch only if
> you feel convenient. 

I think this patch just adds lots of useless messages that a user can't
do anything with, and will just annoy them.  There is probably a real
bugfix burried in there somewhere, but it's too hard to find.  In the
future, please split out bugfixes from "let's tell the user everything
that my driver just did because they might want to know that" type of a
change please.

So for a stable kernel, I'm not going to take this, it's just too noisy.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]