Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1241869 > unrolled thread

[PATCHv4 0/1] Update SCSI hosts to use ida for host number mgmt

Started byLee Duncan <lduncan@suse.com>
First post2015-10-08 02:00 +0200
Last post2015-10-16 22:20 +0200
Articles 10 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCHv4 0/1] Update SCSI hosts to use ida for host number mgmt Lee Duncan <lduncan@suse.com> - 2015-10-08 02:00 +0200
    [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management Lee Duncan <lduncan@suse.com> - 2015-10-08 02:00 +0200
      Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Johannes Thumshirn <jthumshirn@suse.de> - 2015-10-14 14:30 +0200
      Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-10-14 16:00 +0200
        Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Lee Duncan <lduncan@suse.com> - 2015-10-14 20:40 +0200
          Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management James Bottomley <James.Bottomley@HansenPartnership.com> - 2015-10-14 21:00 +0200
            Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Lee Duncan <lduncan@suse.com> - 2015-10-14 23:30 +0200
            Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Hannes Reinecke <hare@suse.com> - 2015-10-15 08:00 +0200
            Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Lee Duncan <lduncan@suse.com> - 2015-10-16 22:10 +0200
              Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no  management Greg KH <gregkh@linuxfoundation.org> - 2015-10-16 22:20 +0200

#1241869 — [PATCHv4 0/1] Update SCSI hosts to use ida for host number mgmt

FromLee Duncan <lduncan@suse.com>
Date2015-10-08 02:00 +0200
Subject[PATCHv4 0/1] Update SCSI hosts to use ida for host number mgmt
Message-ID<qh6MO-3hq-9@gated-at.bofh.it>
This patch updates the SCSI hosts module to use the ida
index-management routines to manage its host_no index instead
of using an ATOMIC integer. This means that host numbers
can now be reclaimed and re-used.

NOTE: it was not feasible to use idr_*() functions instead of
ida_*() functions, since using idr_find() without additional
locking to find our Scsi_Host structure left a window between
idr lookup and host structure deletion that would have
required additional locking to close.

Changes from v3:
 - Switched from idr to ida since managing our instance
   pointer required extra locking
Changes from v2 and v1:
 - First two version used idr instead of ida

Lee Duncan (1):
  SCSI: hosts: update to use ida_simple for host_no management

 drivers/scsi/hosts.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1241872 — [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromLee Duncan <lduncan@suse.com>
Date2015-10-08 02:00 +0200
Subject[PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qh6MO-3hq-7@gated-at.bofh.it>
In reply to#1241869
Update the SCSI hosts module to use the ida_simple*() routines
to manage its host_no index instead of an ATOMIC integer. This
means that the SCSI host number will now be reclaimable.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/scsi/hosts.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..b6a5ffa886b7 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
 #include <linux/transport_class.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-
+#include <linux/idr.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
@@ -42,7 +42,7 @@
 #include "scsi_logging.h"
 
 
-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);	/* host_no for next new host */
+static DEFINE_IDA(host_index_ida);
 
 
 static void scsi_host_cls_release(struct device *dev)
@@ -337,6 +337,8 @@ static void scsi_host_dev_release(struct device *dev)
 
 	kfree(shost->shost_data);
 
+	ida_simple_remove(&host_index_ida, shost->host_no);
+
 	if (parent)
 		put_device(parent);
 	kfree(shost);
@@ -370,6 +372,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 {
 	struct Scsi_Host *shost;
 	gfp_t gfp_mask = GFP_KERNEL;
+	int index;
 
 	if (sht->unchecked_isa_dma && privsize)
 		gfp_mask |= __GFP_DMA;
@@ -388,11 +391,11 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	init_waitqueue_head(&shost->host_wait);
 	mutex_init(&shost->scan_mutex);
 
-	/*
-	 * subtract one because we increment first then return, but we need to
-	 * know what the next host number was before increment
-	 */
-	shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
+	index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL);
+	if (index < 0)
+		goto fail_kfree;
+	shost->host_no = index;
+
 	shost->dma_channel = 0xff;
 
 	/* These three are default values which can be overridden */
@@ -477,7 +480,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 		shost_printk(KERN_WARNING, shost,
 			"error handler thread failed to spawn, error = %ld\n",
 			PTR_ERR(shost->ehandler));
-		goto fail_kfree;
+		goto fail_index_remove;
 	}
 
 	shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -493,6 +496,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 
  fail_kthread:
 	kthread_stop(shost->ehandler);
+ fail_index_remove:
+	ida_simple_remove(&host_index_ida, shost->host_no);
  fail_kfree:
 	kfree(shost);
 	return NULL;
@@ -588,6 +593,7 @@ int scsi_init_hosts(void)
 void scsi_exit_hosts(void)
 {
 	class_unregister(&shost_class);
+	ida_destroy(&host_index_ida);
 }
 
 int scsi_is_host_device(const struct device *dev)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1246618 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromJohannes Thumshirn <jthumshirn@suse.de>
Date2015-10-14 14:30 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjtlT-6PL-9@gated-at.bofh.it>
In reply to#1241872
On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
> Update the SCSI hosts module to use the ida_simple*() routines
> to manage its host_no index instead of an ATOMIC integer. This
> means that the SCSI host number will now be reclaimable.
> 
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/scsi/hosts.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 8bb173e01084..b6a5ffa886b7 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -33,7 +33,7 @@
>  #include <linux/transport_class.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> -
> +#include <linux/idr.h>
>  #include <scsi/scsi_device.h>
>  #include <scsi/scsi_host.h>
>  #include <scsi/scsi_transport.h>
> @@ -42,7 +42,7 @@
>  #include "scsi_logging.h"
>  
>  
> -static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);	/*
> host_no for next new host */
> +static DEFINE_IDA(host_index_ida);
>  
>  
>  static void scsi_host_cls_release(struct device *dev)
> @@ -337,6 +337,8 @@ static void scsi_host_dev_release(struct device
> *dev)
>  
>  	kfree(shost->shost_data);
>  
> +	ida_simple_remove(&host_index_ida, shost->host_no);
> +
>  	if (parent)
>  		put_device(parent);
>  	kfree(shost);
> @@ -370,6 +372,7 @@ struct Scsi_Host *scsi_host_alloc(struct
> scsi_host_template *sht, int privsize)
>  {
>  	struct Scsi_Host *shost;
>  	gfp_t gfp_mask = GFP_KERNEL;
> +	int index;
>  
>  	if (sht->unchecked_isa_dma && privsize)
>  		gfp_mask |= __GFP_DMA;
> @@ -388,11 +391,11 @@ struct Scsi_Host *scsi_host_alloc(struct
> scsi_host_template *sht, int privsize)
>  	init_waitqueue_head(&shost->host_wait);
>  	mutex_init(&shost->scan_mutex);
>  
> -	/*
> -	 * subtract one because we increment first then return, but
> we need to
> -	 * know what the next host number was before increment
> -	 */
> -	shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
> +	index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL);
> +	if (index < 0)
> +		goto fail_kfree;
> +	shost->host_no = index;
> +
>  	shost->dma_channel = 0xff;
>  
>  	/* These three are default values which can be overridden */
> @@ -477,7 +480,7 @@ struct Scsi_Host *scsi_host_alloc(struct
> scsi_host_template *sht, int privsize)
>  		shost_printk(KERN_WARNING, shost,
>  			"error handler thread failed to spawn, error
> = %ld\n",
>  			PTR_ERR(shost->ehandler));
> -		goto fail_kfree;
> +		goto fail_index_remove;
>  	}
>  
>  	shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
> @@ -493,6 +496,8 @@ struct Scsi_Host *scsi_host_alloc(struct
> scsi_host_template *sht, int privsize)
>  
>   fail_kthread:
>  	kthread_stop(shost->ehandler);
> + fail_index_remove:
> +	ida_simple_remove(&host_index_ida, shost->host_no);
>   fail_kfree:
>  	kfree(shost);
>  	return NULL;
> @@ -588,6 +593,7 @@ int scsi_init_hosts(void)
>  void scsi_exit_hosts(void)
>  {
>  	class_unregister(&shost_class);
> +	ida_destroy(&host_index_ida);
>  }
>  
>  int scsi_is_host_device(const struct device *dev)

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1246779 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2015-10-14 16:00 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjuL1-k6-33@gated-at.bofh.it>
In reply to#1241872
On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
> Update the SCSI hosts module to use the ida_simple*() routines
> to manage its host_no index instead of an ATOMIC integer. This
> means that the SCSI host number will now be reclaimable.

