| Message ID | 20180320210652.GA40906@xian (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 2018-03-21 at 05:06 +0800, kbuild test robot wrote: > Fixes: 793a6223beef ("mpt3sas: Cache enclosure pages during enclosure add.") > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > --- > mpt3sas_scsih.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > index c93c5c5..67a43957 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > @@ -1370,7 +1370,7 @@ mpt3sas_scsih_expander_find_by_handle(struct MPT3SAS_ADAPTER *ioc, u16 handle) > * This searches for enclosure device based on handle, then returns the > * enclosure object. > */ > -struct _enclosure_node * > +static struct _enclosure_node * > mpt3sas_scsih_enclosure_find_by_handle(struct MPT3SAS_ADAPTER *ioc, u16 handle) > { > struct _enclosure_node *enclosure_dev, *r; Hello Chaitra, Are you aware that if the 0-day test infrastructure suggests an improvement for a patch that the patch that that improvement applies to gets ignored unless either the patch is reposted with the improvement applied or that it is explained why the suggested improvement is inappropriate? Thanks, Bart.
Bart, > Are you aware that if the 0-day test infrastructure suggests an improvement > for a patch that the patch that that improvement applies to gets ignored > unless either the patch is reposted with the improvement applied or that it > is explained why the suggested improvement is inappropriate? Correct. I don't apply anything that causes a 0-day warning. The patch will be closed with "Changes Required" status in patchwork. Always build patch submissions to linux-scsi with: make C=1 CF="-D__CHECK_ENDIAN__"
Hi,
Further to that, in the second last hunk there is a very clear
functionality change:
@@ -8756,12 +8859,12 @@ _scsih_mark_responding_expander(struct
MPT3SAS_ADAPTER *ioc,
continue;
sas_expander->responding = 1;
- if (!encl_pg0_rc)
+ if (enclosure_dev) {
sas_expander->enclosure_logical_id =
- le64_to_cpu(enclosure_pg0.EnclosureLogicalID);
-
- sas_expander->enclosure_handle =
- le16_to_cpu(expander_pg0->EnclosureHandle);
+ le64_to_cpu(enclosure_dev->pg
0.EnclosureLogicalID);
+ sas_expander->enclosure_handle =
+ le16_to_cpu(expander_pg0->EnclosureHandle);
+ }
if (sas_expander->handle == handle)
goto out;
Note that the assignment to sas_expander->enclosure_handle is now
dependent on enclosure_dev being non-NULL.
Busy applying the patch to 4.16 - and now I have no idea whether that
functionality change should be part of the change or not. Having worked
through the rest of the patch it seems good otherwise (Keeping in mind
that I'm not familiar with the code in question, nor do I normally work
on kernel code, and this is definitely the first time I took a peek
anywhere near the IO subsystem).
Kind Regards,
Jaco
On 28/03/2018 23:54, Martin K. Petersen wrote:
> Bart,
>
>> Are you aware that if the 0-day test infrastructure suggests an improvement
>> for a patch that the patch that that improvement applies to gets ignored
>> unless either the patch is reposted with the improvement applied or that it
>> is explained why the suggested improvement is inappropriate?
> Correct. I don't apply anything that causes a 0-day warning. The patch
> will be closed with "Changes Required" status in patchwork.
>
> Always build patch submissions to linux-scsi with:
>
> make C=1 CF="-D__CHECK_ENDIAN__"
>
Hi Martin, Bart, I've not seen additional feedback on this (I may simply not be CCed). I've applied the patch to one of our hosts where we've had endless IO lockups (with MQ enabled the host died within a day or two, sometimes sub one hour, without it typically ran for about two weeks). With this patch (on top of 4.16) we're now at four days and 17 hours, with IO still going strong (including a mdadm reshape to add a disk, as well as a rebuild on a drive that failed - concurrently on two different arrays, same controller). Very subjective, but the host also feels more responsive under heavy IO load. What can I do from my side (I've got some development experience) to help push this patch forward? Kind Regards, Jaco On 28/03/2018 23:54, Martin K. Petersen wrote: > Bart, > >> Are you aware that if the 0-day test infrastructure suggests an improvement >> for a patch that the patch that that improvement applies to gets ignored >> unless either the patch is reposted with the improvement applied or that it >> is explained why the suggested improvement is inappropriate? > Correct. I don't apply anything that causes a 0-day warning. The patch > will be closed with "Changes Required" status in patchwork. > > Always build patch submissions to linux-scsi with: > > make C=1 CF="-D__CHECK_ENDIAN__" >
On Tue, 2018-04-10 at 09:15 +0200, Jaco Kroon wrote: > I've not seen additional feedback on this (I may simply not be CCed). > > I've applied the patch to one of our hosts where we've had endless IO > lockups (with MQ enabled the host died within a day or two, sometimes > sub one hour, without it typically ran for about two weeks). With this > patch (on top of 4.16) we're now at four days and 17 hours, with IO > still going strong (including a mdadm reshape to add a disk, as well as > a rebuild on a drive that failed - concurrently on two different arrays, > same controller). Very subjective, but the host also feels more > responsive under heavy IO load. > > What can I do from my side (I've got some development experience) to > help push this patch forward? Hello Jaco, If you want to follow mpt3sas development please consider to subscribe to the linux-scsi mailing list. Recently Broadcom posted a new mpt3sas patch series. See also https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg72823.html. Bart.
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index c93c5c5..67a43957 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -1370,7 +1370,7 @@ mpt3sas_scsih_expander_find_by_handle(struct MPT3SAS_ADAPTER *ioc, u16 handle) * This searches for enclosure device based on handle, then returns the * enclosure object. */ -struct _enclosure_node * +static struct _enclosure_node * mpt3sas_scsih_enclosure_find_by_handle(struct MPT3SAS_ADAPTER *ioc, u16 handle) { struct _enclosure_node *enclosure_dev, *r;
Fixes: 793a6223beef ("mpt3sas: Cache enclosure pages during enclosure add.") Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- mpt3sas_scsih.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)