Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <856ccad2-19a4-32b4-b41f-5a230a55ee30@oracle.com>
Date:   Tue, 22 Nov 2022 12:15:45 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH v6] scsi:iscsi: Fix multiple iscsi session unbind event
 sent to userspace
To:     Wenchao Hao <haowenchao22@gmail.com>
Cc:     Wenchao Hao <haowenchao@huawei.com>, Lee Duncan <lduncan@suse.com>,
        Chris Leech <cleech@redhat.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org, linfeilong@huawei.com
References: <20221108014414.3510940-1-haowenchao@huawei.com>
 <ad54a5dc-b18f-e0e6-4391-1214e5729562@oracle.com>
 <89692b2b-90f7-e8e8-fa77-f14dbe996b72@huawei.com>
 <418c7f6f-0bc3-45bb-2abf-e866df6f4b62@oracle.com>
 <CAOptpSO6=sUPUwgj1og8088djiNA=Bw9um0p024L=0Gb=-ja5w@mail.gmail.com>
Content-Language: en-US
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <CAOptpSO6=sUPUwgj1og8088djiNA=Bw9um0p024L=0Gb=-ja5w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?amRkTy95WUs5anJPMlhDL1hjQWxQa0dLZkI2Skg2KzNVRVVYNlRmc0hFL0lq?=
 =?utf-8?B?UjVDNmU0ZkM2TmZYV3RudTYzZWd1K0FzaVRHekZqalVKNndoKzkwMjNWaE5D?=
 =?utf-8?B?SzFWQXpDZHJCdGViMVVHdm9PNnBOODd2ekIvWnE1ME9VclVZSVJVRkdZQjlr?=
 =?utf-8?B?MUdmOGkwdHFpT2V1dHgwTTd2by8xc3hSV29jZEl1UzdjZmN1U014S3Z0azJE?=
 =?utf-8?B?NTdtNzdINEZ2UElYSEpaSlZhVERVUzd5VTZBeldDQllMU3RBeGx2bGJGUEdz?=
 =?utf-8?B?N2M1ZTM3cmFhWXRKU0kwcGFsbVZvQnYxRWVoQTFsYUtiNmhsYVhENFJML3Bq?=
 =?utf-8?B?RkM1N2h0UDJVOVhyQkYrK0Q3QjJ3bW40UjdQUHFlMjFLbFFSczY0Qmwrak5V?=
 =?utf-8?B?ajRxdXlubmtyTTY5U3hYNWtnNHFsRWRzdEhla0NQaXo2MWVXQTg4dnFzOTJq?=
 =?utf-8?B?VjhXazBTZHRkUmxlTm9xckFzbGZjNmNzNHhvUFVKMmt6bExKelpNMXE0bnZT?=
 =?utf-8?B?YS9HVG1TSG05MkVwdGNqME5jYk96cVhJYjVBNkorTlMyWnAxYVY4K2YwSkI3?=
 =?utf-8?B?enEreG82SmpRejJqck9YVUJ0M0QvSDdJcG5GdlZXUnI2SFFWbmdIaUp6Vm80?=
 =?utf-8?B?WFVxUm5OeWcvV0ZtRGQ0VkE1MStHM1lHOFowRGNWNmVRMzFtaVAvRU9Ca0V5?=
 =?utf-8?B?RzlRUVE0dmdMUkE0QmVzNmpYMXVlY2lwVDZ3YThvaXM0ckxLRW5qUUhJYmQ3?=
 =?utf-8?B?UmUvT3NNQ25XMWJ2MVNoRzFsa0JGdVZpVmJSVGU0QlFCNzZFUmlOOUFWR0ZI?=
 =?utf-8?B?NEJGN1dEZXFabHFDNGVkWTdyMHVvTjBjc3Q3SUdKbWNWNTd0QmNtSlpEeWtK?=
 =?utf-8?B?UmZDaWYwOXcybnZ4bGlpNHY3ZncxMHhEM1k5K09STmZLS2FySGV6bDdDT0RQ?=
 =?utf-8?B?NktTMjZ5VXB0cC9NZjB0Vk10elRSdUx4bXFFeGgveWtxTzNRUlBkdTJ1SDQw?=
 =?utf-8?B?eEkzczNQWnhRQVc5Z0J5UDNLY3JLNzZMYTlMRFdmMHpvdEJGeXd2Z0hSOVp0?=
 =?utf-8?B?RzhNMndLRVhtd21CL2VaUVE0L0pqaHp5THV5RUxwUG1iWFl1MUpheXJPa2hM?=
 =?utf-8?B?TFRReDBHSnU1S0IzVFo1NTROazdzeS9BRmJJTGhJSGhVakFZQjhwYzAzTE9z?=
 =?utf-8?B?ZGM3KzBrZkNaRmhQUWthcnlpNzNHOFgrVmNuNUhGRGFyT0pRL1UzUXJyMDZ6?=
 =?utf-8?B?b1RtaXNHTTh0SFQ2ejQwS0VaRXhYbkZzb2N1K1ZuN3ZLSmRzbHoxLzhKY1Rk?=
 =?utf-8?B?elZOUVlPeW9mK3NReHVydjI3MEZydytqbEM5cDExS0xHMXNQUWJwMkphaU5v?=
 =?utf-8?B?TFl4MVlRTVFXMkVHRC9yb09HOWM3WUkrN1hkWUFYN1pVNDVraThUL1VqRWd4?=
 =?utf-8?B?ZENJdXBQazN2NVZKdXQwR1VuenFhUXhSQm9IYzQ4U3NIWnZsbExxNHRwY0hH?=
 =?utf-8?B?djNucWV0cHhDN1NKT2V5Z0Y4blJRdHJva2t4VURNRnVuenpGeGE5SXVuZFFO?=
 =?utf-8?B?NWllZmFOQkZSQWU5VnpaVFJHTkhZZVdiRWZ3THpGTnE4bW5PL2xPU084ZDBN?=
 =?utf-8?B?SXRHakp6Q2JSQlNPRUtaM0FIcGlaNFY0Ky83eExybDJXL2loUlc2cGxNeVJx?=
 =?utf-8?B?WE5QQnRJTE4waHU4WWtOT0NReTRtZjRybUYvOWF0Z3Z1SVV6TGhEWkNQSUVS?=
 =?utf-8?B?ejZMRFBDM2RwMWJ3NDJyVHl1T3ZUL0pYZURtRDFQR25URmRNSGxnMDliVkVW?=
 =?utf-8?B?K0tJUUtOTXlCcFhBNVpneTkrUEZvVzliNWtXSnY5UzdQMDAyTnVrV1NFZ296?=
 =?utf-8?B?Q2hhUG5RUWdYRUVaU1J6V3V5TlNaZWVSQklpWUdtREZ6UWxBK3h3V25ibmYx?=
 =?utf-8?B?ckN0Vzl0M2lyTzR3Rjc0NllsR2hyamZlT2VkODRnbnpHNjBUMWoxV3JuRGZn?=
 =?utf-8?B?akVHa2FWSzdXQk84MFBoRmVUdmlXdmFRdzRQZ1B3R2taeFZqVTVLQ3M0dThI?=
 =?utf-8?B?UzRDWXhwSXE2am4yNGNhS2pxVlFBU1VLcVBHQmljdXF1ZklQeFppcElTeG83?=
 =?utf-8?B?YmhJNHIyS3YrdUVDYTFsQ2haazRJdk81UmFPZGlZK3J2T0FRZ3BDNjVZS2Zx?=
 =?utf-8?B?bHc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?azNidDI0K2RvRFNtR3Rjeks0L2paakN2K1h2c1F3QmhZVFloTm1sSTMyMjJT?=
 =?utf-8?B?SjBhYU85YTR5VVRjUjR0Nk1wUTY2MHV4RmJheHNEQUI5bWdpUkplc0J4ODBV?=
 =?utf-8?B?NG1rYXlOOGlaQ2QrL1g0bWpzdGJoODBqbld4UndCejZDMFI3OGExQjlNdU5N?=
 =?utf-8?B?NnMrK2xxUEtMT01LMjJzLzk4ckhzbW9rUUdIUG9qMnhOWlVEVk5NblNwMEhF?=
 =?utf-8?B?NCtueU8rajBQbkNMRDN1QzFTTlhyUTY5YysrL0x1bW5GSnhXNnQ1NFU2OGxX?=
 =?utf-8?B?MVkzeHdGZlNBM0pFTVdBYWV3Z2lQUUVsYW54dnFhVlJaSG0xWk12SEtjUkVE?=
 =?utf-8?B?MGM2RVJhVm9kUysvampaWVdtcGFTaFllKzVBWUE2dGMza1BBUEtEVktPM0pt?=
 =?utf-8?B?R0tSNGFwQW5KdzNNVWF5cnQzU3czT2o2V1A1NTVWZ1pFSXpsTGF0QjJQVHBR?=
 =?utf-8?B?S1FYY2lTQ0RMeFZLQWlHMHRpMGdpdHg2bGtKTHl1bzFJQS9HUGdDRGlSSXIr?=
 =?utf-8?B?anExWFFBVlFHOGhuTHdwSHcrb1MxMVp4Tm4wQTBCbm9NZm0rQS96amQwYjQ5?=
 =?utf-8?B?NVpvakFCNFRoMjdFbFgrQS9vMVJtMXBXRkFIV2daZW9DMStLNTQrcHNseVpl?=
 =?utf-8?B?YjVoblZaWDJNQ05MNWtGeWpLYS9Ja2lsa3F4blN5SVdlM0VORzlpSjJvK2xJ?=
 =?utf-8?B?a0JhWWw3Nm1RZ21lNEdueXM3ckw4aTNUdCs5dW1oYTBuVDAzK3dVMWZxMjQ0?=
 =?utf-8?B?OHpxSlg3SnVGUHlyQWpWNUp5amVQSGJ4SG9EU1BBSDcvZ3ZsTWxOd2tydU1s?=
 =?utf-8?B?dVUrNHhKQkJXOE1XVjE5MUxicnZKM1ZDTzN3dzdOaG5NL0o3aFB1N1huanFT?=
 =?utf-8?B?TDY3RW1ncmZJRk1weksyYlNqTnpNemk3Y3V5VzBUY1c4SGpYcDRHZjEweC9q?=
 =?utf-8?B?UTA3UWNRcXF1K0hEUTFHVDdRTzZQU2J1NmlYbjVKc3ZVTFBmcDRBazN0OVh3?=
 =?utf-8?B?Z3lTSUE0endUT1hUaHpHWGxtRU5UdlU1R0c2dWdad0g3YzdqcElDbkFPcC8z?=
 =?utf-8?B?ZDVUM21ScDRqOE5qOWJCb2wxcC8vT1FJdEJxNFdYZ1Y1bEI4QmlyOGhIN2pq?=
 =?utf-8?B?QSt3dFhzMUhnMGRTQWxQR1hkYUVrMm9zUk1XSVM5UEIvKzlodFpQcTBXV3pO?=
 =?utf-8?B?OVNTUlk4Wmh2YmZhSHFaczRJR2NqWk5SMVFpaFQzNndpQjhiME0xVDFJNnh5?=
 =?utf-8?B?K3lDUVdCc2JrcnZPMzRlV1dnQTFBY2FXNytodzAvdUtnT1lMZnhCRmtLWFJD?=
 =?utf-8?B?eERGeWo2N043emRIUGJtMURVRnU3U3M2UWkzTWg1VUlrS0ZMVC85MjA0WmdO?=
 =?utf-8?B?VHRzbVg3RkNOMXc9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 345885de-5bd6-45e3-8127-08daccb59406
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2022 18:15:47.2979
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8P5VOQdv5gdYMagSzuBoGErheHh5naT6/3ZXLm56imLr8bY1bDlkkx9IwxwtxX6PUc+PhQiIgRp2DQSFsQFuuc/kOJJ01WHzKIyz5kOh40w=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5294
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-22_11,2022-11-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 spamscore=0 malwarescore=0
 phishscore=0 mlxlogscore=999 bulkscore=0 suspectscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211220140
