On Wed, Jul 05, 2023 at 09:22:08AM +0000, Rock Li(李宏伟) wrote: > > > On Tue, Jul 04, 2023 at 08:51:44AM +0800, lihongweizz wrote: > > > From: Rock Li <lihongweizz@xxxxxxxxxx> > > > > > > We met a crash issue as below: > > > ... > > > #7 [ff61b991f6f63d10] page_fault at ffffffffab80111e > > > [exception RIP: iscsi_iser_cleanup_task+13] > > > RIP: ffffffffc046c04d RSP: ff61b991f6f63dc0 RFLAGS: 00010246 > > > RAX: 0000000000000000 RBX: ff4bd0aalf7a5610 RCX: ff61b991f6f63dc8 > > > RDX: ff61b991f6f63d68 RSI: ff61b991f6f63d58 RDI: ff4bd0aalf6cdc00 > > > RBP: 0000000000000005 R8: 0000000000000073 R9: > > 0000000000000005 > > > R10: 0000000000000000 R11: 00000ccde3e0f5c0 R12: > > ff4bd08c0e0631f8 > > > R13: ff4bd0a95ffd3c78 R14: ff4bd0a95ffd3c78 R15: ff4bd0aalf6cdc00 > > > ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 > > > #8 [ff616991f6f63dc0] __iscsi_put_task at ffffffffc0bd3652 [libiscsi] > > > #9 [ff61b991f6f63e00] iscsi_put_task at ffffffffc0bd36e9 [libiscsi] > > > ... > > > > > > After analysing the vmcore, we find that the iser connection was > > > already stopped before abort handler running. The iser_conn is already > > > unbindded and released. So we add iser connection validation check > > > inside cleanup task to fix this corner case. > > > > > > Signed-off-by: Rock Li <lihongweizz@xxxxxxxxxx> > > > --- > > > drivers/infiniband/ulp/iser/iscsi_iser.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c > > > b/drivers/infiniband/ulp/iser/iscsi_iser.c > > > index bb9aaff92ca3..35dfbf41fc40 100644 > > > --- a/drivers/infiniband/ulp/iser/iscsi_iser.c > > > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c > > > @@ -366,7 +366,12 @@ static void iscsi_iser_cleanup_task(struct > iscsi_task > > *task) > > > struct iscsi_iser_task *iser_task = task->dd_data; > > > struct iser_tx_desc *tx_desc = &iser_task->desc; > > > struct iser_conn *iser_conn = task->conn->dd_data; > > > - struct iser_device *device = iser_conn->ib_conn.device; > > > + struct iser_device *device; > > > + > > > + /* stop connection might happens before iser cleanup work */ > > > + if (!iser_conn) > > > + return; > > > > And what prevents from iser_conn being not valid here? > > For example, in the flow: > > 1. Start iscsi_iser_cleanup_task > > 2. Get valid task->conn->dd_data > > 3. Pass this if (..) check > > 4. Context switch and release connection 5. iser_conn now points to > released > > memory. > > > > Thanks > > Hi Leon, > Thanks for your reply:) In case iscsi_stop_conn was executed cocurrently or > after iscsi_iser_cleanup_task, above issue would happen. > I've confirmed the values in iscsi_cls_conn and iscsi_conn instances from > vmcore: > > iscsi_stop_conn > ... > WRITE_ONCE(conn->state, ISCSI_CONN_FAILED); --- confirmed > ... > conn->transport->stop_conn => iscsi_iser_conn_stop > iscsi_conn_stop > ... > conn->c_stage = ISCSI_CONN_STOPPED; --- confirmed > conn->dd_data = NULL; --- confirmed > > The crash scene tells us that iscsi_stop_conn was executed before > iscsi_iser_cleanup_task start, the iser_conn instance was already released. It is by chance, it will be better to provide a fix which is race free. Thanks > > > > > > + device = iser_conn->ib_conn.device; > > > > > > /* DEVICE_REMOVAL event might have already released the device */ > > > if (!device) > > > -- > > > 2.27.0 > > >