OK, but why would we want to do this?  We do it for sd because our minor
space for the device nodes is very constrained, so packing is essential.
For HBAs, there's no device space density to worry about, they're
largely statically allocated at boot time and not reusing the numbers
allows easy extraction of hotplug items for the logs (quite useful for
USB) because each separate hotplug has a separate and monotonically
increasing host number.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1247066 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromLee Duncan <lduncan@suse.com>
Date2015-10-14 20:40 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjz7Y-6RL-21@gated-at.bofh.it>
In reply to#1246779
On 10/14/2015 06:55 AM, James Bottomley wrote:
> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
>> Update the SCSI hosts module to use the ida_simple*() routines
>> to manage its host_no index instead of an ATOMIC integer. This
>> means that the SCSI host number will now be reclaimable.
> 
> OK, but why would we want to do this?  We do it for sd because our minor
> space for the device nodes is very constrained, so packing is essential.
> For HBAs, there's no device space density to worry about, they're
> largely statically allocated at boot time and not reusing the numbers
> allows easy extraction of hotplug items for the logs (quite useful for
> USB) because each separate hotplug has a separate and monotonically
> increasing host number.
> 
> James
> 

Good question, James. Apologies for not making the need clear.

The iSCSI subsystem uses a host structure for discovery, then throws it
away. So each time it does discovery it gets a new host structure. With
the current approach, that number is ever increasing. It's only a matter
of time until some user with a hundreds of disks and perhaps thousands
of LUNs, that likes to do periodic discovery (think super-computers)
will run out of host numbers. Or, worse yet, get a negative number
number (because the value is signed right now).