X-Proofpoint-GUID: KY1FGqpu4MsprukYQwYZV7mJ9RztFHC_
X-Proofpoint-ORIG-GUID: KY1FGqpu4MsprukYQwYZV7mJ9RztFHC_
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/22/22 11:29 AM, Wenchao Hao wrote:
> On Wed, Nov 23, 2022 at 1:04 AM Mike Christie
> <michael.christie@oracle.com> wrote:
>>
>> On 11/21/22 8:17 AM, Wenchao Hao wrote:
>>> And the function looks like following after change:
>>>
>>> static void __iscsi_unbind_session(struct work_struct *work)
>>> {
>>>       struct iscsi_cls_session *session =
>>>                       container_of(work, struct iscsi_cls_session,
>>>                                    unbind_work);
>>>       struct Scsi_Host *shost = iscsi_session_to_shost(session);
>>>       struct iscsi_cls_host *ihost = shost->shost_data;
>>>       unsigned long flags;
>>>       unsigned int target_id;
>>>
>>>       ISCSI_DBG_TRANS_SESSION(session, "Unbinding session\n");
>>>
>>>       /* Prevent new scans and make sure scanning is not in progress */
>>>       mutex_lock(&ihost->mutex);
>>>       spin_lock_irqsave(&session->lock, flags);
>>>       if (session->target_state != ISCSI_SESSION_TARGET_SCANNED) {
>>
>> What was the reason for not checking for ALLOCATED and freeing the ida
>> in that case?
>>
> 
> target_state would be in "ALLOCATED" state if iscsid died after add
> session successfully.
> When iscsid restarted, if the session's target_state is "ALLOCATED",
> it should scan
> the session and the target_state would switch to "SCANNED".
> 
> So I think we would not call in __iscsi_unbind_session() with
> session's target_state
> is ALLOCATED.

Makes sense for the normal case.

The only issue is when __iscsi_unbind_session is called via
iscsi_remove_session for the cases where userspace didn't do
the  UNBIND event. Some tools don't do unbind or open-iscsi
sometimes doesn't if the session is down. We will leak the ida,
so you need some code to handle that.