On 03/09/2016 09:05 PM, Mike Christie wrote:
> On 03/08/2016 11:21 AM, Chris Leech wrote:
>> On Fri, Feb 12, 2016 at 09:38:53AM -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.
>>>
>>> Note that no maximum is placed on this value, though
>>> user-space currently only seems to use the lower 24-bits.
>>> It seems better to handle this in user space, though,
>>> than to limit the value range for the session ID here.
>>>
>>> Signed-off-by: Lee Duncan <ldun...@suse.com>
>>
>> Acked-by: Chris Leech <cle...@redhat.com>
>>
> 
> Dropping lkml and linux-scsi for a moment for some userspace stuff.
> 
> If we do this patch, then we need Chris's sysfs attr cache removal and
> in another patch I think we also need to drop the dev_list cache from
> open-iscsi/usr/sysfs.c?
> 
> For example, I think we could hit a bug because we could match a session
> id but the cached device's parent could be different than before.

This will cause problems if I want to use current userspace open-iscsi
with a newer kernel version once the kernel patch is merged, right?
Example use case: I install a stable version of any long term
supported distro (RHEL, SLES, Ubuntu, Debian), but use a newer kernel
because I want to install it on newer server hardware.

OTOH, I get why you want to make this change in the kernel and I agree
with the rationale for it.

What about this logic?

 - still keep an atomic int additionally
 - pass that atomic int as the start parameter to ida_simple_get
 - upon return, increment the atomic int (replace value with
   MAX(current atomic int value, new id) atomically)

finally:
 - if atomic int >= 0x7ffffff (the max for ida) just use 0 as start
   value and don't modify the atomic int anymore (then we have wrap
   around anyway, and then the current code doesn't handle that well
   anyway, so it doesn't deteriorate)

(I'm not a kernel hacker, and I'm assuming that ida_simple_get will
use low numbers by default that increase slowly - otherwise this
obviously won't work very well.)

This is more complicated, but it will preserve current semantics until
a wrap-around occurs and not break the current userspace code. Then,
in a couple of years or so, we can drop the additional atomic int
complication in the kernel and just use ida for ID allocation, once we
know that the removal of the cacvhe has been in userspace for long
enough.

Thoughts?

Regards,
Christian

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to