And this use case is a real one right now, by the way.

As you can see from the patch, it's a small amount of code to ensure
that the host number management is handled more cleanly.

-- 
Lee Duncan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1247082 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2015-10-14 21:00 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjzrk-7fk-27@gated-at.bofh.it>
In reply to#1247066
On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
> On 10/14/2015 06:55 AM, James Bottomley wrote:
> > On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
> >> Update the SCSI hosts module to use the ida_simple*() routines
> >> to manage its host_no index instead of an ATOMIC integer. This
> >> means that the SCSI host number will now be reclaimable.
> > 
> > OK, but why would we want to do this?  We do it for sd because our minor
> > space for the device nodes is very constrained, so packing is essential.
> > For HBAs, there's no device space density to worry about, they're
> > largely statically allocated at boot time and not reusing the numbers
> > allows easy extraction of hotplug items for the logs (quite useful for
> > USB) because each separate hotplug has a separate and monotonically
> > increasing host number.
> > 
> > James
> > 
> 
> Good question, James. Apologies for not making the need clear.
> 
> The iSCSI subsystem uses a host structure for discovery, then throws it
> away. So each time it does discovery it gets a new host structure. With
> the current approach, that number is ever increasing. It's only a matter
> of time until some user with a hundreds of disks and perhaps thousands
> of LUNs, that likes to do periodic discovery (think super-computers)
> will run out of host numbers. Or, worse yet, get a negative number
> number (because the value is signed right now).
> 
> And this use case is a real one right now, by the way.

Um, so even if you do discovery continuously, say one every second, it
still will take 68 years before we wrap the sign.

> As you can see from the patch, it's a small amount of code to ensure
> that the host number management is handled more cleanly.

Well, I'm a bit worried about the loss of a monotonically increasing
host number from the debugging perspective.  Right now, if you look at
any log, hostX always refers to one and only one incarnation throughout
the system lifetime for any given value of X.  With your patch, the
lowest host number gets continually reused ... probably for every hot
plug event.  If the USB and other hotplug system people don't mind this,
I suppose I can live with it, but I'd like to hear their view before
making this change.

James



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1247196 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromLee Duncan <lduncan@suse.com>
Date2015-10-14 23:30 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjBMu-2mZ-11@gated-at.bofh.it>
In reply to#1247082
On 10/14/2015 11:53 AM, James Bottomley wrote:
> On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
>> On 10/14/2015 06:55 AM, James Bottomley wrote:
>>> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
>>>> Update the SCSI hosts module to use the ida_simple*() routines
>>>> to manage its host_no index instead of an ATOMIC integer. This
>>>> means that the SCSI host number will now be reclaimable.
>>>
>>> OK, but why would we want to do this?  We do it for sd because our minor
>>> space for the device nodes is very constrained, so packing is essential.
>>> For HBAs, there's no device space density to worry about, they're
>>> largely statically allocated at boot time and not reusing the numbers
>>> allows easy extraction of hotplug items for the logs (quite useful for
>>> USB) because each separate hotplug has a separate and monotonically
>>> increasing host number.
>>>
>>> James
>>>
>>
>> Good question, James. Apologies for not making the need clear.
>>
>> The iSCSI subsystem uses a host structure for discovery, then throws it
>> away. So each time it does discovery it gets a new host structure. With
>> the current approach, that number is ever increasing. It's only a matter
>> of time until some user with a hundreds of disks and perhaps thousands
>> of LUNs, that likes to do periodic discovery (think super-computers)
>> will run out of host numbers. Or, worse yet, get a negative number
>> number (because the value is signed right now).
>>
>> And this use case is a real one right now, by the way.
> 
> Um, so even if you do discovery continuously, say one every second, it
> still will take 68 years before we wrap the sign.
> 
>> As you can see from the patch, it's a small amount of code to ensure
>> that the host number management is handled more cleanly.
> 
> Well, I'm a bit worried about the loss of a monotonically increasing
> host number from the debugging perspective.  Right now, if you look at
> any log, hostX always refers to one and only one incarnation throughout
> the system lifetime for any given value of X.  With your patch, the
> lowest host number gets continually reused ... probably for every hot
> plug event.  If the USB and other hotplug system people don't mind this,
> I suppose I can live with it, but I'd like to hear their view before
> making this change.
> 
> James
> 

