On Wed, Jan 18, 2017 at 06:45:46PM -0800, James Smart wrote:
> 
> 
> On 1/18/2017 2:11 AM, Johannes Thumshirn wrote:
> >On Tue, Jan 17, 2017 at 05:20:46PM -0800, James Smart wrote:
> >>NVME Initiator: Base modifications
> >>
> >>This is part B of parts A..F.
> >>
> >>Part B is limited to lpfc_attr.c: lpfc attribute modifications
> >>
> >>*********
> >>
> >>Refer to Part A for a description of base modifications
> >>
> >>Signed-off-by: Dick Kennedy <dick.kenn...@broadcom.com>
> >>Signed-off-by: James Smart <james.sm...@broadcom.com>
> >>---
> >[...]
> >
> >>+   len += snprintf(buf + len, PAGE_SIZE - len,
> >>+                   "%s%d WWPN x%llx WWNN x%llx DID x%06x %s\n",
> >>+                   "NVME LPORT lpfc",
> >Is it the lack of coffee or should it be
> >                     "NVME LPORT lpfc%d WWPN x%llx WWNN x%llx DID x%06x %s\n"
> >
> >I think you're doing it to not hit the 80 chars limit, but then there are
> >way more offenders than that one, so...
> 
> The line split is certainly due to the 80 limit and have that issue a lot.
> 
> As for what the string name should be - I agree with you. something is
> confused.
> 
> >>+int
> >>+lpfc_emptyq_wait(struct lpfc_hba *phba, struct list_head *q, spinlock_t 
> >>*lock)
> >>+{
> >>+   int cnt = 0;
> >>+
> >>+   spin_lock_irq(lock);
> >>+   while (!list_empty(q)) {
> >>+           spin_unlock_irq(lock);
> >>+           msleep(20);
> >>+           if (cnt++ > 250) {  /* 5 secs */
> >>+                   lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,
> >>+                                   "0466 %s %s\n",
> >>+                                   "Outstanding IO when ",
> >>+                                   "bringing Adapter offline\n");
> >>+                           return 0;
> >>+           }
> >>+           spin_lock_irq(lock);
> >>+   }
> >>+   spin_unlock_irq(lock);
> >>+   return 1;
> >>+}
> >>+
> >Aren't you using lpc_emptyq_wait() in patches prior to that already? This
> >breaks git bisect. Pleas test-build (ideally + checkpatch and sparse/smatch)
> >each patch in the series individually.
> 
> I called out - in patch2 - that Patches 2 through 7, known as parts A..F,
> area really one big patch. They will not follow the git bisect rules.  I
> could have sent them in one huge patch, but chose to break them up.
> Unfortunately, the mods accumulated over time with lots of reworks -
> creating a base that was too intertwined to put into small functional
> patches without spending oodles of time to carve them up.   I hope you can
> bear with me on this set and review the 7 pieces as one big patch.

For review one big patch clearly is a huge problem, I agree. But the final
merged version must not break bisects. So we can probably defer the discussion
to the end of the merging.

Byte,
        Johannes
-- 
Johannes Thumshirn                                          Storage
jthumsh...@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to