Re: [PATCH 11/20] scsi_dh_alua: Use separate alua_port_group structure
On 07/24/2015 04:58 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 11:06:09AM +0200, Hannes Reinecke wrote:
>> +pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
>> +if (!pg) {
>> +sdev_printk(KERN_WARNING, sdev,
>> +"%s: kzalloc port group failed\n",
>> +ALUA_DH_NAME);
>> +/* Temporary failure, bypass */
>> +return SCSI_DH_DEV_TEMP_BUSY;
>> +}
>> +pg->group_id = group_id;
>> +pg->buff = pg->inq;
>> +pg->bufflen = ALUA_INQUIRY_SIZE;
>> +pg->tpgs = h->tpgs;
>> +pg->state = TPGS_STATE_OPTIMIZED;
>> +kref_init(&pg->kref);
>> +spin_lock(&port_group_lock);
>> +list_add(&pg->node, &port_group_list);
>> +h->pg = pg;
>> +spin_unlock(&port_group_lock);
>
> Is there any high level protection against someone racing to allocate
> this structure, e.g. from a sysfs-initiated scan?
>
Not in this patch, as this is called during device scan only. It is
assumed that higher levels will protect against simultaneous scans.
Real protection against concurrent updates is done in patch
'scsi_dh_alua: parse device id', as with that we can easily hit existing
port groups.
>> -len = (h->buff[0] << 24) + (h->buff[1] << 16) +
>> -(h->buff[2] << 8) + h->buff[3] + 4;
>> +len = get_unaligned_be32(&pg->buff[0]) + 4;
>
> Andother spurious get/set_unaligned conversion. I'd really recommend doing
> all of them before the atual series.
>
Okay, will be doing so.
>> +rcu_read_lock();
>> +pg = rcu_dereference(h->pg);
>> +if (!pg) {
>> +rcu_read_unlock();
>> +return -ENXIO;
>> +}
>> +rcu_read_unlock();
>> +
>> if (optimize)
>> -h->flags |= ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ALUA_OPTIMIZE_STPG;
>> else
>> -h->flags &= ~ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ~ALUA_OPTIMIZE_STPG;
>
> You'll need to move the rcu_read_unlock here to be safe.
>
Ok.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/20] scsi_dh_alua: Use separate alua_port_group structure
On Wed, Jul 08, 2015 at 11:06:09AM +0200, Hannes Reinecke wrote:
> + pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
> + if (!pg) {
> + sdev_printk(KERN_WARNING, sdev,
> + "%s: kzalloc port group failed\n",
> + ALUA_DH_NAME);
> + /* Temporary failure, bypass */
> + return SCSI_DH_DEV_TEMP_BUSY;
> + }
> + pg->group_id = group_id;
> + pg->buff = pg->inq;
> + pg->bufflen = ALUA_INQUIRY_SIZE;
> + pg->tpgs = h->tpgs;
> + pg->state = TPGS_STATE_OPTIMIZED;
> + kref_init(&pg->kref);
> + spin_lock(&port_group_lock);
> + list_add(&pg->node, &port_group_list);
> + h->pg = pg;
> + spin_unlock(&port_group_lock);
Is there any high level protection against someone racing to allocate
this structure, e.g. from a sysfs-initiated scan?
> - len = (h->buff[0] << 24) + (h->buff[1] << 16) +
> - (h->buff[2] << 8) + h->buff[3] + 4;
> + len = get_unaligned_be32(&pg->buff[0]) + 4;
Andother spurious get/set_unaligned conversion. I'd really recommend doing
all of them before the atual series.
> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (!pg) {
> + rcu_read_unlock();
> + return -ENXIO;
> + }
> + rcu_read_unlock();
> +
> if (optimize)
> - h->flags |= ALUA_OPTIMIZE_STPG;
> + pg->flags |= ALUA_OPTIMIZE_STPG;
> else
> - h->flags &= ~ALUA_OPTIMIZE_STPG;
> + pg->flags |= ~ALUA_OPTIMIZE_STPG;
You'll need to move the rcu_read_unlock here to be safe.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/20] scsi_dh_alua: Use separate alua_port_group structure
The port group needs to be a separate structure as several
LUNs might belong to the same group.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/device_handler/scsi_dh_alua.c | 210 +++--
1 file changed, 137 insertions(+), 73 deletions(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
b/drivers/scsi/device_handler/scsi_dh_alua.c
index ea4a920..3f18f6b 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -64,9 +64,13 @@
#define ALUA_OPTIMIZE_STPG 1
#define ALUA_RTPG_EXT_HDR_UNSUPP 2
-struct alua_dh_data {
+static LIST_HEAD(port_group_list);
+static DEFINE_SPINLOCK(port_group_lock);
+
+struct alua_port_group {
+ struct kref kref;
+ struct list_headnode;
int group_id;
- int rel_port;
int tpgs;
int state;
int pref;
@@ -75,6 +79,12 @@ struct alua_dh_data {
unsigned char *buff;
int bufflen;
unsigned char transition_tmo;
+};
+
+struct alua_dh_data {
+ struct alua_port_group *pg;
+ int rel_port;
+ int tpgs;
struct scsi_device *sdev;
activate_complete callback_fn;
void*callback_data;
@@ -86,21 +96,35 @@ struct alua_dh_data {
static char print_alua_state(int);
static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *);
-static int realloc_buffer(struct alua_dh_data *h, unsigned len)
+static int realloc_buffer(struct alua_port_group *pg, unsigned len)
{
- if (h->buff && h->buff != h->inq)
- kfree(h->buff);
+ if (pg->buff && pg->buff != pg->inq)
+ kfree(pg->buff);
- h->buff = kmalloc(len, GFP_NOIO);
- if (!h->buff) {
- h->buff = h->inq;
- h->bufflen = ALUA_INQUIRY_SIZE;
+ pg->buff = kmalloc(len, GFP_NOIO);
+ if (!pg->buff) {
+ pg->buff = pg->inq;
+ pg->bufflen = ALUA_INQUIRY_SIZE;
return 1;
}
- h->bufflen = len;
+ pg->bufflen = len;
return 0;
}
+static void release_port_group(struct kref *kref)
+{
+ struct alua_port_group *pg;
+
+ pg = container_of(kref, struct alua_port_group, kref);
+ printk(KERN_WARNING "alua: release port group %d\n", pg->group_id);
+ spin_lock(&port_group_lock);
+ list_del(&pg->node);
+ spin_unlock(&port_group_lock);
+ if (pg->buff && pg->inq != pg->buff)
+ kfree(pg->buff);
+ kfree(pg);
+}
+
/*
* submit_rtpg - Issue a REPORT TARGET GROUP STATES command
* @sdev: sdev the command should be sent to
@@ -223,6 +247,8 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct
alua_dh_data *h)
static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
{
unsigned char *d;
+ int group_id = -1;
+ struct alua_port_group *pg = NULL;
if (!sdev->vpd_pg83)
return SCSI_DH_DEV_UNSUPP;
@@ -239,7 +265,7 @@ static int alua_check_vpd(struct scsi_device *sdev, struct
alua_dh_data *h)
break;
case 0x5:
/* Target port group */
- h->group_id = (d[6] << 8) + d[7];
+ group_id = (d[6] << 8) + d[7];
break;
default:
break;
@@ -247,7 +273,7 @@ static int alua_check_vpd(struct scsi_device *sdev, struct
alua_dh_data *h)
d += d[3] + 4;
}
- if (h->group_id == -1) {
+ if (group_id == -1) {
/*
* Internal error; TPGS supported but required
* VPD identification descriptors not present.
@@ -256,15 +282,33 @@ static int alua_check_vpd(struct scsi_device *sdev,
struct alua_dh_data *h)
sdev_printk(KERN_INFO, sdev,
"%s: No target port descriptors found\n",
ALUA_DH_NAME);
- h->state = TPGS_STATE_OPTIMIZED;
h->tpgs = TPGS_MODE_NONE;
return SCSI_DH_DEV_UNSUPP;
}
sdev_printk(KERN_INFO, sdev,
"%s: port group %02x rel port %02x\n",
- ALUA_DH_NAME, h->group_id, h->rel_port);
+ ALUA_DH_NAME, group_id, h->rel_port);
- return 0;
+ pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
+ if (!pg) {
+ sdev_printk(KERN_WARNING, sdev,
+ "%s: kzalloc port group failed\n",
+ ALUA_DH_NAME);
+ /* Temporary failure, bypass */
+ return SCSI_DH_DEV_TEMP_BUSY;
+ }
+ pg->group_id = group_id;
+ pg->buff = pg->inq;