James:

Understand your point, but I've never seen the host number not repeating
be a benefit in debugging or testing. And Hannes suggested this fix, so
I can only assume he also did not see a benefit of unique host
numbering. And purely aesthetically, seeing "host4595483528" in sysfs
would not be very user-friendly.

But one possible solution to address your concern would be to increase
the host number until it ran out of room (or hit some large maximum),
and only then start re-using host numbers. This would preserve the
current monotonically-increasing behavior at least initially. But I
worry that having this bi-modal numbering scheme might confuse some.
-- 
Lee Duncan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1247421 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromHannes Reinecke <hare@suse.com>
Date2015-10-15 08:00 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qjJK5-5GL-45@gated-at.bofh.it>
In reply to#1247082
On 10/14/2015 08:53 PM, James Bottomley wrote:
> On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
>> On 10/14/2015 06:55 AM, James Bottomley wrote:
>>> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
>>>> Update the SCSI hosts module to use the ida_simple*() routines
>>>> to manage its host_no index instead of an ATOMIC integer. This
>>>> means that the SCSI host number will now be reclaimable.
>>>
>>> OK, but why would we want to do this?  We do it for sd because our minor
>>> space for the device nodes is very constrained, so packing is essential.
>>> For HBAs, there's no device space density to worry about, they're
>>> largely statically allocated at boot time and not reusing the numbers
>>> allows easy extraction of hotplug items for the logs (quite useful for
>>> USB) because each separate hotplug has a separate and monotonically
>>> increasing host number.
>>>
>>> James
>>>
>>
>> Good question, James. Apologies for not making the need clear.
>>
>> The iSCSI subsystem uses a host structure for discovery, then throws it
>> away. So each time it does discovery it gets a new host structure. With
>> the current approach, that number is ever increasing. It's only a matter
>> of time until some user with a hundreds of disks and perhaps thousands
>> of LUNs, that likes to do periodic discovery (think super-computers)
>> will run out of host numbers. Or, worse yet, get a negative number
>> number (because the value is signed right now).
>>
>> And this use case is a real one right now, by the way.
> 
> Um, so even if you do discovery continuously, say one every second, it
> still will take 68 years before we wrap the sign.
> 
>> As you can see from the patch, it's a small amount of code to ensure
>> that the host number management is handled more cleanly.
> 
> Well, I'm a bit worried about the loss of a monotonically increasing
> host number from the debugging perspective.  Right now, if you look at
> any log, hostX always refers to one and only one incarnation throughout
> the system lifetime for any given value of X.  With your patch, the
> lowest host number gets continually reused ... probably for every hot
> plug event.  If the USB and other hotplug system people don't mind this,
> I suppose I can live with it, but I'd like to hear their view before
> making this change.
> 
Typically host numbers are not a real issue; whenever I need to
debug something more often than not I need to figure out
informations about the scsi device. And not once in my entire career
I had any needs to rely on the SCSI host number.

Plus the SCSI host number is the only thing in the stack which
_does_ increase; everything else like bus/target/LUN
numbers are stable and won't change much, irrespective of the
number of rescans or unloads. Which always irritated me.

So I'm definitely in favour of reusing the SCSI host numbers.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1249126 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromLee Duncan <lduncan@suse.com>
Date2015-10-16 22:10 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qkju9-8qx-7@gated-at.bofh.it>
In reply to#1247082
Adding linux-usb and linux-hotplug to cc list, in case they wish to comment.

Summary: I want to change SCSI host number so that it gets re-used, like
disk index numbers, instead of always increasing.

Please see below.

