From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424208AbcBQWhM (ORCPT ); Wed, 17 Feb 2016 17:37:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59274 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1423707AbcBQWhK (ORCPT ); Wed, 17 Feb 2016 17:37:10 -0500 Date: Wed, 17 Feb 2016 14:37:08 -0800 From: Chris Leech To: open-iscsi@googlegroups.com Cc: Lee Duncan , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Use ida_simple for SCSI iSCSI transport session id Message-ID: <20160217223708.w35zn3h66gwv5jlw@straylight.hirudinean.org> Mail-Followup-To: Chris Leech , open-iscsi@googlegroups.com, Lee Duncan , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org References: <1455298733-18651-1-git-send-email-lduncan@suse.com> <1455299691.2396.45.camel@HansenPartnership.com> <20160215182653.pvo6crry2wqwgjso@straylight.hirudinean.org> <56C36D0C.3030002@cs.wisc.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56C36D0C.3030002@cs.wisc.edu> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 16, 2016 at 12:40:12PM -0600, Mike Christie wrote: > On 02/15/2016 12:26 PM, Chris Leech wrote: > > On Fri, Feb 12, 2016 at 09:54:51AM -0800, James Bottomley wrote: > >> On Fri, 2016-02-12 at 09:38 -0800, Lee Duncan wrote: > >>> The scsi_transport_iscsi module already uses the ida_simple > >>> routines for managing the target ID, if requested to do > >>> so. This change replaces an ever-increasing atomic integer > >>> that tracks the session ID itself with the ida_simple > >>> family of routines. This means that the session ID > >>> will be reclaimed and can be reused when the session > >>> is freed. > >> > >> Is reusing session ID's really a good idea? For sequential sessions it > >> means that the ID of the next session will be re-used, i.e. the same as > >> the previous sessions, which could lead to target confusion. I think > >> local uniqueness of session IDs is more important than wrap around > >> because sessions are short lived entities and the chances of the same > >> session being alive by the time we've wrapped is pretty tiny. > > > > I've got a few complaints about target resources being tied up because > > we don't reuse session IDs. The ISID becomes a component in the > > I_T nexus identifier, so changing it invalidates persistent reservations. > > > >> If you can demostrate a multi-target problem, perhaps we should rather > >> fix this by making the next session id a target local quantity? > > > > Mike's got a good point that we don't really need to base the ISID off > > of our local session identifier (kobject name). I think getting reuse > > right may be a bit trickier than being a target local value, because it > > needs to be unique across target portal groups. Which probably furthers > > the argument that we should deal with that in the userspace tools. > > > > If we plan to split the protocol ISID cleanly from the kobject name, > > I guess the question is if aggressive reuse of the local identifier is > > better than dealing with the unlikely collision on rollover? > > I thought Lee's patch to convert the host_no from a atomic_t to ida > based was merged in Martin's tree. If that is going upstream, then I > thought you would want to fix the session id too. > > Is the concern similar to /dev/sdX reuse and bad apps? In this case, > some app might do a logout and login, but not update the sysfs mapping. > You could then hit corruption due to the sysfs session id now mapping to > a different target. OK, I don't want to cause a rehash of the same concerns. I took a look at whatever code I could think to check in that builds on top of the Open-iSCSI tools, and I don't think this will break anything. I'm OK with this. - Chris