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.kennedy@xxxxxxxxxxxx>
> >>Signed-off-by: James Smart <james.smart@xxxxxxxxxxxx>
> >>---
> >[...]
> >
> >>+ 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
jthumshirn@xxxxxxx +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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
