On Thu, 2014-11-20 at 12:57 +0200, Sagi Grimberg wrote: > On 11/20/2014 8:09 AM, Nicholas A. Bellinger wrote: > > Hi Sagi & Co, > > > > On Wed, 2014-11-19 at 19:18 +0200, Sagi Grimberg wrote: > >> Hey Nic & Co, > >> > >> This series mainly consists of error flow fixes: > >> Patches 1-2: Some logging refactoring. It is much easier to > >> instruct a user to increase the log level in this case. > >> Patches 3-10: Some error flow fixes for live target stack shutdown > >> and cable pull with stress IO scenarios. > >> Patches 11-15: Remove t10_pi attribute and fix a crash in due to a bad > >> dereference. > >> Patches 16-19: Fixes in the area of bond failover scenarios > >> Patch 20: Workaround for live target stack unload in the presence > >> of multiple (50+) of active sessions. > >> Patch 21: Get rid of redundant wait for logout completion. > >> > >> While this set makes things better, there is still some work > >> left to do especially in the area of multi-session error flows. > >> > >> The set applies cleanly on master branch (with or without the isert > >> patch from Chris Moore). > >> > >> Changes from v0: > >> - Fix CHAP and mutual CHAP login breakage introduced in patch #9 > >> kudos to Slava for staying alert > >> - Fix mthca/mlx4 HCAs breakage introduced in patch #13 (non supporting PI) > >> kudos to Adam for finding this quickly > >> - Removed patch "Remove t10_pi attribute altogether". > >> Instead, included patch "Expose supported protection ops according to t10_pi" > >> if t10_pi is not set we fall to SW mode. > >> - Added patch #21 to get rid of redundant wait for logout completion > >> > >> Sagi Grimberg (21): > >> iser-target: Use debug_level parameter to control logging level > >> iser-target: Adjust log levels and prettify some prints > >> iscsi-target: Add call to wait_conn in establishment error flow > >> iser-target: Destroy the connection when getting a connect error > >> event > >> iser-target: Initiate connection termination only once > >> iser-target: Don't deffer disconnected handler to a work > >> iser-target: Reject connect request in failure path > >> iser-target: Introduce state ISER_CONN_FULL_FEATURE > >> iser-target: Parallelize CM connection establishment > >> iser-target: Use kref_get_unless_zero in connected_handler > >> iser-target: Move pi context allocation to a function > >> iser-target: Move protection information handling to a routine > >> iser-target: Allocate PI contexts dynamically > >> iser-target: Fix NULL dereference in SW mode DIF > >> iscsi,iser-target: Expose supported protection ops according to > >> t10_pi > >> iser-target: Acquire conn_mutex when changing connection state > >> iser-target: Move cma_id setup to a function > >> iser-target: Handle ADDR_CHANGE event for listener cm_id > >> iser-target: Deffer the last part of wait_conn to a work > >> iser-target: Work-around live target stack shutdown resource cleanup > >> iser-target: Get rid of redundant wait for logout_comp > >> > >> drivers/infiniband/ulp/isert/ib_isert.c | 1030 +++++++++++++++++------------ > >> drivers/infiniband/ulp/isert/ib_isert.h | 42 ++- > >> drivers/target/iscsi/iscsi_target_login.c | 10 +- > >> 3 files changed, 670 insertions(+), 412 deletions(-) > >> > > > > Thanks for this detailed series of active I/O shutdown fixes, and > > apologies for the delayed response to -v0 posting. > > > > So which patches would you consider as >= v3.14.y + v3.10.y material..? > > I would say that all patches that don't refer to PI which are: > > iser-target: Move pi context allocation to a function > iser-target: Move protection information handling to a routine > iser-target: Allocate PI contexts dynamically > iser-target: Fix NULL dereference in SW mode DIF > iscsi,iser-target: Expose supported protection ops according to t10_pi > Ok, so these five should be CC'ed to >= v3.15.y (sorry, typo on v3.14.y above) to address tDIF specific regressions Is there anything else seperate from the DIF fixes that needs to go back further..? > > > > My only immediate concern is that a fair amount of these patches will > > need to be CC'ed for stable, so a re-spin without patch #1 + #2 would > > make life easier for myself and stable maintainers when applying these > > fixes to future stable versions. Would you mind posting a -v2..? > > Hmmm, given the fact I have ~25 more patches piped on top of this > would be a bit of a hassle... > Yeah, sorry for the extra overhead here, but I think it will save alot of future headaches when backporting the bugfixes to stable. > > > > To the logging changes themselves, I'd prefer sticking with > > CONFIG_DYNAMIC_DEBUG to match iscsi_target_mod + friends instead of a > > new module parameter unless there is something you need for debugging > > these tough shutdown cases that pr_debug can't otherwise do..? > > There are a couple of reason why I suggested the logging changes: > > 1. I find it easier to get end users to provide additional info in this > way. > > 2. I desperately need log levels rather the all-or-nothing existing > way. pr_debug adds too much noise when working on control plain flows, > and it is just not feasible (for me at least) to inspect logs with > pr_debug on for 100+ active sessions doing IO. And pr_info will be > logged by default so I can't use it for "debug info". > > 3. I find __func__ print extremely useful and would like to avoid > explicitly adding it to each print in the code. Moreover I am thinking > of adding some identifier to the prints in the multi-connection case > where it is important to distinguish between connections. Currently I > print the pointer, but I was thinking of something nicer like what > iscsi/drbd is doing. > > Would it help if I try and break patches 1+2 to stable-applicable > and not-stable-applicable? > Yes, please. Put all the bugfixes that need to be CC'ed for stable at the head of the series, and then remaining changes + improvements that won't be CC'ed to stable at the tail of the series. > Having said that, I'll lose these changes if you have a strong > objection here. > I don't have an objection to the log changes if they make things easier for you to debug. As for the ~25 patches beyond this series, are alot of them going to be backported to stable as well..? If so, it would probably make sense to include the log changes at the end of that new series, instead of at the end of a re-spun -v2 of this series.. --nab -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