On 10/14/2015 11:53 AM, James Bottomley wrote:
> On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
>> On 10/14/2015 06:55 AM, James Bottomley wrote:
>>> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
>>>> Update the SCSI hosts module to use the ida_simple*() routines
>>>> to manage its host_no index instead of an ATOMIC integer. This
>>>> means that the SCSI host number will now be reclaimable.
>>>
>>> OK, but why would we want to do this?  We do it for sd because our minor
>>> space for the device nodes is very constrained, so packing is essential.
>>> For HBAs, there's no device space density to worry about, they're
>>> largely statically allocated at boot time and not reusing the numbers
>>> allows easy extraction of hotplug items for the logs (quite useful for
>>> USB) because each separate hotplug has a separate and monotonically
>>> increasing host number.
>>>
>>> James
>>>
>>
>> Good question, James. Apologies for not making the need clear.
>>
>> The iSCSI subsystem uses a host structure for discovery, then throws it
>> away. So each time it does discovery it gets a new host structure. With
>> the current approach, that number is ever increasing. It's only a matter
>> of time until some user with a hundreds of disks and perhaps thousands
>> of LUNs, that likes to do periodic discovery (think super-computers)
>> will run out of host numbers. Or, worse yet, get a negative number
>> number (because the value is signed right now).
>>
>> And this use case is a real one right now, by the way.
> 
> Um, so even if you do discovery continuously, say one every second, it
> still will take 68 years before we wrap the sign.
> 
>> As you can see from the patch, it's a small amount of code to ensure
>> that the host number management is handled more cleanly.
> 
> Well, I'm a bit worried about the loss of a monotonically increasing
> host number from the debugging perspective.  Right now, if you look at
> any log, hostX always refers to one and only one incarnation throughout
> the system lifetime for any given value of X.  With your patch, the
> lowest host number gets continually reused ... probably for every hot
> plug event.  If the USB and other hotplug system people don't mind this,
> I suppose I can live with it, but I'd like to hear their view before
> making this change.
> 
> James
> 
> 
> 
> 

-- 
Lee Duncan
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1249135 — Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

FromGreg KH <gregkh@linuxfoundation.org>
Date2015-10-16 22:20 +0200
SubjectRe: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management
Message-ID<qkjDQ-ax-9@gated-at.bofh.it>
In reply to#1249126
On Fri, Oct 16, 2015 at 01:03:42PM -0700, Lee Duncan wrote:
> Adding linux-usb and linux-hotplug to cc list, in case they wish to comment.
> 
> Summary: I want to change SCSI host number so that it gets re-used, like
> disk index numbers, instead of always increasing.
> 
> Please see below.
> 
> On 10/14/2015 11:53 AM, James Bottomley wrote:
> > On Wed, 2015-10-14 at 11:34 -0700, Lee Duncan wrote:
> >> On 10/14/2015 06:55 AM, James Bottomley wrote:
> >>> On Wed, 2015-10-07 at 16:51 -0700, Lee Duncan wrote:
> >>>> Update the SCSI hosts module to use the ida_simple*() routines
> >>>> to manage its host_no index instead of an ATOMIC integer. This
> >>>> means that the SCSI host number will now be reclaimable.
> >>>
> >>> OK, but why would we want to do this?  We do it for sd because our minor
> >>> space for the device nodes is very constrained, so packing is essential.
> >>> For HBAs, there's no device space density to worry about, they're
> >>> largely statically allocated at boot time and not reusing the numbers
> >>> allows easy extraction of hotplug items for the logs (quite useful for
> >>> USB) because each separate hotplug has a separate and monotonically
> >>> increasing host number.
> >>>
> >>> James
> >>>
> >>
> >> Good question, James. Apologies for not making the need clear.
> >>
> >> The iSCSI subsystem uses a host structure for discovery, then throws it
> >> away. So each time it does discovery it gets a new host structure. With
> >> the current approach, that number is ever increasing. It's only a matter
> >> of time until some user with a hundreds of disks and perhaps thousands
> >> of LUNs, that likes to do periodic discovery (think super-computers)
> >> will run out of host numbers. Or, worse yet, get a negative number
> >> number (because the value is signed right now).
> >>
> >> And this use case is a real one right now, by the way.
> > 
> > Um, so even if you do discovery continuously, say one every second, it
> > still will take 68 years before we wrap the sign.
> > 
> >> As you can see from the patch, it's a small amount of code to ensure
> >> that the host number management is handled more cleanly.
> > 
> > Well, I'm a bit worried about the loss of a monotonically increasing
> > host number from the debugging perspective.  Right now, if you look at
> > any log, hostX always refers to one and only one incarnation throughout
> > the system lifetime for any given value of X.  With your patch, the
> > lowest host number gets continually reused ... probably for every hot
> > plug event.  If the USB and other hotplug system people don't mind this,
> > I suppose I can live with it, but I'd like to hear their view before
> > making this change.

USB "people" don't care about this, why would we?  You can plug and
unplug and plug devices in lots of times and they get the "old" names
all the time, this is something that tools have had to deal with for
well over a decade.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web