Re: [PATCH] libfc: unsafe refcounting in fc_rport_work()
- Original Message -
From: James Bottomley
To: Hannes Reinecke , Martin K. Petersen
Cc: Christoph Hellwig , Ewan Milne ,
linux-scsi@vger.kernel.org
Sent: Wed, 20 Apr 2016 15:03:12 -0400 (EDT)
Subject: Re: [PATCH] libfc: unsafe refcounting in fc_rport_work()
On Wed, 2016-04-20 at 15:24 +0200, Hannes Reinecke wrote:
> When pushing items on a workqueue we cannot take reference
> when the workqueue item is executed, as the structure might
> already been freed at that time.
> So instead we need to take a reference before adding it
> to the workqueue, thereby ensuring that the workqueue item
> will always be valid.
>Have you actually seen this happen? The rdata structure is fully ref
>counted, so if it's done a final put, then something should see
>unreferenced memory. It looks like the model is that the final put is
>done from the queue, so I don't quite see how you can lose the final
>reference in either of the places you alter.
I have two dumps with freed rport structures that have delayed_work
items that are still on the active timer list. Both are with FCoE, one
using bnx and the other using ixgbe. Something is wrong. I'm not
sure why we don't see anything on regular FC.
It seems to me that we should be holding a kref on the rport while
a work item is pending, but I'm not sure releasing it at the end of the
work function is OK, either. The workqueue code might still reference
the embedded work or delayed_work on the way out, no?
I agree with Christoph, this whole thing needs looking at. All the
delayed_work stuff seems problematic to me. James S fixed a 3-way
deadlock involving this code a while back, for instance.
-Ewan
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/libfc/fc_rport.c | 25 +++--
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_rport.c
> b/drivers/scsi/libfc/fc_rport.c
> index 589ff9a..8b08263f 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -263,7 +263,6 @@ static void fc_rport_work(struct work_struct
> *work)
> ids = rdata->ids;
> rdata->event = RPORT_EV_NONE;
> rdata->major_retries = 0;
> - kref_get(&rdata->kref);
> mutex_unlock(&rdata->rp_mutex);
>
> if (!rport)
> @@ -297,7 +296,6 @@ static void fc_rport_work(struct work_struct
> *work)
> FC_RPORT_DBG(rdata, "lld callback ev %d\n",
> event);
> rdata->lld_event_callback(lport, rdata,
> event);
> }
> - kref_put(&rdata->kref, lport->tt.rport_destroy);
> break;
>
> case RPORT_EV_FAILED:
> @@ -377,6 +375,7 @@ static void fc_rport_work(struct work_struct
> *work)
> mutex_unlock(&rdata->rp_mutex);
> break;
> }
> + kref_put(&rdata->kref, lport->tt.rport_destroy);
> }
>
> /**
> @@ -438,8 +437,15 @@ static void fc_rport_enter_delete(struct
> fc_rport_priv *rdata,
>
> fc_rport_state_enter(rdata, RPORT_ST_DELETE);
>
> - if (rdata->event == RPORT_EV_NONE)
> - queue_work(rport_event_queue, &rdata->event_work);
> + if (rdata->event == RPORT_EV_NONE) {
> + if (!kref_get_unless_zero(&rdata->kref)) {
> + FC_RPORT_DBG(rdata, "port already
> deleted\n");
> + return;
> + }
> + if (!queue_work(rport_event_queue, &rdata
> ->event_work))
> + kref_put(&rdata->kref,
> + rdata->local_port
> ->tt.rport_destroy);
> + }
> rdata->event = event;
> }
>
> @@ -487,8 +493,15 @@ static void fc_rport_enter_ready(struct
> fc_rport_priv *rdata)
>
> FC_RPORT_DBG(rdata, "Port is Ready\n");
>
> - if (rdata->event == RPORT_EV_NONE)
> - queue_work(rport_event_queue, &rdata->event_work);
> + if (rdata->event == RPORT_EV_NONE) {
> + if (!kref_get_unless_zero(&rdata->kref)) {
> + FC_RPORT_DBG(rdata, "port already
> deleted\n");
> + return;
> + }
> + if (!queue_work(rport_event_queue, &rdata
> ->event_work))
> + kref_put(&rdata->kref,
> + rdata->local_port
> ->tt.rport_destroy);
> + }
> rdata->event = RPORT_EV_READY;
> }
>
--
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] libfc: unsafe refcounting in fc_rport_work()
On Wed, Apr 20, 2016 at 03:03:12PM -0400, James Bottomley wrote: > Plus, kref_get_unless_zero() should not be used. At that point, the > structure would be freed, so there's no point looking for it. Agreed for this particular case. Note that the whole code seems rather whckay as it uses a rport_destroy method that just has one instance, so I'd really like to see things cleaned up to remove this method and add a warapper for the kref_put before doing further changes. > kref_get_unless_zero is for refcounts that don't necessarily free the > structure (embedded ones). The main case actually is for embedded krefs. The important bit is that the structure is on some lookup data structure (typically list), and kref_get_unless_zero protects against the window from between dropping the last references and doing the list removal in the destructor. -- 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] libfc: unsafe refcounting in fc_rport_work()
On Wed, 2016-04-20 at 15:24 +0200, Hannes Reinecke wrote:
> When pushing items on a workqueue we cannot take reference
> when the workqueue item is executed, as the structure might
> already been freed at that time.
> So instead we need to take a reference before adding it
> to the workqueue, thereby ensuring that the workqueue item
> will always be valid.
Have you actually seen this happen? The rdata structure is fully ref
counted, so if it's done a final put, then something should see
unreferenced memory. It looks like the model is that the final put is
done from the queue, so I don't quite see how you can lose the final
reference in either of the places you alter.
Plus, kref_get_unless_zero() should not be used. At that point, the
structure would be freed, so there's no point looking for it.
kref_get_unless_zero is for refcounts that don't necessarily free the
structure (embedded ones).
James
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/libfc/fc_rport.c | 25 +++--
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_rport.c
> b/drivers/scsi/libfc/fc_rport.c
> index 589ff9a..8b08263f 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -263,7 +263,6 @@ static void fc_rport_work(struct work_struct
> *work)
> ids = rdata->ids;
> rdata->event = RPORT_EV_NONE;
> rdata->major_retries = 0;
> - kref_get(&rdata->kref);
> mutex_unlock(&rdata->rp_mutex);
>
> if (!rport)
> @@ -297,7 +296,6 @@ static void fc_rport_work(struct work_struct
> *work)
> FC_RPORT_DBG(rdata, "lld callback ev %d\n",
> event);
> rdata->lld_event_callback(lport, rdata,
> event);
> }
> - kref_put(&rdata->kref, lport->tt.rport_destroy);
> break;
>
> case RPORT_EV_FAILED:
> @@ -377,6 +375,7 @@ static void fc_rport_work(struct work_struct
> *work)
> mutex_unlock(&rdata->rp_mutex);
> break;
> }
> + kref_put(&rdata->kref, lport->tt.rport_destroy);
> }
>
> /**
> @@ -438,8 +437,15 @@ static void fc_rport_enter_delete(struct
> fc_rport_priv *rdata,
>
> fc_rport_state_enter(rdata, RPORT_ST_DELETE);
>
> - if (rdata->event == RPORT_EV_NONE)
> - queue_work(rport_event_queue, &rdata->event_work);
> + if (rdata->event == RPORT_EV_NONE) {
> + if (!kref_get_unless_zero(&rdata->kref)) {
> + FC_RPORT_DBG(rdata, "port already
> deleted\n");
> + return;
> + }
> + if (!queue_work(rport_event_queue, &rdata
> ->event_work))
> + kref_put(&rdata->kref,
> + rdata->local_port
> ->tt.rport_destroy);
> + }
> rdata->event = event;
> }
>
> @@ -487,8 +493,15 @@ static void fc_rport_enter_ready(struct
> fc_rport_priv *rdata)
>
> FC_RPORT_DBG(rdata, "Port is Ready\n");
>
> - if (rdata->event == RPORT_EV_NONE)
> - queue_work(rport_event_queue, &rdata->event_work);
> + if (rdata->event == RPORT_EV_NONE) {
> + if (!kref_get_unless_zero(&rdata->kref)) {
> + FC_RPORT_DBG(rdata, "port already
> deleted\n");
> + return;
> + }
> + if (!queue_work(rport_event_queue, &rdata
> ->event_work))
> + kref_put(&rdata->kref,
> + rdata->local_port
> ->tt.rport_destroy);
> + }
> rdata->event = RPORT_EV_READY;
> }
>
--
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] SCSI: LIBSCSI: Fixed codeing style errors
On 16-04-20 03:51 AM, Bob Stlt wrote:
Fixed codeing style formatting errors.
Signed-off-by: Bob Stlt
---
drivers/scsi/libiscsi.c | 90 -
1 file changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 6bffd91..41be9d3 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -197,7 +197,7 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_task *task)
pad_len = iscsi_padding(rlen);
rc = iscsi_add_hdr(task, sizeof(ecdb_ahdr->ahslength) +
- sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+ sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
Seriously? checkpatch.pl in the current linux-stable.git is
allowing me to add a minimum number of spaces immediately
before a name so it is aligned to beginning of the first
argument of a function. IMO that is more readable that what is
proposed above.
if (rc)
return rc;
@@ -210,10 +210,10 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_task *task)
memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
ISCSI_DBG_SESSION(task->conn->session,
- "iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
- "rlen %d pad_len %d ahs_length %d iscsi_headers_size "
- "%u\n", cmd->cmd_len, rlen, pad_len, ahslength,
- task->hdr_len);
+ "iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+ "rlen %d pad_len %d ahs_length %d iscsi_headers_size "
+ "%u\n", cmd->cmd_len, rlen, pad_len, ahslength,
+ task->hdr_len);
return 0;
When I try something like the above, checkpatch.pl complains
about string concatenation across lines. When I put the whole
string on one line, it complains that the line is longer than 80
characters. That leads me to some string obfuscation which is
much less readable but gets the okay from checkpatch.pl :-)
Doug Gilbert
}
@@ -236,10 +236,10 @@ static int iscsi_prep_bidi_ahs(struct iscsi_task *task)
rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
ISCSI_DBG_SESSION(task->conn->session,
- "bidi-in rlen_ahdr->read_length(%d) "
- "rlen_ahdr->ahslength(%d)\n",
- be32_to_cpu(rlen_ahdr->read_length),
- be16_to_cpu(rlen_ahdr->ahslength));
+ "bidi-in rlen_ahdr->read_length(%d) "
+ "rlen_ahdr->ahslength(%d)\n",
+ be32_to_cpu(rlen_ahdr->read_length),
+ be16_to_cpu(rlen_ahdr->ahslength));
return 0;
}
@@ -500,7 +500,7 @@ static void iscsi_free_task(struct iscsi_task *task)
if (conn->login_task == task)
return;
- kfifo_in(&session->cmdpool.queue, (void*)&task, sizeof(void*));
+ kfifo_in(&session->cmdpool.queue, (void *)&task, sizeof(void *));
if (sc) {
/* SCSI eh reuses commands to verify us */
@@ -736,7 +736,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct
iscsi_hdr *hdr,
BUG_ON(conn->c_stage == ISCSI_CONN_STOPPED);
if (!kfifo_out(&session->cmdpool.queue,
-(void*)&task, sizeof(void*)))
+(void *)&task, sizeof(void *)))
return NULL;
}
/*
@@ -831,7 +831,7 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn,
struct iscsi_hdr *hdr,
struct iscsi_session *session = conn->session;
struct scsi_cmnd *sc = task->sc;
- iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
+ iscsi_update_cmdsn(session, (struct iscsi_nopin *)rhdr);
conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
sc->result = (DID_OK << 16) | rhdr->cmd_status;
@@ -901,12 +901,12 @@ invalid_datalen:
}
if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
- ISCSI_FLAG_CMD_OVERFLOW)) {
+ ISCSI_FLAG_CMD_OVERFLOW)) {
int res_count = be32_to_cpu(rhdr->residual_count);
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-res_count <= scsi_bufflen(sc)))
+ res_count <= scsi_bufflen(sc)))
/* write side for bidi or uni-io set_resid */
scsi_set_resid(sc, res_count);
else
@@ -939,7 +939,7 @@ iscsi_data_in_rsp(struct iscsi_conn *conn, struct iscsi_hdr
*hdr,
sc->result = (DID_OK << 16) | rhdr->cmd_status;
conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
if (rhdr->flags & (ISCSI_FLAG_DATA_UNDERFLOW |
- ISCSI_FLAG_DATA_OVERFLOW)) {
+ ISCSI_FLAG_DATA_OVERFLOW)) {
Re: alloc failure in qla1280 probe -- need to decrease can_queue?
[+Cc Michael Reed as get_maintainer.pl lists him as qla1280 maintainer ]
On Mon, Apr 18, 2016 at 03:07:15PM -0700, Laura Abbott wrote:
> Hi,
>
> We received a bug report https://bugzilla.redhat.com/show_bug.cgi?id=1321033
> of qla1280 scsi host failure on 4.4 based kernels that looks to be caused
> by page alloc failure:
>
> [4.804166] scsi host0: QLogic QLA1040 PCI to SCSI Host Adapter
> Firmware version: 7.65.06, Driver version 3.27.1
> [4.804174] [ cut here ]
> [4.804184] WARNING: CPU: 2 PID: 305 at mm/page_alloc.c:2989
> __alloc_pages_nodemask+0xae8/0xbc0()
> [4.804186] Modules linked in: amdkfd amd_iommu_v2 radeon i2c_algo_bit
> drm_kms_helper ttm drm megaraid_sas serio_raw 8021q garp bnx2 stp llc mrp
> sunhme qla1280(+) fjes
> [4.804208] CPU: 2 PID: 305 Comm: systemd-udevd Not tainted
> 4.4.6-201.fc22.x86_64 #1
> [4.804210] Hardware name: Google Enterprise Search Appliance/0DT021, BIOS
> 1.1.2 08/14/2006
> [4.804212] 0286 2f01064c 88042985b710
> 813b542e
> [4.804216] 81a75024 88042985b748
> 810a40f2
> [4.804220] 000b
>
> [4.804223] Call Trace:
> [4.804231] [] dump_stack+0x63/0x85
> [4.804236] [] warn_slowpath_common+0x82/0xc0
> [4.804239] [] warn_slowpath_null+0x1a/0x20
> [4.804242] [] __alloc_pages_nodemask+0xae8/0xbc0
> [4.804247] [] ? _raw_spin_unlock_irqrestore+0xe/0x10
> [4.804251] [] ? irq_work_queue+0x8e/0xa0
> [4.804256] [] ? console_unlock+0x20a/0x540
> [4.804262] [] alloc_pages_current+0x8c/0x110
> [4.804265] [] alloc_kmem_pages+0x19/0x90
> [4.804268] [] kmalloc_order_trace+0x2e/0xe0
> [4.804272] [] __kmalloc+0x232/0x260
> [4.804277] [] init_tag_map+0x3d/0xc0
> [4.804290] [] __blk_queue_init_tags+0x45/0x80
> [4.804293] [] blk_init_tags+0x14/0x20
> [4.804298] [] scsi_add_host_with_dma+0x80/0x300
> [4.804305] [] qla1280_probe_one+0x683/0x9ef [qla1280]
> [4.804309] [] local_pci_probe+0x45/0xa0
> [4.804312] [] pci_device_probe+0xfd/0x140
> [4.804316] [] driver_probe_device+0x222/0x490
> [4.804319] [] __driver_attach+0x84/0x90
> [4.804321] [] ? driver_probe_device+0x490/0x490
> [4.804324] [] bus_for_each_dev+0x6c/0xc0
> [4.804326] [] driver_attach+0x1e/0x20
> [4.804328] [] bus_add_driver+0x1eb/0x280
> [4.804331] [] ? 0xa0015000
> [4.804333] [] driver_register+0x60/0xe0
> [4.804336] [] __pci_register_driver+0x4c/0x50
> [4.804339] [] qla1280_init+0x1ce/0x1000 [qla1280]
> [4.804341] [] ? 0xa0015000
> [4.804345] [] do_one_initcall+0xb3/0x200
> [4.804348] [] ? kmem_cache_alloc_trace+0x196/0x210
> [4.804352] [] ? do_init_module+0x27/0x1cb
> [4.804354] [] do_init_module+0x5f/0x1cb
> [4.804358] [] load_module+0x2040/0x2680
> [4.804360] [] ? __symbol_put+0x60/0x60
> [4.804363] [] SYSC_init_module+0x149/0x190
> [4.804366] [] SyS_init_module+0xe/0x10
> [4.804369] [] entry_SYSCALL_64_fastpath+0x12/0x71
> [4.804371] ---[ end trace 0ea3b625f86705f7 ]---
> [4.804581] qla1280: probe of :11:04.0 failed with error -12
>
> This looks very similar to
> http://www.spinics.net/lists/linux-usb/msg136998.html
> which was fixed by 55ff8cfbc4e1 ("USB: uas: Reduce can_queue to MAX_CMNDS").
> Does a similar fix need to be applied here?
>
> Thanks,
> Laura
Can you (or better the reporter) try below? Unfortunately I don't have a
qla1280 setup here, so I couldn't test it myself.
Byte,
Johannes
>From f95e82e7e5f675c9869ea1da78021aa6abc7972b Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn
Date: Wed, 20 Apr 2016 16:07:37 +0200
Subject: [PATCH] qla1280: Reduce can_queue to 32
The qla1280 driver sets the scsi_host_template's can_queue field to 0xf
which results in an allocation failure when allocating the block layer tags
for the driver's queues like the one shown below:
[4.804166] scsi host0: QLogic QLA1040 PCI to SCSI Host Adapter Firmware
version: 7.65.06, Driver version 3.27.1
[4.804174] [ cut here ]
[4.804184] WARNING: CPU: 2 PID: 305 at mm/page_alloc.c:2989
alloc_pages_nodemask+0xae8/0xbc0()
[4.804186] Modules linked in: amdkfd amd_iommu_v2 radeon i2c_algo_bit
m_kms_helper ttm drm megaraid_sas serio_raw 8021q garp bnx2 stp llc mrp nhme
qla1280(+) fjes
[4.804208] CPU: 2 PID: 305 Comm: systemd-udevd Not tainted
4.6-201.fc22.x86_64 #1
[4.804210] Hardware name: Google Enterprise Search Appliance/0DT021, OS
1.1.2 08/14/2006
[4.804212] 0286 2f01064c 88042985b710
ff813b542e
[4.804216] 81a75024 88042985b748
ff810a40f2
[4.804220] 000b
00
[4.804223] Call Trace:
[4.804231]
Re: [PATCH] libfc: unsafe refcounting in fc_rport_work()
On Wed, Apr 20, 2016 at 03:24:21PM +0200, Hannes Reinecke wrote: > When pushing items on a workqueue we cannot take reference > when the workqueue item is executed, as the structure might > already been freed at that time. > So instead we need to take a reference before adding it > to the workqueue, thereby ensuring that the workqueue item > will always be valid. > > Signed-off-by: Hannes Reinecke Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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] libfc: unsafe refcounting in fc_rport_work()
When pushing items on a workqueue we cannot take reference
when the workqueue item is executed, as the structure might
already been freed at that time.
So instead we need to take a reference before adding it
to the workqueue, thereby ensuring that the workqueue item
will always be valid.
Signed-off-by: Hannes Reinecke
---
drivers/scsi/libfc/fc_rport.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index 589ff9a..8b08263f 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -263,7 +263,6 @@ static void fc_rport_work(struct work_struct *work)
ids = rdata->ids;
rdata->event = RPORT_EV_NONE;
rdata->major_retries = 0;
- kref_get(&rdata->kref);
mutex_unlock(&rdata->rp_mutex);
if (!rport)
@@ -297,7 +296,6 @@ static void fc_rport_work(struct work_struct *work)
FC_RPORT_DBG(rdata, "lld callback ev %d\n", event);
rdata->lld_event_callback(lport, rdata, event);
}
- kref_put(&rdata->kref, lport->tt.rport_destroy);
break;
case RPORT_EV_FAILED:
@@ -377,6 +375,7 @@ static void fc_rport_work(struct work_struct *work)
mutex_unlock(&rdata->rp_mutex);
break;
}
+ kref_put(&rdata->kref, lport->tt.rport_destroy);
}
/**
@@ -438,8 +437,15 @@ static void fc_rport_enter_delete(struct fc_rport_priv
*rdata,
fc_rport_state_enter(rdata, RPORT_ST_DELETE);
- if (rdata->event == RPORT_EV_NONE)
- queue_work(rport_event_queue, &rdata->event_work);
+ if (rdata->event == RPORT_EV_NONE) {
+ if (!kref_get_unless_zero(&rdata->kref)) {
+ FC_RPORT_DBG(rdata, "port already deleted\n");
+ return;
+ }
+ if (!queue_work(rport_event_queue, &rdata->event_work))
+ kref_put(&rdata->kref,
+rdata->local_port->tt.rport_destroy);
+ }
rdata->event = event;
}
@@ -487,8 +493,15 @@ static void fc_rport_enter_ready(struct fc_rport_priv
*rdata)
FC_RPORT_DBG(rdata, "Port is Ready\n");
- if (rdata->event == RPORT_EV_NONE)
- queue_work(rport_event_queue, &rdata->event_work);
+ if (rdata->event == RPORT_EV_NONE) {
+ if (!kref_get_unless_zero(&rdata->kref)) {
+ FC_RPORT_DBG(rdata, "port already deleted\n");
+ return;
+ }
+ if (!queue_work(rport_event_queue, &rdata->event_work))
+ kref_put(&rdata->kref,
+rdata->local_port->tt.rport_destroy);
+ }
rdata->event = RPORT_EV_READY;
}
--
1.8.5.6
--
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 1/1] scsi: add Synology DSM 6.0 and higher to 1024 sector blacklist
On 2016-04-12 21:22, Martin K. Petersen wrote: >> "Mike" == Mike Christie writes: > >>> Mike: Any preference? > > Mike> I think quirking SYNOLOGY is best. It will be the safest route and > Mike> work for all devices. > > Michel: Please spin a new version of the patch. Apologies for not getting back earlier. SYNOLOGY have released a new version of their OS indicating that they have worked to fix the iSCSI problem (and another one to actually make that update installable): https://www.synology.com/en-global/releaseNote/RS10613xs+ - "Fixed multiple issues that might cause iSCSI service to hang under heavy loading and performing Vmware VAAI commands." - "Fixed an issue where DSM 6.0-7321 Update 1 could not be normally installed with Auto-Update." (They have not updated my support ticket on this case thus far.) Both my SYNOLOGY appliances now seem to show correct vpd data when queried: root@server:/# sg_vpd -p bl /dev/sdb Block limits VPD page (SBC): Write same non-zero (WSNZ): 1 Maximum compare and write length: 1 blocks Optimal transfer length granularity: 128 blocks Maximum transfer length: 8192 blocks Optimal transfer length: 1152 blocks Maximum prefetch length: 0 blocks root@server:/# sg_vpd -p bl /dev/sdd Block limits VPD page (SBC): Write same non-zero (WSNZ): 1 Maximum compare and write length: 1 blocks Optimal transfer length granularity: 128 blocks Maximum transfer length: 8192 blocks Optimal transfer length: 512 blocks Maximum prefetch length: 0 blocks I am currently running tests with an unmodified Debian 4.4.0-1-amd64 kernel, so unless I can reproduce the problem, I suggest we leave the current blacklist as is (thereby only quirking the pre-6.0 SYNOLOGY volumes that are affected) and discard the new patch. -- Michel -- 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] SCSI: LIBSCSI: Fixed codeing style errors
Fixed codeing style formatting errors.
Signed-off-by: Bob Stlt
---
drivers/scsi/libiscsi.c | 90 -
1 file changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 6bffd91..41be9d3 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -197,7 +197,7 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_task *task)
pad_len = iscsi_padding(rlen);
rc = iscsi_add_hdr(task, sizeof(ecdb_ahdr->ahslength) +
- sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+ sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
if (rc)
return rc;
@@ -210,10 +210,10 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_task *task)
memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
ISCSI_DBG_SESSION(task->conn->session,
- "iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
- "rlen %d pad_len %d ahs_length %d iscsi_headers_size "
- "%u\n", cmd->cmd_len, rlen, pad_len, ahslength,
- task->hdr_len);
+ "iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+ "rlen %d pad_len %d ahs_length %d iscsi_headers_size "
+ "%u\n", cmd->cmd_len, rlen, pad_len, ahslength,
+ task->hdr_len);
return 0;
}
@@ -236,10 +236,10 @@ static int iscsi_prep_bidi_ahs(struct iscsi_task *task)
rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
ISCSI_DBG_SESSION(task->conn->session,
- "bidi-in rlen_ahdr->read_length(%d) "
- "rlen_ahdr->ahslength(%d)\n",
- be32_to_cpu(rlen_ahdr->read_length),
- be16_to_cpu(rlen_ahdr->ahslength));
+ "bidi-in rlen_ahdr->read_length(%d) "
+ "rlen_ahdr->ahslength(%d)\n",
+ be32_to_cpu(rlen_ahdr->read_length),
+ be16_to_cpu(rlen_ahdr->ahslength));
return 0;
}
@@ -500,7 +500,7 @@ static void iscsi_free_task(struct iscsi_task *task)
if (conn->login_task == task)
return;
- kfifo_in(&session->cmdpool.queue, (void*)&task, sizeof(void*));
+ kfifo_in(&session->cmdpool.queue, (void *)&task, sizeof(void *));
if (sc) {
/* SCSI eh reuses commands to verify us */
@@ -736,7 +736,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct
iscsi_hdr *hdr,
BUG_ON(conn->c_stage == ISCSI_CONN_STOPPED);
if (!kfifo_out(&session->cmdpool.queue,
-(void*)&task, sizeof(void*)))
+(void *)&task, sizeof(void *)))
return NULL;
}
/*
@@ -831,7 +831,7 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn,
struct iscsi_hdr *hdr,
struct iscsi_session *session = conn->session;
struct scsi_cmnd *sc = task->sc;
- iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
+ iscsi_update_cmdsn(session, (struct iscsi_nopin *)rhdr);
conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
sc->result = (DID_OK << 16) | rhdr->cmd_status;
@@ -901,12 +901,12 @@ invalid_datalen:
}
if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW |
- ISCSI_FLAG_CMD_OVERFLOW)) {
+ ISCSI_FLAG_CMD_OVERFLOW)) {
int res_count = be32_to_cpu(rhdr->residual_count);
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-res_count <= scsi_bufflen(sc)))
+ res_count <= scsi_bufflen(sc)))
/* write side for bidi or uni-io set_resid */
scsi_set_resid(sc, res_count);
else
@@ -939,7 +939,7 @@ iscsi_data_in_rsp(struct iscsi_conn *conn, struct iscsi_hdr
*hdr,
sc->result = (DID_OK << 16) | rhdr->cmd_status;
conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
if (rhdr->flags & (ISCSI_FLAG_DATA_UNDERFLOW |
- ISCSI_FLAG_DATA_OVERFLOW)) {
+ ISCSI_FLAG_DATA_OVERFLOW)) {
int res_count = be32_to_cpu(rhdr->residual_count);
if (res_count > 0 &&
@@ -978,7 +978,7 @@ static void iscsi_tmf_rsp(struct iscsi_conn *conn, struct
iscsi_hdr *hdr)
static int iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr)
{
-struct iscsi_nopout hdr;
+ struct iscsi_nopout hdr;
struct iscsi_task *task;
if (!rhdr && conn->ping_task)
@@ -1080,7 +1080,7 @@ static int iscsi_handle_reject(struct iscsi_conn *conn,
struct iscsi_hdr *hdr,
spin_unlock(&conn->session->back_lock);
spin_lock(
