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
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...
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? Having said that, I'll lose these changes if you have a strong objection here. Cheers, Sagi. -- 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
