Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
On Dec 16, 12:17am, Christoph Hellwig wrote: } Subject: Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol Good morning, I hope the week is going well for everyone. Really late to this thread but am just getting caught d up after the holidays and putting out fires. > On Thu, Dec 15, 2016 at 08:50:39PM +, Tran, Quinn wrote: > > > Christoph, Qlogic was asked to support other Target Stack (SC ST) > > using the same qla2xxx.ko upstream driver. I had mentioned this > > during the last 2015 LSF get together. Hopefully, that gives a > > better context. The understanding that came away was Qlogic do > > what is needed to provide this support without creating any dead > > code. > Right. And what you're doing here is to create tons of pointless > and dead code. If you can support something else without clearly > making the in-kernel version worse it's all fine. This patch on the > other hand is complete crap and totally unacceptable in this form. It would be helpful to get an architectural overview of how Qlogic is proposing to interface SCST to the in-kernel Qlogic target driver code. Are you going to follow a model of having an analogue of tcm_qla2xxx for SCST? We've been running for 3-4 years on the SCST target interface driver we wrote to allow us to use the in-kernel Qlogic target driver code with SCST. We used the model of implementing the interface in something we referred to as the scst_qla2xxx module and the model seems solid. There was a bit of an impedance mismatch but the changes to the core kernel code were negligible. Our code is being scheduled for an upgrade to 4.4 and to chase down what have been reports about issues on 4 GBPS Qlogic cards. Any notion of a timeframe on the Cavium/Qlogic based solution? Have a good day. Dr. Greg }-- End of excerpt from Christoph Hellwig As always, Dr. G.W. Wettstein, Ph.D. Enjellic Systems Development, LLC. 4206 N. 19th Ave. Specializing in information infra-structure Fargo, ND 58102development. PH: 701-281-1686 FAX: 701-281-3949 EMAIL: g...@enjellic.com -- "Try to remove the color-problem by restarting your computer several times." -- Microsoft-Internet Explorer README.TXT -- 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
Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
On 12/15/2016 09:50 PM, Tran, Quinn wrote: > Qlogic was asked to support other Target Stack (SCST) using the same > qla2xxx.ko upstream driver. I had mentioned this during the last 2015 > LSF get together. Hopefully, that gives a better context. The > understanding that came away was “Qlogic do what is needed to provide > this support without creating any dead code”. > > [ ... ] > > The patches look like its copying a lot of fields without doing a whole > lot, instead the patches are translating the request from tcm_qla2xxx to > qla2xxx. Hello Quinn, Are you perhaps referring to the 2015 LSF/MM session I had proposed ([LSF/MM TOPIC] Unifying the LIO and SCST target drivers - https://marc.info/?l=linux-scsi&m=142122995413575)? The approach taken in patch 08/22 of this series is not what I had in mind. More in general, as you maybe know shim (translation) layers are not welcome in the Linux kernel. Simplifying the LIO core is still on my to do list. A possible starting point is the patch series I posted one year ago ([PATCH 00/20] SCSI target patches for kernel v4.4 - https://www.spinics.net/lists/target-devel/msg10788.html). That kind of simplification helps with reducing the maintenance burden for target drivers that support the two in-kernel SCSI target stacks without violating any of the established rules for kernel driver development. Bart. -- 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
Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
On Thu, Dec 15, 2016 at 08:50:39PM +, Tran, Quinn wrote: > Christoph, > > Qlogic was asked to support other Target Stack (SC ST) using the same > qla2xxx.ko upstream driver. I had mentioned this during the last 2015 LSF > get together. Hopefully, that gives a better context. The understanding > that came away was “Qlogic do what is needed to provide this support without > creating any dead code”. Right. And what you're doing here is to create tons of pointless and dead code. If you can support something else without clearly making the in-kernel version worse it's all fine. This patch on the other hand is complete crap and totally unacceptable in this form. -- 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
Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
Christoph,
Qlogic was asked to support other Target Stack (SC ST) using the same
qla2xxx.ko upstream driver. I had mentioned this during the last 2015 LSF get
together. Hopefully, that gives a better context. The understanding that came
away was “Qlogic do what is needed to provide this support without creating any
dead code”.
As for the cleanup patch(es), we fail to mention that conversation. The patch
attempts to remove any knowledge of TCM’s specific symbol and try to formalize
the interface. Qla2xxx driver will behave in a more generic manner when it
comes to Target Mode. All TCM knowledge will be confined to the tcm_qla2xxx
driver before communicating with QLA driver. The same goes for other target
stack.
The patches look like its copying a lot of fields without doing a whole lot,
instead the patches are translating the request from tcm_qla2xxx to qla2xxx.
As for T10-PI change in this series, the goal were the same : 1) remove TCM
knowledge/cleanup, 2) additional bug fixes were needed while testing with other
stacks.
Regards,
Quinn Tran
-Original Message-
From: on behalf of "Madhani, Himanshu"
Date: Thursday, December 15, 2016 at 11:29 AM
To: Christoph Hellwig
Cc: target-devel , Nicholas Bellinger
, "Malavali, Giridhar" ,
"linux-scsi@vger.kernel.org"
Subject: Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol
dependency
Resent-From:
Hi Christoph,
On 12/14/16, 1:18 PM, "Christoph Hellwig" wrote:
> - the new qla2x00_free_fcport is entirely pointless, please drop
> that part of the patch (and even if it wasn't pointless it should
> have been a patch on it's own)
> - please use struct names and not typedefs for your new structures
> - pretty much avery single items in your list should be a separate
> patch. And some of them are actively counterproductive and should
> be dropped:
>
>- Remove direct access of scsi_status field in se_cmd
> - Remove se_cmd from qlt_do_ctio_completion
> - Remove se_cmd access in srr code section
> - Move se_cmd struct outside of qla_tgt_cmd/qla_tgt_mgmt_cmd.
We combined the patches which includes fixes, enhancements and cleanups to
Support multiple target stacks, reviews from customer, and to improve code
maintainability. This patch tries to organize code logically which is spread
across qla_target and tcm_qla2xxx. We will split this patch into multiple
subset and resubmit the series.
Thanks,
- Himanshu
>
?�{.n�+���+%��lzwm��b�맲��r��zX��??�(��?��ܨ}���Ơz�&j:+v���?zZ+��+zf���h���~i���z�?�w���?&�)ߢ?f
Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
Hi Christoph, On 12/14/16, 1:18 PM, "Christoph Hellwig" wrote: > - the new qla2x00_free_fcport is entirely pointless, please drop > that part of the patch (and even if it wasn't pointless it should > have been a patch on it's own) > - please use struct names and not typedefs for your new structures > - pretty much avery single items in your list should be a separate > patch. And some of them are actively counterproductive and should > be dropped: > >- Remove direct access of scsi_status field in se_cmd > - Remove se_cmd from qlt_do_ctio_completion > - Remove se_cmd access in srr code section > - Move se_cmd struct outside of qla_tgt_cmd/qla_tgt_mgmt_cmd. We combined the patches which includes fixes, enhancements and cleanups to Support multiple target stacks, reviews from customer, and to improve code maintainability. This patch tries to organize code logically which is spread across qla_target and tcm_qla2xxx. We will split this patch into multiple subset and resubmit the series. Thanks, - Himanshu >
Re: [PATCH 08/22] qla2xxx: Refactore target code to remove symbol dependency
- the new qla2x00_free_fcport is entirely pointless, please drop that part of the patch (and even if it wasn't pointless it should have been a patch on it's own) - please use struct names and not typedefs for your new structures - pretty much avery single items in your list should be a separate patch. And some of them are actively counterproductive and should be dropped: - Remove direct access of scsi_status field in se_cmd - Remove se_cmd from qlt_do_ctio_completion - Remove se_cmd access in srr code section - Move se_cmd struct outside of qla_tgt_cmd/qla_tgt_mgmt_cmd. -- 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
