[PATCH] virtio_scsi: fix memory leak on full queue condition.
virtscsi_queuecommand was leaking memory when the virtio queue was full. Tested: Guest operates correctly even with very small queue sizes, validated we're not leaking kmalloc-192 sized allocations anymore. Signed-off-by: Eric Northup --- drivers/scsi/virtio_scsi.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 595af1a..dd8dc27 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -469,6 +469,8 @@ static int virtscsi_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *sc) sizeof cmd->req.cmd, sizeof cmd->resp.cmd, GFP_ATOMIC) >= 0) ret = 0; + else + mempool_free(cmd, virtscsi_cmd_pool); out: return ret; -- 1.7.7.3 -- 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: scsi target, likely GPL violation
On 11/07/2012 05:02 PM, Jon Mason wrote: > On Wed, Nov 7, 2012 at 9:50 AM, Andy Grover wrote: >> Your company appears to be shipping kernel features in RTS OS that are >> not made available under the GPL, specifically support for the >> EXTENDED_COPY and COMPARE_AND_WRITE SCSI commands, in order to claim >> full Vmware vSphere 5 VAAI support. >> >> http://www.risingtidesystems.com/storage.html >> http://www.linux-iscsi.org/wiki/VAAI >> >> Private emails to you and RTS CEO Marc Fleischmann have not elicited a >> useful response. >> >> You are subsystem maintainer for the in-kernel SCSI target support >> (drivers/target/*), and your company appears to be violating the GPL. > > The peanut gallery needs more information, as this is quite an > incendiary claim to be making on a Linux kernel forum. How are they > violating the GPL? I'm not a lawyer, nor do I play one on TV, but if > I understand the GPL correctly, RTS only needs to provide the relevant > source to their customers upon request. Are there customers (perhaps > Redhat) that they have not provided this to? If so, then they need to > be publicly shamed (gpl-violations.org would be a good place to go as > well). If not, then they are within their rights to behave the way > they are currently behaving. > > Again, we need more info before we start flinging the tomatoes at them. Look at the marketing material on their website. They demonstrate how RTS OS fully implements vSphere 5 VAAI features. We know to a very high certainty that this must have been implemented with kernel code, so therefore this must be a GPL violation, since these changes have not been made public. -- Andy -- 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: scsi target, likely GPL violation
On 11/07/2012 05:57 PM, Chris Friesen wrote: > On 11/07/2012 07:02 PM, Jon Mason wrote: >> I'm not a lawyer, nor do I play one on TV, but if >> I understand the GPL correctly, RTS only needs to provide the relevant >> source to their customers upon request. > > Not quite. > > Assuming the GPL applies, and that they have modified the code, then > they must either: > > 1) include the source with the distributed binary > > or > > 2) include with the binary an offer to provide the source to *any* third > party So you'd have me find one of their customers, and then get the source via your #2 method... ...and then turn around and submit it to Nick since he's the target subsystem maintainer? Nick is probably the one who wrote it! I'm happy to do that, but we should recognize something is seriously skewed when the person nominally in charge of the in-kernel code also has a vested interest in *not* seeing new features added, since it then competes better with his company's offering. RTS is trying to use an "open core" business model. This works fine for BSD-licensed code or code originally authored entirely by you, but their code (all of it even the new stuff) is a derivative work of the Linux kernel source code, and the GPL says they need to contribute it back. Regards -- Andy -- 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: scsi target, likely GPL violation
On Thu, 2012-11-08 at 08:57 -0800, Andy Grover wrote: > On 11/07/2012 05:57 PM, Chris Friesen wrote: > > On 11/07/2012 07:02 PM, Jon Mason wrote: > >> I'm not a lawyer, nor do I play one on TV, but if > >> I understand the GPL correctly, RTS only needs to provide the relevant > >> source to their customers upon request. > > > > Not quite. > > > > Assuming the GPL applies, and that they have modified the code, then > > they must either: > > > > 1) include the source with the distributed binary > > > > or > > > > 2) include with the binary an offer to provide the source to *any* third > > party > > So you'd have me find one of their customers, and then get the source > via your #2 method... > > ...and then turn around and submit it to Nick since he's the target > subsystem maintainer? Nick is probably the one who wrote it! > > I'm happy to do that, but we should recognize something is seriously > skewed when the person nominally in charge of the in-kernel code also > has a vested interest in *not* seeing new features added, since it then > competes better with his company's offering. > > RTS is trying to use an "open core" business model. This works fine for > BSD-licensed code or code originally authored entirely by you, but their > code (all of it even the new stuff) is a derivative work of the Linux > kernel source code, and the GPL says they need to contribute it back. > Andy, Accusing us of violating GPL is a serious legal claim. In fact, we are not violating GPL. In short, this is because we wrote the code you are referring to (the SCSI target core in our commercial RTS OS product), we have exclusive copyright ownership of it, and this code contains no GPL code from the community. GPL obligations only apply downstream to licensees, and not to the author of the code. Those who use the code under GPL are subject to its conditions; we are not. As you know, we contributed the Linux SCSI target core, including the relevant interfaces, to the Linux kernel. To be clear, we wrote that code entirely ourselves, so we have the right to use it as we please. The version we use in RTS OS is a different, proprietary version, which we also wrote ourselves. However, the fact that we contributed a version of the code to the Linux kernel does not require us to provide our proprietary version to anyone. If you want to understand better how dual licensing works, perhaps we can talk off list. But we don’t really have a responsibility to respond to untrue accusations, nor to explain GPL, nor discuss our proprietary code. We’re very disappointed that Red Hat would not be more professional in its communications about licensing compliance matters, particularly to a company like ours that has been a major contributor to Linux and therefore also to Red Hat’s own products. So, while I invite you to talk about this with us directly, I also advise you – respectfully – not to make public accusations that are not true. That is harmful to our reputation – and candidly, it doesn’t reflect well on you or your company. --nab -- 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] target/iblock: Fix double iblock_complete_cmd callback bug
From: Nicholas Bellinger
This patch fixes a double completion bug where ibr->pending = 2 usage
plus the extra callback to iblock_complete_cmd() invoked after bio
submission in iblock_execute_rw() can interfere with the normal
bio->bi_done() -> iblock_bio_done() -> iblock_complete_cmd() completion
path, causing a double target_complete_cmd() call to occur.
This bug was introduced during v3.4-rc2 code with:
commit 5787cacd0bd5ee016ad807b244550d34fe2beebe
Author: Christoph Hellwig
Date: Tue Apr 24 00:25:06 2012 -0400
target: remove struct se_task
Also drop the bio_cnt >= IBLOCK_MAX_BIO_PER_TASK rolling call to
iblock_submit_bios() to avoid the exception case where outstanding
bios completing via iblock_bio_done() are not accounted for when
returning returning non zero from iblock_execute_rw().
This bug was originally reported by Kelsey when a single se_cmd
descriptor required more than one bio to complete.
Reported-by: Kelsey Prantis
Cc: Christoph Hellwig
Cc: sta...@vger.kernel.org
Signed-off-by: Nicholas Bellinger
---
drivers/target/target_core_iblock.c | 11 +--
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/target/target_core_iblock.c
b/drivers/target/target_core_iblock.c
index 57d7674..d066932 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -47,7 +47,6 @@
#include "target_core_iblock.h"
-#define IBLOCK_MAX_BIO_PER_TASK 32 /* max # of bios to submit at a
time */
#define IBLOCK_BIO_POOL_SIZE 128
static struct se_subsystem_api iblock_template;
@@ -602,7 +601,6 @@ static int iblock_execute_rw(struct se_cmd *cmd)
struct scatterlist *sg;
u32 sg_num = sgl_nents;
sector_t block_lba;
- unsigned bio_cnt;
int rw;
int i;
@@ -658,8 +656,7 @@ static int iblock_execute_rw(struct se_cmd *cmd)
bio_list_init(&list);
bio_list_add(&list, bio);
- atomic_set(&ibr->pending, 2);
- bio_cnt = 1;
+ atomic_set(&ibr->pending, 1);
for_each_sg(sgl, sg, sgl_nents, i) {
/*
@@ -669,10 +666,6 @@ static int iblock_execute_rw(struct se_cmd *cmd)
*/
while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
!= sg->length) {
- if (bio_cnt >= IBLOCK_MAX_BIO_PER_TASK) {
- iblock_submit_bios(&list, rw);
- bio_cnt = 0;
- }
bio = iblock_get_bio(cmd, block_lba, sg_num);
if (!bio)
@@ -680,7 +673,6 @@ static int iblock_execute_rw(struct se_cmd *cmd)
atomic_inc(&ibr->pending);
bio_list_add(&list, bio);
- bio_cnt++;
}
/* Always in 512 byte units for Linux/Block */
@@ -689,7 +681,6 @@ static int iblock_execute_rw(struct se_cmd *cmd)
}
iblock_submit_bios(&list, rw);
- iblock_complete_cmd(cmd);
return 0;
fail_put_bios:
--
1.7.2.5
--
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 0/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation
From: Nicholas Bellinger Hi folks, This series for-3.8 adds support for proper WRITE_SAME w/ UNMAP=0 emulation for IBLOCK device backends to follow MKP's WRITE_SAME patches that have been merged for v3.7-rc1. Currently it uses a bio_add_page() call for each sector in order to allow scatterlist w/ page offsets to work, as blkdev_issue_write_same() currently assumes underlying hw support + zero page offset. So far it has been tested on target-pending/for-next code with iscsi-target and tcm_loop fabric ports. Please review, --nab Nicholas Bellinger (3): target/sbc: Make WRITE_SAME check differentiate between UNMAP=[1,0] target: Add max_write_same_len device attribute target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation support drivers/target/target_core_configfs.c |4 ++ drivers/target/target_core_device.c | 11 + drivers/target/target_core_iblock.c | 78 +--- drivers/target/target_core_internal.h |1 + drivers/target/target_core_sbc.c | 19 +++- drivers/target/target_core_spc.c |8 +++- include/target/target_core_base.h |4 ++ 7 files changed, 104 insertions(+), 21 deletions(-) -- 1.7.2.5 -- 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 1/3] target/sbc: Make WRITE_SAME check differentiate between UNMAP=[1,0]
From: Nicholas Bellinger
This patch updates sbc_write_same_supported() to set SCF_WRITE_SAME_DISCARD
to signal when WRITE_SAME w/ UNMAP=1 is requested.
Also, allow WRITE_SAME w/ UNMAP=0 to be passed to backend driver logic.
Cc: Christoph Hellwig
Cc: Martin K. Petersen
Signed-off-by: Nicholas Bellinger
---
drivers/target/target_core_sbc.c | 19 +++
include/target/target_core_base.h |1 +
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index b802421..ee9fa52 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -235,7 +235,7 @@ static inline unsigned long long
transport_lba_64_ext(unsigned char *cdb)
return ((unsigned long long)__v2) | (unsigned long long)__v1 << 32;
}
-static int sbc_write_same_supported(struct se_device *dev,
+static int sbc_write_same_supported(struct se_cmd *cmd,
unsigned char *flags)
{
if ((flags[0] & 0x04) || (flags[0] & 0x02)) {
@@ -244,16 +244,11 @@ static int sbc_write_same_supported(struct se_device *dev,
" Emulation\n");
return -ENOSYS;
}
-
/*
-* Currently for the emulated case we only accept
-* tpws with the UNMAP=1 bit set.
+* UNMAP=1 bit translantes to blkdev_issue_discard() execution
*/
- if (!(flags[0] & 0x08)) {
- pr_err("WRITE_SAME w/o UNMAP bit not"
- " supported for Block Discard Emulation\n");
- return -ENOSYS;
- }
+ if (flags[0] & 0x08)
+ cmd->se_cmd_flags |= SCF_WRITE_SAME_DISCARD;
return 0;
}
@@ -431,7 +426,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
size = sbc_get_size(cmd, 1);
cmd->t_task_lba = get_unaligned_be64(&cdb[12]);
- if (sbc_write_same_supported(dev, &cdb[10]) < 0)
+ if (sbc_write_same_supported(cmd, &cdb[10]) < 0)
return TCM_UNSUPPORTED_SCSI_OPCODE;
cmd->execute_cmd = ops->execute_write_same;
break;
@@ -507,7 +502,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
size = sbc_get_size(cmd, 1);
cmd->t_task_lba = get_unaligned_be64(&cdb[2]);
- if (sbc_write_same_supported(dev, &cdb[1]) < 0)
+ if (sbc_write_same_supported(cmd, &cdb[1]) < 0)
return TCM_UNSUPPORTED_SCSI_OPCODE;
cmd->execute_cmd = ops->execute_write_same;
break;
@@ -528,7 +523,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
* Follow sbcr26 with WRITE_SAME (10) and check for the
existence
* of byte 1 bit 3 UNMAP instead of original reserved field
*/
- if (sbc_write_same_supported(dev, &cdb[1]) < 0)
+ if (sbc_write_same_supported(cmd, &cdb[1]) < 0)
return TCM_UNSUPPORTED_SCSI_OPCODE;
cmd->execute_cmd = ops->execute_write_same;
break;
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 5350f6e..2787b85 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -154,6 +154,7 @@ enum se_cmd_flags_table {
SCF_ALUA_NON_OPTIMIZED = 0x8000,
SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC = 0x0002,
SCF_ACK_KREF= 0x0004,
+ SCF_WRITE_SAME_DISCARD = 0x0008,
};
/* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
--
1.7.2.5
--
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 3/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation support
From: Nicholas Bellinger
This patch adds support for emulation of WRITE_SAME w/ UNMAP=0 within
iblock_execute_write_same() backend code.
The emulation uses a bio_add_page() call for each sector, and by default
enforces a limit of max_write_same_len=0x (65536) sectors following
what scsi_debug reports per default for MAXIMUM WRITE SAME LENGTH.
It also sets max_write_same_len to the operational default at setup ->
iblock_configure_device() time.
Cc: Christoph Hellwig
Cc: Martin K. Petersen
Signed-off-by: Nicholas Bellinger
---
drivers/target/target_core_iblock.c | 78 +++
1 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/drivers/target/target_core_iblock.c
b/drivers/target/target_core_iblock.c
index 53f4501..33a5248 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -151,6 +151,10 @@ static int iblock_configure_device(struct se_device *dev)
pr_debug("IBLOCK: BLOCK Discard support available,"
" disabled by default\n");
}
+ /*
+* Enable WRITE_SAME emulation for IBLOCK, use scsi_debug.c default
+*/
+ dev->dev_attrib.max_write_same_len = 0x;
if (blk_queue_nonrot(q))
dev->dev_attrib.is_nonrot = 1;
@@ -375,22 +379,80 @@ err:
return ret;
}
+static struct bio *iblock_get_bio(struct se_cmd *, sector_t, u32);
+static void iblock_submit_bios(struct bio_list *, int);
+static void iblock_complete_cmd(struct se_cmd *);
+
static sense_reason_t
iblock_execute_write_same(struct se_cmd *cmd)
{
struct iblock_dev *ib_dev = IBLOCK_DEV(cmd->se_dev);
- int ret;
+ struct iblock_req *ibr;
+ struct scatterlist *sg;
+ struct bio *bio;
+ struct bio_list list;
+ sector_t block_lba = cmd->t_task_lba;
+ unsigned int sectors = spc_get_write_same_sectors(cmd);
+ int rc;
+
+ if (cmd->se_cmd_flags & SCF_WRITE_SAME_DISCARD) {
+ rc = blkdev_issue_discard(ib_dev->ibd_bd, cmd->t_task_lba,
+ spc_get_write_same_sectors(cmd), GFP_KERNEL, 0);
+ if (rc < 0) {
+ pr_warn("blkdev_issue_discard() failed: %d\n", rc);
+ return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+ }
+ target_complete_cmd(cmd, GOOD);
+ return 0;
+ }
+ if (sectors > cmd->se_dev->dev_attrib.max_write_same_len) {
+ pr_warn("WRITE_SAME sectors: %u exceeds max_write_same_len:
%u\n",
+ sectors, cmd->se_dev->dev_attrib.max_write_same_len);
+ return TCM_INVALID_CDB_FIELD;
+ }
+ sg = &cmd->t_data_sg[0];
- ret = blkdev_issue_discard(ib_dev->ibd_bd, cmd->t_task_lba,
- spc_get_write_same_sectors(cmd), GFP_KERNEL,
- 0);
- if (ret < 0) {
- pr_debug("blkdev_issue_discard() failed for WRITE_SAME\n");
- return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+ ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL);
+ if (!ibr)
+ goto fail;
+ cmd->priv = ibr;
+
+ bio = iblock_get_bio(cmd, block_lba, 1);
+ if (!bio)
+ goto fail_free_ibr;
+
+ bio_list_init(&list);
+ bio_list_add(&list, bio);
+
+ atomic_set(&ibr->pending, 1);
+
+ while (sectors) {
+ while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
+ != sg->length) {
+
+ bio = iblock_get_bio(cmd, block_lba, 1);
+ if (!bio)
+ goto fail_put_bios;
+
+ atomic_inc(&ibr->pending);
+ bio_list_add(&list, bio);
+ }
+
+ /* Always in 512 byte units for Linux/Block */
+ block_lba += sg->length >> IBLOCK_LBA_SHIFT;
+ sectors -= 1;
}
- target_complete_cmd(cmd, GOOD);
+ iblock_submit_bios(&list, WRITE);
return 0;
+
+fail_put_bios:
+ while ((bio = bio_list_pop(&list)))
+ bio_put(bio);
+fail_free_ibr:
+ kfree(ibr);
+fail:
+ return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
enum {
--
1.7.2.5
--
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 2/3] target: Add max_write_same_len device attribute
From: Nicholas Bellinger
This patch adds a new max_write_same_len device attribute for use with
WRITE_SAME w/ UNMAP=0 backend emulation.
Also, update block limits VPD emulation code in spc_emulate_evpd_b0() to
set the default MAXIMUM WRITE SAME LENGTH value of zero.
Cc: Christoph Hellwig
Cc: Martin K. Petersen
Signed-off-by: Nicholas Bellinger
---
drivers/target/target_core_configfs.c |4
drivers/target/target_core_device.c | 11 +++
drivers/target/target_core_internal.h |1 +
drivers/target/target_core_spc.c |8 +++-
include/target/target_core_base.h |3 +++
5 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/drivers/target/target_core_configfs.c
b/drivers/target/target_core_configfs.c
index 7b473b6..2b14164 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -676,6 +676,9 @@ SE_DEV_ATTR(unmap_granularity, S_IRUGO | S_IWUSR);
DEF_DEV_ATTRIB(unmap_granularity_alignment);
SE_DEV_ATTR(unmap_granularity_alignment, S_IRUGO | S_IWUSR);
+DEF_DEV_ATTRIB(max_write_same_len);
+SE_DEV_ATTR(max_write_same_len, S_IRUGO | S_IWUSR);
+
CONFIGFS_EATTR_OPS(target_core_dev_attrib, se_dev_attrib, da_group);
static struct configfs_attribute *target_core_dev_attrib_attrs[] = {
@@ -701,6 +704,7 @@ static struct configfs_attribute
*target_core_dev_attrib_attrs[] = {
&target_core_dev_attrib_max_unmap_block_desc_count.attr,
&target_core_dev_attrib_unmap_granularity.attr,
&target_core_dev_attrib_unmap_granularity_alignment.attr,
+ &target_core_dev_attrib_max_write_same_len.attr,
NULL,
};
diff --git a/drivers/target/target_core_device.c
b/drivers/target/target_core_device.c
index 599374e..54439bc 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -706,6 +706,16 @@ int se_dev_set_unmap_granularity_alignment(
return 0;
}
+int se_dev_set_max_write_same_len(
+ struct se_device *dev,
+ u32 max_write_same_len)
+{
+ dev->dev_attrib.max_write_same_len = max_write_same_len;
+ pr_debug("dev[%p]: Set max_write_same_len: %u\n",
+ dev, dev->dev_attrib.max_write_same_len);
+ return 0;
+}
+
int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
{
if (flag != 0 && flag != 1) {
@@ -1393,6 +1403,7 @@ struct se_device *target_alloc_device(struct se_hba *hba,
const char *name)
dev->dev_attrib.unmap_granularity = DA_UNMAP_GRANULARITY_DEFAULT;
dev->dev_attrib.unmap_granularity_alignment =
DA_UNMAP_GRANULARITY_ALIGNMENT_DEFAULT;
+ dev->dev_attrib.max_write_same_len = DA_MAX_WRITE_SAME_LEN;
dev->dev_attrib.fabric_max_sectors = DA_FABRIC_MAX_SECTORS;
dev->dev_attrib.optimal_sectors = DA_FABRIC_MAX_SECTORS;
diff --git a/drivers/target/target_core_internal.h
b/drivers/target/target_core_internal.h
index bc9c522..93e9c1f 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -24,6 +24,7 @@ int se_dev_set_max_unmap_lba_count(struct se_device *, u32);
intse_dev_set_max_unmap_block_desc_count(struct se_device *, u32);
intse_dev_set_unmap_granularity(struct se_device *, u32);
intse_dev_set_unmap_granularity_alignment(struct se_device *, u32);
+intse_dev_set_max_write_same_len(struct se_device *, u32);
intse_dev_set_emulate_dpo(struct se_device *, int);
intse_dev_set_emulate_fua_write(struct se_device *, int);
intse_dev_set_emulate_fua_read(struct se_device *, int);
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 4b3c183..cf1b8bb 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -465,7 +465,7 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
* Exit now if we don't support TP.
*/
if (!have_tp)
- return 0;
+ goto max_write_same;
/*
* Set MAXIMUM UNMAP LBA COUNT
@@ -491,6 +491,12 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
if (dev->dev_attrib.unmap_granularity_alignment != 0)
buf[32] |= 0x80; /* Set the UGAVALID bit */
+ /*
+* MAXIMUM WRITE SAME LENGTH
+*/
+max_write_same:
+ put_unaligned_be64(dev->dev_attrib.max_write_same_len, &buf[36]);
+
return 0;
}
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 2787b85..1b45879 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -71,6 +71,8 @@
#define DA_UNMAP_GRANULARITY_DEFAULT 0
/* Default unmap_granularity_alignment */
#define DA_UNMAP_GRANULARITY_ALIGNMENT_DEFAULT 0
+/* Default max_write_same_len, disabled by default */
+#define DA_MAX_WRITE_SAME_LEN 0
/* Default max transfer length */
#define DA_FABRIC_MAX_SECTORS 8192
/* E
Re: scsi target, likely GPL violation
>> ...and then turn around and submit it to Nick since he's the target >> subsystem maintainer? Nick is probably the one who wrote it! >> >> I'm happy to do that, but we should recognize something is seriously >> skewed when the person nominally in charge of the in-kernel code also >> has a vested interest in *not* seeing new features added, since it then >> competes better with his company's offering. >> >> RTS is trying to use an "open core" business model. This works fine for >> BSD-licensed code or code originally authored entirely by you, but their >> code (all of it even the new stuff) is a derivative work of the Linux >> kernel source code, and the GPL says they need to contribute it back. >> > > Andy, > > Accusing us of violating GPL is a serious legal claim. > > In fact, we are not violating GPL. In short, this is because we wrote > the code you are referring to (the SCSI target core in our commercial > RTS OS product), we have exclusive copyright ownership of it, and this > code contains no GPL code from the community. GPL obligations only > apply downstream to licensees, and not to the author of the code. Those > who use the code under GPL are subject to its conditions; we are not. Just to clarify since I'm not a major GPL expert. Are you: a) distributing a Linux kernel b) with a module built against it to be linked into it, whether completely written in house or not? Then the module is under the GPL, if you think you are like nvidia or someone then think again, the corner case they live under is that they don't distribute a Linux kernel with their module *ever*, and they have a clear call for non-derived work status since its 90% the same code as in their Windows drivers. But if you distribute a kernel and a module in one place which I assume RTS OS does, then you are in dangerous territory and could be hit with cease and desist notices, which has happened to people shipping kernels and linked nvidia binary drivers before. Dave. -- 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: scsi target, likely GPL violation
On 11/08/2012 12:05 PM, Nicholas A. Bellinger wrote: > Accusing us of violating GPL is a serious legal claim. > > In fact, we are not violating GPL. In short, this is because we wrote > the code you are referring to (the SCSI target core in our commercial > RTS OS product), we have exclusive copyright ownership of it, and this > code contains no GPL code from the community. GPL obligations only > apply downstream to licensees, and not to the author of the code. Those > who use the code under GPL are subject to its conditions; we are not. Hi Nick, thanks for finally responding. I believe your argument is wrong for two reasons. First, LIO is a derivative work of the Linux kernel. It uses kernel APIs and headers. You ship Linux as part of RTS OS. Even if you had not asked for LIO to be included in mainline, this would still be true and would require you to publish your changes under the GPLv2. Second, you claim you hold exclusive copyright for the code. Not true. One example: on http://www.risingtidesystems.com/storage.html you claim support for FCoE. You didn't build tcm_fc, Intel did. Under the GPLv2. Furthermore, SRP support came from SCST, iirc. None of these contributors gave RTS any right to use their copyrighted code except under the conditions of the GPLv2. > As you know, we contributed the Linux SCSI target core, including the > relevant interfaces, to the Linux kernel. To be clear, we wrote that > code entirely ourselves, so we have the right to use it as we please. > The version we use in RTS OS is a different, proprietary version, which > we also wrote ourselves. However, the fact that we contributed a > version of the code to the Linux kernel does not require us to provide > our proprietary version to anyone. In addition to the two reasons above, how does it make any sense that you are spending time maintaining in-tree LIO when none of that code can theoretically benefit RTS's proprietary product? The more likely scenario is you are basing your product on mainline LIO. > If you want to understand better how dual licensing works, perhaps we > can talk off list. But we don’t really have a responsibility to respond > to untrue accusations, nor to explain GPL, nor discuss our proprietary > code. All the contributions and improvements (there have been a LOT) since LIO entered mainline are GPLed by their authors. If you incorporated any of those improvements or fixes into RTS OS, then there's a third reason why your code is subject to the GPL. > We’re very disappointed that Red Hat would not be more professional in > its communications about licensing compliance matters, particularly to a > company like ours that has been a major contributor to Linux and > therefore also to Red Hat’s own products. So, while I invite you to > talk about this with us directly, I also advise you – respectfully – not > to make public accusations that are not true. That is harmful to our > reputation – and candidly, it doesn’t reflect well on you or your > company. I tried the private route but you and your CEO stalled, and then stopped talking to me. Please just start behaving properly w/r/t the GPL so we can all get back to coding. Regards -- Andy -- 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 1/2] drivers/message/fusion/mptscsih.c: missing break
From: Alan Cox Subject: drivers/message/fusion/mptscsih.c: missing break This happens to do the right thing in all cases on fibre channel but not on other media types Signed-off-by: Alan Cox Cc: James Bottomley Cc: Nagalakshmi Nandigama Cc: Kashyap Desai Signed-off-by: Andrew Morton --- drivers/message/fusion/mptscsih.c |1 + 1 file changed, 1 insertion(+) diff -puN drivers/message/fusion/mptscsih.c~drivers-message-fusion-mptscsihc-missing-break drivers/message/fusion/mptscsih.c --- a/drivers/message/fusion/mptscsih.c~drivers-message-fusion-mptscsihc-missing-break +++ a/drivers/message/fusion/mptscsih.c @@ -792,6 +792,7 @@ mptscsih_io_done(MPT_ADAPTER *ioc, MPT_F * than an unsolicited DID_ABORT. */ sc->result = DID_RESET << 16; + break; case MPI_IOCSTATUS_SCSI_EXT_TERMINATED: /* 0x004C */ if (ioc->bus_type == FC) _ -- 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 2/2] drivers/scsi/hptiop: support HighPoint RR4520/RR4522 HBA
From: HighPoint Linux Team Subject: drivers/scsi/hptiop: support HighPoint RR4520/RR4522 HBA Support IOP RR4520/RR4522 which are based on Marvell frey. Signed-off-by: HighPoint Linux Team Signed-off-by: Andrew Morton --- Documentation/scsi/hptiop.txt | 69 + drivers/scsi/hptiop.c | 413 ++-- drivers/scsi/hptiop.h | 72 + 3 files changed, 530 insertions(+), 24 deletions(-) diff -puN Documentation/scsi/hptiop.txt~hptiop-support-highpoint-rr4520-rr4522-hba Documentation/scsi/hptiop.txt --- a/Documentation/scsi/hptiop.txt~hptiop-support-highpoint-rr4520-rr4522-hba +++ a/Documentation/scsi/hptiop.txt @@ -37,7 +37,7 @@ For Intel IOP based adapters, the contro 0x40Inbound Queue Port 0x44Outbound Queue Port -For Marvell IOP based adapters, the IOP is accessed via PCI BAR0 and BAR1: +For Marvell not Frey IOP based adapters, the IOP is accessed via PCI BAR0 and BAR1: BAR0 offsetRegister 0x20400Inbound Doorbell Register @@ -55,9 +55,31 @@ For Marvell IOP based adapters, the IOP 0x40-0x1040Inbound Queue 0x1040-0x2040Outbound Queue +For Marvell Frey IOP based adapters, the IOP is accessed via PCI BAR0 and BAR1: -I/O Request Workflow --- + BAR0 offsetRegister + 0x0IOP configuration information. + + BAR1 offsetRegister + 0x4000Inbound List Base Address Low + 0x4004Inbound List Base Address High + 0x4018Inbound List Write Pointer + 0x402CInbound List Configuration and Control + 0x4050Outbound List Base Address Low + 0x4054Outbound List Base Address High + 0x4058Outbound List Copy Pointer Shadow Base Address Low + 0x405COutbound List Copy Pointer Shadow Base Address High + 0x4088Outbound List Interrupt Cause + 0x408COutbound List Interrupt Enable + 0x1020CPCIe Function 0 Interrupt Enable + 0x10400PCIe Function 0 to CPU Message A + 0x10420CPU to PCIe Function 0 Message A + 0x10480CPU to PCIe Function 0 Doorbell + 0x10484CPU to PCIe Function 0 Doorbell Enable + + +I/O Request Workflow of Not Marvell Frey +-- All queued requests are handled via inbound/outbound queue port. A request packet can be allocated in either IOP or host memory. @@ -101,6 +123,45 @@ register 0. An outbound message with the of an inbound message. +I/O Request Workflow of Marvell Frey +-- + +All queued requests are handled via inbound/outbound list. + +To send a request to the controller: + +- Allocate a free request in host DMA coherent memory. + + Requests allocated in host memory must be aligned on 32-bytes boundary. + +- Fill the request with index of the request in the flag. + + Fill a free inbound list unit with the physical address and the size of + the request. + + Set up the inbound list write pointer with the index of previous unit, + round to 0 if the index reaches the supported count of requests. + +- Post the inbound list writer pointer to IOP. + +- The IOP process the request. When the request is completed, the flag of + the request with or-ed IOPMU_QUEUE_MASK_HOST_BITS will be put into a + free outbound list unit and the index of the outbound list unit will be + put into the copy pointer shadow register. An outbound interrupt will be + generated. + +- The host read the outbound list copy pointer shadow register and compare + with previous saved read ponter N. If they are different, the host will + read the (N+1)th outbound list unit. + + The host get the index of the request from the (N+1)th outbound list + unit and complete the request. + +Non-queued requests (reset communication/reset/flush etc) can be sent via PCIe +Function 0 to CPU Message A register. The CPU to PCIe Function 0 Message register +with the same value indicates the completion of message. + + User-level Interface - @@ -112,7 +173,7 @@ The driver exposes following sysfs attri - -Copyright (C) 2006-2009 HighPoint Technologies, Inc. All Rights Reserved. +Copyright (C) 2006-2012 HighPoint Technologies, Inc. All Rights Reserved. This file is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of diff -puN drivers/scsi/hptiop.c~hptiop-support-highpoint-rr4520-rr4522-hba drivers/scsi/hptiop.c --- a/drivers/scsi/hptiop.c~hptiop-support-highpoint-rr4520-rr4522-hba +++ a/drivers/scsi/hptiop.c @@ -1,6 +1,6 @@ /* * HighPoint RR3xxx/4xxx controller driver for Linux - * Copyright (C) 2006-2009 HighPoint Technologies, Inc. All Rights Reserved. + * Copyright (C)
Re: scsi target, likely GPL violation
On Thu, 2012-11-08 at 13:22 -0800, Andy Grover wrote: > On 11/08/2012 12:05 PM, Nicholas A. Bellinger wrote: > > Accusing us of violating GPL is a serious legal claim. > > > > In fact, we are not violating GPL. In short, this is because we wrote > > the code you are referring to (the SCSI target core in our commercial > > RTS OS product), we have exclusive copyright ownership of it, and this > > code contains no GPL code from the community. GPL obligations only > > apply downstream to licensees, and not to the author of the code. Those > > who use the code under GPL are subject to its conditions; we are not. > > Hi Nick, thanks for finally responding. > > I believe your argument is wrong for two reasons. > > First, LIO is a derivative work of the Linux kernel. It uses kernel APIs > and headers. You ship Linux as part of RTS OS. Even if you had not asked > for LIO to be included in mainline, this would still be true and would > require you to publish your changes under the GPLv2. > > Second, you claim you hold exclusive copyright for the code. Not true. > One example: on http://www.risingtidesystems.com/storage.html you claim > support for FCoE. You didn't build tcm_fc, Intel did. Under the GPLv2. > Furthermore, SRP support came from SCST, iirc. None of these > contributors gave RTS any right to use their copyrighted code except > under the conditions of the GPLv2. > Andy, Support for certified VAAI is part of our commercial target core. The target core constitutes a stand-alone kernel subsystem of which we are the sole copyright owners. In addition, our target contains a number of backend drivers, of which we are also the sole copyright owners, and a number of fabric modules, of which some we are the sole copyright owners, and of which others we are not, as you pointed out. A substantial fraction of the code of which we own the sole copyright was certified by BlackDuck Software as early as in 2007. We contributed our target to the Linux kernel in 2010, at which point we forked it into the upstream version and our commercial version. These target versions have been diverging over time, as we keep maintaining either one of them independently. For our commercial target core, we only use Linux kernel symbols that are not marked as GPL. In addition, we define the API between the target core and its backend drivers and between the target core and its fabric modules, we define the ABI between the target core and user space, and we have done so years before our code went upstream into the Linux kernel. We have been contributing substantially to the upstream target version to keep improving Linux. We have also been improving our commercial target version to afford the considerable effort and expense involved in our ongoing Linux contributions, and to compensate other top Linux kernel developers for their contributions to the upstream target version. RTS OS is based on a stock Linux enterprise kernel. This Linux kernel has naturally the ability to load either one of our standalone self-contained target module versions without any modifications. Again, we’re very disappointed by these untrue and highly damaging accusations from Red Hat. We have generously contributed to Linux, and we have generously supported the Linux community for their contributions to our upstream target and other Linux kernel parts. You have mostly just incorporated our work into Red Hat’s products. So yes, Andy, please start behaving properly, so that at least we can get back to making Linux better. --nab -- 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] virtio-scsi: Fix incorrect lock release order in virtscsi_kick_cmd
From: Nicholas Bellinger This patch fixes a regression bug in virtscsi_kick_cmd() that relinquishes the acquired spinlocks in the incorrect order using the wrong spin_unlock macros, namely releasing vq->vq_lock before tgt->tgt_lock while invoking the calls to virtio_ring.c:virtqueue_add_buf() and friends. This bug was originally introduced in v3.5-rc7 code with: commit 2bd37f0fde99cbf8b78fb55f1128e8c3a63cf1da Author: Paolo Bonzini Date: Wed Jun 13 16:56:34 2012 +0200 [SCSI] virtio-scsi: split scatterlist per target Go ahead and make sure that vq->vq_lock is relinquished w/ spin_unlock first, then release tgt->tgt_lock w/ spin_unlock_irqrestore. Cc: Paolo Bonzini Cc: James Bottomley Cc: Christoph Hellwig Cc: sta...@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/scsi/virtio_scsi.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 595af1a..b2abb8a 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -417,11 +417,11 @@ static int virtscsi_kick_cmd(struct virtio_scsi_target_state *tgt, spin_lock(&vq->vq_lock); ret = virtqueue_add_buf(vq->vq, tgt->sg, out_num, in_num, cmd, gfp); - spin_unlock(&tgt->tgt_lock); + spin_unlock(&vq->vq_lock); if (ret >= 0) ret = virtqueue_kick_prepare(vq->vq); - spin_unlock_irqrestore(&vq->vq_lock, flags); + spin_unlock_irqrestore(&tgt->tgt_lock, flags); if (ret > 0) virtqueue_notify(vq->vq); -- 1.7.2.5 -- 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 v9 00/10] ZPODD Patches
v9: Build ZPODD as part of libata instead of another standalone module as it is tightly related to other libata files. Identify and init ZPODD during probe time instead of after SCSI device is created as suggested by Tejun Heo. Make use of pm qos flag to give ACPI hint when choosing ACPI state. Expose qos flag to give user control of whether power off is allowed. This patchset used Rafael's pm-qos work: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-qos v8: This version is a redesign, it doesn't have much to do with previous versions. The ZPODD implementation is done almost entirely in ATA layer now, except 2 helper functions from SCSI sr driver to block disk events. The basic idea is that, when ata port is runtime suspended, it will check if the ODD is ready to be powered off. And if yes, events is blocked and power omitted; if not, ODD's power supply remains unchanged by keeping ACPI state at D0. Some background knowledge about ZPODD is added below v1 history log. v7: Re work of runtime pm of sr driver, based on ideas of Alan Stern and Oliver Neukum. Jeff, due to the ready_to_power_off flag added, there is a small change in [PATCH v7 6/6] libata: acpi: respect may_power_off flag, please check if I can still get your ack, thanks. v6: When user changes may_power_off flag through sysfs entry and if device is already runtime suspended, resume resume it so that it can respect this flag next time it is runtime suspended as suggested by Alan Stern. Call scsi_autopm_get/put_device once in sr_check_events as suggested by Alan Stern. v5: Add may_power_off flag to scsi device. Alan Stern suggested that I should not mess runtime suspend with runtime power off, but the current zpodd implementation made it not easy to seperate. So I re-wrote the zpodd implementation, the end result is, normal ODD can also enter runtime suspended state, but their power won't be removed. v4: Rebase on top of Linus' tree, due to this, the problem of a missing flag in v3 is gone; Add a new function scsi_autopm_put_device_autosuspend to first mark last busy for the device and then put autosuspend it as suggested by Oliver Neukum. Typo fix as pointed by Sergei Shtylyov. Check can_power_off flag before any runtime pm operations in sr. v3: Rebase on top of scsi-misc tree; Add the sr related patches previously in Jeff's libata tree; Re-organize the sr patches. A problem for now: for patch scsi: sr: support zero power ODD(ZPODD) I can't set a flag in libata-acpi.c since a related function is missing in scsi-misc tree. Will fix this when 3.6-rc1 released. v2: Bug fix for v1; Use scsi_autopm_* in sr driver instead of pm_runtime_*; v1: Here are some patches to make ZPODD easier to use for end users and a fix for using ZPODD with system suspend. Some background knowledge about ZPODD: ODD means Optical Disc Drive. ZPODD means Zero Power ODD, it is a mechanism to place the ODD into zero power state when the system is running at S0 system state without user's awareness. It achieved this by ACPI and SATA device attention pin. For power off, normal ACPI control method is used to place the device into D3 cold ACPI device state, aka. device power supply omitted. For power on, when user press the eject button of a drawer type ODD or when user inserts an ODD into a slot type ODD, the device attention pin will trigger. In the current x86 implementation, this pin will connect to a GPE, and the GPE will trigger an ACPI interrupt. With our pre-registered ACPI notification code, the device can be runtime resumed, and we place the device back to full power state by setting its ACPI state to D0. The whole process is transparent to the end user. Aaron Lu (10): scsi: sr: support runtime pm ata: zpodd: Add CONFIG_SATA_ZPODD ata: zpodd: identify and init ZPODD devices libata: acpi: move acpi notification code to zpodd libata: separate ATAPI code ata: zpodd: check zero power ready status block: add a new interface to block events scsi: sr: support (un)block events ata: zpodd: handle power transition of ODD ata: expose pm qos flags to user space for ata device block/genhd.c | 26 drivers/ata/Kconfig| 12 ++ drivers/ata/Makefile | 3 +- drivers/ata/libata-acpi.c | 123 ++--- drivers/ata/libata-atapi.c | 88 drivers/ata/libata-core.c | 4 +- drivers/ata/libata-eh.c| 87 +--- drivers/ata/libata-scsi.c | 4 + drivers/ata/libata-zpodd.c | 333 + drivers/ata/libata.h | 33 + drivers/scsi/Makefile | 1 + drivers/scsi/sr.c | 30 +++- drivers/scsi/sr_zpodd.c| 21 +++ drivers/scsi/sr_zpodd.h| 9 ++ include/linux/genhd.h | 1 + include/linux/libata.h | 2 + include/uapi/linux/cdrom.h | 34 + 17 files changed, 638 insertions(+), 173 deletions(-) create mode 100644 drivers/ata/libata-atapi.c create mode 100644 drivers/ata/libata-zpodd.c create
[PATCH v9 01/10] scsi: sr: support runtime pm
This patch adds runtime pm support for sr.
It did this by increasing the runtime usage_count of the device when:
- its block device is opened;
- the events checking is to run.
And decreasing the runtime usage_count of the device when:
- its block device is closed;
- After the events checking is done.
The idea is discussed in this mail thread:
http://thread.gmane.org/gmane.linux.acpi.devel/55243/focus=52703
Signed-off-by: Aaron Lu
---
drivers/scsi/sr.c | 30 +-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..4d1a610 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include
#include
@@ -146,7 +147,8 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk
*disk)
kref_get(&cd->kref);
if (scsi_device_get(cd->device))
goto out_put;
- goto out;
+ if (!scsi_autopm_get_device(cd->device))
+ goto out;
out_put:
kref_put(&cd->kref, sr_kref_release);
@@ -162,6 +164,7 @@ static void scsi_cd_put(struct scsi_cd *cd)
mutex_lock(&sr_ref_mutex);
kref_put(&cd->kref, sr_kref_release);
+ scsi_autopm_put_device(sdev);
scsi_device_put(sdev);
mutex_unlock(&sr_ref_mutex);
}
@@ -211,7 +214,7 @@ static unsigned int sr_check_events(struct
cdrom_device_info *cdi,
unsigned int clearing, int slot)
{
struct scsi_cd *cd = cdi->handle;
- bool last_present;
+ bool last_present = cd->media_present;
struct scsi_sense_hdr sshdr;
unsigned int events;
int ret;
@@ -220,6 +223,8 @@ static unsigned int sr_check_events(struct
cdrom_device_info *cdi,
if (CDSL_CURRENT != slot)
return 0;
+ scsi_autopm_get_device(cd->device);
+
events = sr_get_events(cd->device);
cd->get_event_changed |= events & DISK_EVENT_MEDIA_CHANGE;
@@ -246,10 +251,9 @@ static unsigned int sr_check_events(struct
cdrom_device_info *cdi,
}
if (!(clearing & DISK_EVENT_MEDIA_CHANGE))
- return events;
+ goto out;
do_tur:
/* let's see whether the media is there with TUR */
- last_present = cd->media_present;
ret = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
/*
@@ -270,7 +274,7 @@ do_tur:
}
if (cd->ignore_get_event)
- return events;
+ goto out;
/* check whether GET_EVENT is reporting spurious MEDIA_CHANGE */
if (!cd->tur_changed) {
@@ -287,6 +291,18 @@ do_tur:
cd->tur_changed = false;
cd->get_event_changed = false;
+out:
+ /*
+* If there is no medium detected or the medium has been there
+* since last poll, try to suspend the device. Otherwise, keep
+* it active for one more poll interval so that if user space
+* application opens the block device, we can avoid a runtime
+* status change.
+*/
+ pm_runtime_put_noidle(&cd->device->sdev_gendev);
+ if (!cd->media_present || last_present)
+ pm_runtime_suspend(&cd->device->sdev_gendev);
+
return events;
}
@@ -718,6 +734,8 @@ static int sr_probe(struct device *dev)
sdev_printk(KERN_DEBUG, sdev,
"Attached scsi CD-ROM %s\n", cd->cdi.name);
+ scsi_autopm_put_device(cd->device);
+
return 0;
fail_put:
@@ -965,6 +983,8 @@ static int sr_remove(struct device *dev)
{
struct scsi_cd *cd = dev_get_drvdata(dev);
+ scsi_autopm_get_device(cd->device);
+
blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
del_gendisk(cd->disk);
--
1.7.12.4
--
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 v9 02/10] ata: zpodd: Add CONFIG_SATA_ZPODD
Added a new config CONFIG_SATA_ZPODD, which is used to support SATA based zero power ODD. It depends on ATA_ACPI, and selects BLK_DEV_SR as the implementation of ZPODD depends on SCSI sr driver. A new file libata-zpodd.c is added, which will be used to host ZPODD related code. It is empty for this commit. Signed-off-by: Aaron Lu --- drivers/ata/Kconfig | 12 drivers/ata/Makefile | 1 + 2 files changed, 13 insertions(+) create mode 100644 drivers/ata/libata-zpodd.c diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index e08d322..9bcb8fb 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -58,6 +58,18 @@ config ATA_ACPI You can disable this at kernel boot time by using the option libata.noacpi=1 +config SATA_ZPODD + bool "SATA Zero Power ODD Support" + depends on ATA_ACPI + select BLK_DEV_SR + default n + help + This option adds support for SATA ZPODD. It requires both + ODD and the platform support, and if enabled, will automatically + power on/off the ODD when certain condition is satisfied. This + does not impact user's experience of the ODD, only power is saved + when ODD is not in use(i.e. no disc inside). + config SATA_PMP bool "SATA Port Multiplier support" default y diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index 9329daf..85e3de4 100644 --- a/drivers/ata/Makefile +++ b/drivers/ata/Makefile @@ -107,3 +107,4 @@ libata-y:= libata-core.o libata-scsi.o libata-eh.o libata-transport.o libata-$(CONFIG_ATA_SFF) += libata-sff.o libata-$(CONFIG_SATA_PMP) += libata-pmp.o libata-$(CONFIG_ATA_ACPI) += libata-acpi.o +libata-$(CONFIG_SATA_ZPODD)+= libata-zpodd.o diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c new file mode 100644 index 000..e69de29 -- 1.7.12.4 -- 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 v9 03/10] ata: zpodd: identify and init ZPODD devices
The ODD can be enabled for ZPODD if the following three conditions are
satisfied:
1 The ODD supports device attention;
2 The platform can runtime power off the ODD through ACPI;
3 The ODD is either slot type or drawer type.
For such ODDs, zpodd_init is called and a new structure is allocated for
it to store ZPODD related stuffs.
And the zpodd_dev_enabled function is used to test if ZPODD is currently
enabled for this ODD.
Signed-off-by: Aaron Lu
---
drivers/ata/libata-acpi.c | 2 +
drivers/ata/libata-core.c | 4 +-
drivers/ata/libata-zpodd.c | 124 +
drivers/ata/libata.h | 17 +++
include/uapi/linux/cdrom.h | 34 +
5 files changed, 180 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index fd9ecf7..3c61100 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1059,6 +1059,8 @@ void ata_acpi_bind(struct ata_device *dev)
void ata_acpi_unbind(struct ata_device *dev)
{
+ if (zpodd_dev_enabled(dev))
+ zpodd_deinit(dev);
ata_acpi_remove_pm_notifier(dev);
ata_acpi_unregister_power_resource(dev);
}
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3cc7096..a2293b6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2395,8 +2395,10 @@ int ata_dev_configure(struct ata_device *dev)
dma_dir_string = ", DMADIR";
}
- if (ata_id_has_da(dev->id))
+ if (ata_id_has_da(dev->id)) {
dev->flags |= ATA_DFLAG_DA;
+ zpodd_init(dev);
+ }
/* print device info to dmesg */
if (ata_msg_drv(ap) && print_info)
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index e69de29..fce6ea6 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -0,0 +1,124 @@
+#include
+#include
+
+#include "libata.h"
+
+struct zpodd {
+ bool slot:1;
+ bool drawer:1;
+
+ struct ata_device *dev;
+};
+
+static int run_atapi_cmd(struct ata_device *dev, const char *cdb,
+ unsigned short cdb_len, char *buf, unsigned int buf_len)
+{
+ struct ata_taskfile tf = {0};
+
+ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+ tf.command = ATA_CMD_PACKET;
+
+ if (buf) {
+ tf.protocol = ATAPI_PROT_PIO;
+ tf.lbam = buf_len;
+ } else {
+ tf.protocol = ATAPI_PROT_NODATA;
+ }
+
+ return ata_exec_internal(dev, &tf, cdb,
+ buf ? DMA_FROM_DEVICE : DMA_NONE, buf, buf_len, 0);
+}
+
+/*
+ * Per the spec, only slot type and drawer type ODD can be supported
+ *
+ * Return 0 for slot type, 1 for drawer, -ENODEV for other types or on error.
+ */
+static int check_loading_mechanism(struct ata_device *dev)
+{
+ char buf[16];
+ unsigned int ret;
+ struct rm_feature_desc *desc = (void *)(buf + 8);
+
+ char cdb[] = { GPCMD_GET_CONFIGURATION,
+ 2, /* only 1 feature descriptor requested */
+ 0, 3, /* 3, removable medium feature */
+ 0, 0, 0,/* reserved */
+ 0, sizeof(buf),
+ 0, 0, 0,
+ };
+
+ ret = run_atapi_cmd(dev, cdb, sizeof(cdb), buf, sizeof(buf));
+ if (ret)
+ return -ENODEV;
+
+ if (be16_to_cpu(desc->feature_code) != 3)
+ return -ENODEV;
+
+ if (desc->mech_type == 0 && desc->load == 0 && desc->eject == 1)
+ return 0; /* slot */
+ else if (desc->mech_type == 1 && desc->load == 0 && desc->eject == 1)
+ return 1; /* drawer */
+ else
+ return -ENODEV;
+}
+
+static bool device_can_poweroff(struct ata_device *ata_dev)
+{
+ acpi_handle handle;
+ acpi_status status;
+ struct acpi_device_power_state *states;
+ struct acpi_device *acpi_dev;
+
+ handle = ata_dev_acpi_handle(ata_dev);
+ if (!handle)
+ return false;
+
+ status = acpi_bus_get_device(handle, &acpi_dev);
+ if (ACPI_FAILURE(status))
+ return false;
+
+ /*
+* If firmware has _PS3 or _PR3 for this device,
+* it means this device can be runtime powered off
+*/
+ states = acpi_dev->power.states;
+ if (states[ACPI_STATE_D3_HOT].flags.valid ||
+ states[ACPI_STATE_D3_COLD].flags.explicit_set)
+ return true;
+ else
+ return false;
+}
+
+void zpodd_init(struct ata_device *dev)
+{
+ int ret;
+ struct zpodd *zpodd;
+
+ if (dev->private_data)
+ return;
+
+ if (!device_can_poweroff(dev))
+ return;
+
+ if ((ret = check_loading_mechanism(dev)) == -ENODEV)
+ return;
+
+ zpodd = kzalloc(sizeof(struct zpodd),
[PATCH v9 04/10] libata: acpi: move acpi notification code to zpodd
Since the ata acpi notification code introduced in commit
3bd46600a7a7e938c54df8cdbac9910668c7dfb0 is solely for ZPODD, and we
now have a dedicated place for it, move these code there.
And the add/remove_pm_notifier code is simplified a little bit that it
does not check things like if the handle is NULL and if a corresponding
acpi_device is there for the handle as they are guaranteed by the
device_can_poweroff already.
Signed-off-by: Aaron Lu
---
drivers/ata/libata-acpi.c | 71 --
drivers/ata/libata-zpodd.c | 30
2 files changed, 30 insertions(+), 71 deletions(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 3c61100..6b6819c 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -970,57 +970,6 @@ void ata_acpi_on_disable(struct ata_device *dev)
ata_acpi_clear_gtf(dev);
}
-static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
-{
- struct ata_device *ata_dev = context;
-
- if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev &&
- pm_runtime_suspended(&ata_dev->sdev->sdev_gendev))
- scsi_autopm_get_device(ata_dev->sdev);
-}
-
-static void ata_acpi_add_pm_notifier(struct ata_device *dev)
-{
- struct acpi_device *acpi_dev;
- acpi_handle handle;
- acpi_status status;
-
- handle = ata_dev_acpi_handle(dev);
- if (!handle)
- return;
-
- status = acpi_bus_get_device(handle, &acpi_dev);
- if (ACPI_FAILURE(status))
- return;
-
- if (dev->sdev->can_power_off) {
- acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- ata_acpi_wake_dev, dev);
- device_set_run_wake(&dev->sdev->sdev_gendev, true);
- }
-}
-
-static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
-{
- struct acpi_device *acpi_dev;
- acpi_handle handle;
- acpi_status status;
-
- handle = ata_dev_acpi_handle(dev);
- if (!handle)
- return;
-
- status = acpi_bus_get_device(handle, &acpi_dev);
- if (ACPI_FAILURE(status))
- return;
-
- if (dev->sdev->can_power_off) {
- device_set_run_wake(&dev->sdev->sdev_gendev, false);
- acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- ata_acpi_wake_dev);
- }
-}
-
static void ata_acpi_register_power_resource(struct ata_device *dev)
{
struct scsi_device *sdev = dev->sdev;
@@ -1053,7 +1002,6 @@ static void ata_acpi_unregister_power_resource(struct
ata_device *dev)
void ata_acpi_bind(struct ata_device *dev)
{
- ata_acpi_add_pm_notifier(dev);
ata_acpi_register_power_resource(dev);
}
@@ -1061,7 +1009,6 @@ void ata_acpi_unbind(struct ata_device *dev)
{
if (zpodd_dev_enabled(dev))
zpodd_deinit(dev);
- ata_acpi_remove_pm_notifier(dev);
ata_acpi_unregister_power_resource(dev);
}
@@ -1103,9 +1050,6 @@ static int ata_acpi_bind_device(struct ata_port *ap,
struct scsi_device *sdev,
acpi_handle *handle)
{
struct ata_device *ata_dev;
- acpi_status status;
- struct acpi_device *acpi_dev;
- struct acpi_device_power_state *states;
if (ap->flags & ATA_FLAG_ACPI_SATA)
ata_dev = &ap->link.device[sdev->channel];
@@ -1117,21 +1061,6 @@ static int ata_acpi_bind_device(struct ata_port *ap,
struct scsi_device *sdev,
if (!*handle)
return -ENODEV;
- status = acpi_bus_get_device(*handle, &acpi_dev);
- if (ACPI_FAILURE(status))
- return 0;
-
- /*
-* If firmware has _PS3 or _PR3 for this device,
-* and this ata ODD device support device attention,
-* it means this device can be powered off
-*/
- states = acpi_dev->power.states;
- if ((states[ACPI_STATE_D3_HOT].flags.valid ||
- states[ACPI_STATE_D3_COLD].flags.explicit_set) &&
- ata_dev->flags & ATA_DFLAG_DA)
- sdev->can_power_off = 1;
-
return 0;
}
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index fce6ea6..ba8c985 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -1,11 +1,14 @@
#include
#include
+#include
+#include
#include "libata.h"
struct zpodd {
bool slot:1;
bool drawer:1;
+ bool from_notify:1; /* resumed as a result of acpi notification */
struct ata_device *dev;
};
@@ -90,6 +93,31 @@ static bool device_can_poweroff(struct ata_device *ata_dev)
return false;
}
+static void zpodd_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct ata_device *ata_dev = context;
+ struct zpodd *zpodd = ata_dev->private_data;
+ struct device *dev = &ata_dev->sdev->sdev_gendev;
+
+
[PATCH v9 06/10] ata: zpodd: check zero power ready status
Per the Mount Fuji spec, the ODD is considered zero power ready when:
- For slot type ODD, no media inside;
- For tray type ODD, no media inside and tray closed.
The information can be retrieved by either the returned information of
command GET_EVENT_STATUS_NOTIFICATION(the command is used to poll for
media event) or sense code.
In this implementation, the zero power ready status is determined by
the following factors:
1 polled media status byte, and this info is recorded in status_ready
field of zpodd structure;
2 sense code by issuing a TEST_UNIT_READY command after status_ready
is found to be true.
The information provided by the media status byte is not accurate, it is
possible that after a new disc is just inserted, the status byte still
returns media not present. So this information can not be used as the
final deciding factor. But since SCSI ODD driver sr will always poll the
ODD every 2 seconds, this information is readily available without any
much cost. So it is used as a hint: when we find zero power ready status
in the media status byte, we will see if it is really the case with the
sense code method. This way, we can avoid sending too many
TEST_UNIT_READY commands to the ODD.
When we first sensed the ODD in the zero power ready state, the
timestamp will be recoreded. And after ODD stayed in this state for some
pre-defined period, the ODD is considered as power off ready and the
zp_ready flag will be set. The zp_ready flag serves as the deciding
factor other code will use to see if power off is OK for the ODD. The
Mount Fuji spec suggests a delay should be used here, to avoid the case
user ejects the ODD and then instantly inserts a new one again, so that
we can avoid a power transition. And some ODDs may be slow to place its
head to the home position after disc is ejected, so a delay here is
generally a good idea.
The zero power ready status check is performed in the ata port's runtime
suspend code path, when port is not frozen yet, as we need to issue some
IOs to the ODD.
Signed-off-by: Aaron Lu
---
drivers/ata/libata-acpi.c | 8 +++-
drivers/ata/libata-scsi.c | 4 ++
drivers/ata/libata-zpodd.c | 116 +
drivers/ata/libata.h | 4 ++
4 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 6b6819c..13ee178 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -784,7 +784,13 @@ static int ata_acpi_push_id(struct ata_device *dev)
*/
int ata_acpi_on_suspend(struct ata_port *ap)
{
- /* nada */
+ struct ata_device *dev;
+
+ ata_for_each_dev(dev, &ap->link, ENABLED) {
+ if (zpodd_dev_enabled(dev))
+ zpodd_check_zpready(dev);
+ }
+
return 0;
}
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e3bda07..6f235b9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2665,6 +2665,10 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
ata_scsi_rbuf_put(cmd, true, &flags);
}
+ if (zpodd_dev_enabled(qc->dev) &&
+ scsicmd[0] == GET_EVENT_STATUS_NOTIFICATION)
+ zpodd_snoop_status(qc->dev, cmd);
+
cmd->result = SAM_STAT_GOOD;
}
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index ba8c985..533a39e 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -2,13 +2,21 @@
#include
#include
#include
+#include
#include "libata.h"
+#define POWEROFF_DELAY (30 * 1000) /* 30 seconds for power off delay */
+
struct zpodd {
bool slot:1;
bool drawer:1;
bool from_notify:1; /* resumed as a result of acpi notification */
+ bool status_ready:1;/* ready status derived from media event poll,
+ it is not accurate, but serves as a hint */
+ bool zp_ready:1;/* zero power ready state */
+
+ unsigned long last_ready; /* last zero power ready timestamp */
struct ata_device *dev;
};
@@ -93,6 +101,114 @@ static bool device_can_poweroff(struct ata_device *ata_dev)
return false;
}
+/*
+ * Snoop the result of GET_STATUS_NOTIFICATION_EVENT, the media
+ * status byte has information on media present/door closed.
+ *
+ * This information serves only as a hint, as it is not accurate.
+ * The sense code method will be used when deciding if the ODD is
+ * really zero power ready.
+ */
+void zpodd_snoop_status(struct ata_device *dev, struct scsi_cmnd *scmd)
+{
+ bool ready;
+ char buf[8];
+ struct event_header *eh = (void *)buf;
+ struct media_event_desc *med = (void *)(buf + 4);
+ struct sg_table *table = &scmd->sdb.table;
+ struct zpodd *zpodd = dev->private_data;
+
+ if (sg_copy_to_buffer(table->sgl, table->nents, buf,
[PATCH v9 05/10] libata: separate ATAPI code
The atapi_eh_tur and atapi_eh_request_sense can be reused by ZPODD
code, so separate them out to a file named libata-atapi.c, and the
Makefile is modified accordingly. No functional changes should result
from this commit.
Signed-off-by: Aaron Lu
---
drivers/ata/Makefile | 2 +-
drivers/ata/libata-atapi.c | 88 ++
drivers/ata/libata-eh.c| 85
drivers/ata/libata.h | 4 +++
4 files changed, 93 insertions(+), 86 deletions(-)
create mode 100644 drivers/ata/libata-atapi.c
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 85e3de4..36377f9 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -103,7 +103,7 @@ obj-$(CONFIG_ATA_GENERIC) += ata_generic.o
# Should be last libata driver
obj-$(CONFIG_PATA_LEGACY) += pata_legacy.o
-libata-y := libata-core.o libata-scsi.o libata-eh.o libata-transport.o
+libata-y := libata-core.o libata-scsi.o libata-eh.o libata-atapi.o
libata-transport.o
libata-$(CONFIG_ATA_SFF) += libata-sff.o
libata-$(CONFIG_SATA_PMP) += libata-pmp.o
libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
diff --git a/drivers/ata/libata-atapi.c b/drivers/ata/libata-atapi.c
new file mode 100644
index 000..28684ae
--- /dev/null
+++ b/drivers/ata/libata-atapi.c
@@ -0,0 +1,88 @@
+#include
+#include
+#include "libata.h"
+
+/**
+ * atapi_eh_tur - perform ATAPI TEST_UNIT_READY
+ * @dev: target ATAPI device
+ * @r_sense_key: out parameter for sense_key
+ *
+ * Perform ATAPI TEST_UNIT_READY.
+ *
+ * LOCKING:
+ * EH context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask on failure.
+ */
+unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key)
+{
+ u8 cdb[ATAPI_CDB_LEN] = { TEST_UNIT_READY, 0, 0, 0, 0, 0 };
+ struct ata_taskfile tf;
+ unsigned int err_mask;
+
+ ata_tf_init(dev, &tf);
+
+ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+ tf.command = ATA_CMD_PACKET;
+ tf.protocol = ATAPI_PROT_NODATA;
+
+ err_mask = ata_exec_internal(dev, &tf, cdb, DMA_NONE, NULL, 0, 0);
+ if (err_mask == AC_ERR_DEV)
+ *r_sense_key = tf.feature >> 4;
+ return err_mask;
+}
+
+/**
+ * atapi_eh_request_sense - perform ATAPI REQUEST_SENSE
+ * @dev: device to perform REQUEST_SENSE to
+ * @sense_buf: result sense data buffer (SCSI_SENSE_BUFFERSIZE bytes long)
+ * @dfl_sense_key: default sense key to use
+ *
+ * Perform ATAPI REQUEST_SENSE after the device reported CHECK
+ * SENSE. This function is EH helper.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask on failure
+ */
+unsigned int atapi_eh_request_sense(struct ata_device *dev,
+ u8 *sense_buf, u8 dfl_sense_key)
+{
+ u8 cdb[ATAPI_CDB_LEN] = {
+ REQUEST_SENSE, 0, 0, 0, SCSI_SENSE_BUFFERSIZE, 0 };
+ struct ata_port *ap = dev->link->ap;
+ struct ata_taskfile tf;
+
+ DPRINTK("ATAPI request sense\n");
+
+ /* FIXME: is this needed? */
+ memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
+
+ /* initialize sense_buf with the error register,
+* for the case where they are -not- overwritten
+*/
+ sense_buf[0] = 0x70;
+ sense_buf[2] = dfl_sense_key;
+
+ /* some devices time out if garbage left in tf */
+ ata_tf_init(dev, &tf);
+
+ tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+ tf.command = ATA_CMD_PACKET;
+
+ /* is it pointless to prefer PIO for "safety reasons"? */
+ if (ap->flags & ATA_FLAG_PIO_DMA) {
+ tf.protocol = ATAPI_PROT_DMA;
+ tf.feature |= ATAPI_PKT_DMA;
+ } else {
+ tf.protocol = ATAPI_PROT_PIO;
+ tf.lbam = SCSI_SENSE_BUFFERSIZE;
+ tf.lbah = 0;
+ }
+
+ return ata_exec_internal(dev, &tf, cdb, DMA_FROM_DEVICE,
+sense_buf, SCSI_SENSE_BUFFERSIZE, 0);
+}
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index e60437c..6487b88 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1579,91 +1579,6 @@ static int ata_eh_read_log_10h(struct ata_device *dev,
}
/**
- * atapi_eh_tur - perform ATAPI TEST_UNIT_READY
- * @dev: target ATAPI device
- * @r_sense_key: out parameter for sense_key
- *
- * Perform ATAPI TEST_UNIT_READY.
- *
- * LOCKING:
- * EH context (may sleep).
- *
- * RETURNS:
- * 0 on success, AC_ERR_* mask on failure.
- */
-static unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key)
-{
- u8 cdb[ATAPI_CDB_LEN] = { TEST_UNIT_READY, 0, 0, 0, 0, 0 };
- struct ata_taskfile tf;
- unsigned int err_mask;
-
- ata_tf_init(dev, &tf);
-
- tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
- tf.command = ATA_CMD_PACKET;
-
[PATCH v9 08/10] scsi: sr: support (un)block events
2 interfaces are added to block/unblock events for the disk sr manages.
This is used by SATA ZPODD, when ODD is runtime powered off, the events
poll is no longer needed so better be blocked. And once powered on,
events poll will be unblocked.
These 2 interfaces are needed here because SATA layer does not have
access to the gendisk structure sr manages.
Signed-off-by: Aaron Lu
---
drivers/scsi/Makefile | 1 +
drivers/scsi/sr_zpodd.c | 21 +
drivers/scsi/sr_zpodd.h | 9 +
3 files changed, 31 insertions(+)
create mode 100644 drivers/scsi/sr_zpodd.c
create mode 100644 drivers/scsi/sr_zpodd.h
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 888f73a..474efe2 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -177,6 +177,7 @@ sd_mod-objs := sd.o
sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o
sr_mod-objs:= sr.o sr_ioctl.o sr_vendor.o
+sr_mod-$(CONFIG_SATA_ZPODD) += sr_zpodd.o
ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
:= -DCONFIG_NCR53C8XX_PREFETCH -DSCSI_NCR_BIG_ENDIAN \
-DCONFIG_SCSI_NCR53C8XX_NO_WORD_TRANSFERS
diff --git a/drivers/scsi/sr_zpodd.c b/drivers/scsi/sr_zpodd.c
new file mode 100644
index 000..0686e8c
--- /dev/null
+++ b/drivers/scsi/sr_zpodd.c
@@ -0,0 +1,21 @@
+#include
+#include
+#include
+#include "sr.h"
+
+bool sr_block_events(struct device *dev)
+{
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+ if (cd)
+ return disk_try_block_events(cd->disk);
+ return false;
+}
+EXPORT_SYMBOL(sr_block_events);
+
+void sr_unblock_events(struct device *dev)
+{
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+ if (cd)
+ disk_unblock_events(cd->disk);
+}
+EXPORT_SYMBOL(sr_unblock_events);
diff --git a/drivers/scsi/sr_zpodd.h b/drivers/scsi/sr_zpodd.h
new file mode 100644
index 000..49c6a55
--- /dev/null
+++ b/drivers/scsi/sr_zpodd.h
@@ -0,0 +1,9 @@
+#ifndef __SR_ZPODD_H__
+#define __SR_ZPODD_H__
+
+#include
+
+extern bool sr_block_events(struct device *dev);
+extern void sr_unblock_events(struct device *dev);
+
+#endif
--
1.7.12.4
--
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 v9 09/10] ata: zpodd: handle power transition of ODD
When ata port is runtime suspended, it will check if the ODD attched to
it is in zero power ready state by checking the zp_ready field. And if
this field indicates it is not ready to be powered off, NO_POWEROFF qos
flag will be set to avoid choosing ACPI_STATE_D3_COLD for it.
Once powered off, disk events will be blocked to avoid waking it up
every two seconds.
And on resume, it will re-gain power and go through the recovery
process. When reset for the ata port is done, the ODD is considered
functional, and post processing like eject tray if the ODD is drawer
type is done there. And disk events is unblocked here.
For normal ODDs that do not support zero power state, NO_POWEROFF qos
flag will always be set.
Signed-off-by: Aaron Lu
---
drivers/ata/libata-acpi.c | 40 ++---
drivers/ata/libata-eh.c| 2 ++
drivers/ata/libata-zpodd.c | 63 ++
drivers/ata/libata.h | 8 ++
include/linux/libata.h | 2 ++
5 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 13ee178..91f3405 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -838,6 +838,24 @@ void ata_acpi_on_resume(struct ata_port *ap)
}
}
+static int ata_acpi_choose_state(struct ata_device *dev)
+{
+ /* Always choose D3 for PATA devices */
+ if (!(dev->link->ap->flags & ATA_FLAG_ACPI_SATA))
+ return ACPI_STATE_D3;
+
+ if (zpodd_dev_enabled(dev)) {
+ if (zpodd_poweroff_ready(dev))
+ dev_pm_qos_update_request(&dev->poweroff_req, 0);
+ else
+ dev_pm_qos_update_request(&dev->poweroff_req,
+ PM_QOS_FLAG_NO_POWER_OFF);
+ }
+
+ return acpi_pm_device_sleep_state(&dev->sdev->sdev_gendev,
+ NULL, ACPI_STATE_D3_COLD);
+}
+
/**
* ata_acpi_set_state - set the port power state
* @ap: target ATA port
@@ -864,17 +882,16 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t
state)
continue;
if (state.event != PM_EVENT_ON) {
- acpi_state = acpi_pm_device_sleep_state(
- &dev->sdev->sdev_gendev, NULL, ACPI_STATE_D3);
- if (acpi_state > 0)
+ acpi_state = ata_acpi_choose_state(dev);
+ if (acpi_state > 0) {
acpi_bus_set_power(handle, acpi_state);
- /* TBD: need to check if it's runtime pm request */
- acpi_pm_device_run_wake(
- &dev->sdev->sdev_gendev, true);
+ if (zpodd_dev_enabled(dev) &&
+ acpi_state == ACPI_STATE_D3_COLD)
+ zpodd_post_poweroff(dev);
+ }
} else {
- /* Ditto */
- acpi_pm_device_run_wake(
- &dev->sdev->sdev_gendev, false);
+ if (zpodd_dev_enabled(dev))
+ zpodd_pre_poweron(dev);
acpi_bus_set_power(handle, ACPI_STATE_D0);
}
}
@@ -1008,7 +1025,12 @@ static void ata_acpi_unregister_power_resource(struct
ata_device *dev)
void ata_acpi_bind(struct ata_device *dev)
{
+ /* ODD can't be put to D3 cold state, unless it is zero power capable */
+ s32 value = dev->class == ATA_DEV_ATAPI ? PM_QOS_FLAG_NO_POWER_OFF : 0;
+
ata_acpi_register_power_resource(dev);
+ dev_pm_qos_add_request(&dev->sdev->sdev_gendev, &dev->poweroff_req,
+ DEV_PM_QOS_FLAGS, value);
}
void ata_acpi_unbind(struct ata_device *dev)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 6487b88..1348e7c 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -3771,6 +3771,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t
prereset,
rc = atapi_eh_clear_ua(dev);
if (rc)
goto rest_fail;
+ if (zpodd_dev_enabled(dev))
+ zpodd_post_resume(dev);
}
}
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index 533a39e..777f9c7 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -5,6 +5,7 @@
#include
#include "libata.h"
+#include "../scsi/sr_zpodd.h"
#define POWEROFF_DELAY (30 * 1000) /* 30 seconds for power off delay */
@@ -15,6 +16,7 @@ struct zpodd {
bool status_ready:1;/* ready status derived from media event poll,
it is not ac
[PATCH v9 10/10] ata: expose pm qos flags to user space for ata device
Expose pm qos flags to user space so that user has a chance to disable pm features like power off, if he/she has a broken platform or devices or simply does not like this pm feature. This flag is exposed to user space only for ata device or atapi device that is zero power capable. For normal atapi device, it will never be powered off. Signed-off-by: Aaron Lu --- drivers/ata/libata-acpi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 91f3405..3984290 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -1031,6 +1031,8 @@ void ata_acpi_bind(struct ata_device *dev) ata_acpi_register_power_resource(dev); dev_pm_qos_add_request(&dev->sdev->sdev_gendev, &dev->poweroff_req, DEV_PM_QOS_FLAGS, value); + if (dev->class == ATA_DEV_ATA || zpodd_dev_enabled(dev)) + dev_pm_qos_expose_flags(&dev->sdev->sdev_gendev, 0); } void ata_acpi_unbind(struct ata_device *dev) -- 1.7.12.4 -- 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 v9 07/10] block: add a new interface to block events
A new interface to block disk events is added, this interface is
meant to eliminate a race between PM runtime callback and disk events
checking.
Suppose the following device tree:
device_sata_port (the parent)
device_ODD (the child)
When ODD is runtime suspended, sata port will have a chance to enter
runtime suspended state. And in sata port's runtime suspend callback,
it will check if it is OK to omit the power of the ODD. And if yes, the
periodically running events checking work will be stopped, as the ODD
will be waken up by that check and cancel it can make the ODD stay in
zero power state much longer(no worry about how the ODD gets media
change event in ZPODD's case).
I used disk_block_events to do the events blocking, but there is a race
and can lead to a deadlock: when I call disk_block_events in sata port's
runtime suspend callback, the events checking work may already be running
and it will resume the ODD synchronously, and PM core will try to resume
its parent, the sata port first. The PM core makes sure that runtime
resume callback does not run concurrently with runtime suspend callback,
and if the runtime suspend callback is running, the runtime resume
callback will wait for it done. So the events checking work will wait
for sata port's runtime suspend callback, while the sata port's runtime
suspend callback is waiting for the disk events work to finish. Deadlock.
ODD_suspenddisk_events_workfn
ata_port_suspend check_events
disk_block_events resume ODD
cancel_delayed_work_sync resume parent
(waiting for disk_events_workfn) (waiting for suspend callback)
So a new function disk_try_block_events is added, it will try to
cancel the delayed work if it is pending. If succeed, disk_block_events
will be called and we are done; if failed, false is returned without
doing anything. In this way, the race can be avoided.
The newly added interface and the disk_unblock_events are exported, as
sr driver will need to use them to block/unblock disk events.
Signed-off-by: Aaron Lu
---
block/genhd.c | 26 ++
include/linux/genhd.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index 6cace66..8632fd3 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1469,6 +1469,31 @@ void disk_block_events(struct gendisk *disk)
mutex_unlock(&ev->block_mutex);
}
+/*
+ * Under some circumstances, there is a race between the calling thread
+ * of disk_block_events and the events checking function. To avoid such a race,
+ * this function will check if the delayed work is pending. If not, it means
+ * the work is either not queued or is already running, false is returned.
+ * And if yes, try to cancel the delayed work. If succedded, disk_block_events
+ * will be called and there is no worry that cancel_delayed_work_sync will
+ * deadlock the events checking function. And if failed, false is returned.
+ */
+bool disk_try_block_events(struct gendisk *disk)
+{
+ struct disk_events *ev = disk->ev;
+
+ if (!ev)
+ return false;
+
+ if (cancel_delayed_work(&disk->ev->dwork)) {
+ disk_block_events(disk);
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(disk_try_block_events);
+
static void __disk_unblock_events(struct gendisk *disk, bool check_now)
{
struct disk_events *ev = disk->ev;
@@ -1512,6 +1537,7 @@ void disk_unblock_events(struct gendisk *disk)
if (disk->ev)
__disk_unblock_events(disk, false);
}
+EXPORT_SYMBOL(disk_unblock_events);
/**
* disk_flush_events - schedule immediate event checking and flushing
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 4f440b3..b67247f 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -420,6 +420,7 @@ static inline int get_disk_ro(struct gendisk *disk)
}
extern void disk_block_events(struct gendisk *disk);
+extern bool disk_try_block_events(struct gendisk *disk);
extern void disk_unblock_events(struct gendisk *disk);
extern void disk_flush_events(struct gendisk *disk, unsigned int mask);
extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask);
--
1.7.12.4
--
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 RESEND v4 0/5] Migrate SCSI drivers to use dev_pm_ops
This patchset has been quiet for a while, so resend them. v4: Only Patch 4 is modified: Fixed a line over 80 characters warning by checkpatch.pl; Update the changelog so that it is no more a try :-) v3: Only patch 4 is modified: Remove the special case for system freeze in scsi_bus_suspend_common as pointed out by Alan Stern; Updated some comments; Removed the use of typedef (*pm_callback_t)(struct device *). v2: Change the runtime suspend behaviour of sd driver by putting the device into stopped power state. Revert 2 patches which are no longer needed as pointed out by Alan Stern. Find out device callbacks in bus callbacks as suggested by Alan Stern. Due to these changes, patch number grows from 2 -> 5. v1: The 2 patches will migrate SCSI drivers to use the pm callbacks defined in dev_pm_ops as pm_message is deprecated and should not be used by driver. Bus level callback is changed to use callbacks defined in dev_pm_ops when needed and sd's pm callback is updated to use what are defined in dev_pm_ops. Aaron Lu (5): sd: put to stopped power state when runtime suspend Revert "[SCSI] scsi_pm: set device runtime state before parent suspended" Revert "[SCSI] runtime resume parent for child's system-resume" pm: use callbacks from dev_pm_ops for scsi devices sd: update sd to use the new pm callbacks drivers/scsi/scsi_pm.c | 98 +++--- drivers/scsi/sd.c | 18 +++--- 2 files changed, 67 insertions(+), 49 deletions(-) -- 1.7.12.21.g871e293 -- 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 v4 1/5] sd: put to stopped power state when runtime suspend
When device is runtime suspended, put it to stopped power state to save
some power.
This will also make the behaviour consistent with what the scsi_pm.c
thinks about sd as the comment says:
sd treats runtime suspend, system suspend and system hibernate identical.
With this patch, it is now identical.
And sd_shutdown will also do nothing when it finds the device has been
runtime suspended, if we do not spin down the disk in runtime suspend
by putting it into stopped power state, the disk will be shut down
incorrectly.
And the the same problem can be solved for runtime power off after
runtime suspended case by this change.
With the current runtime scheme for disk, it will only be runtime
suspended when no process opens the disk, so this shouldn't happen a
lot, which makes it acceptable to spin down the disk when runtime
suspended. If some day a more aggressive runtime scheme is used, like
the 'request based runtime pm for disk' that Alan Stern and Lin Ming
has been working, we can introduce some policy to control this. But for
now, make it simple and correct by spinning down the disk.
Signed-off-by: Aaron Lu
Acked-by: Alan Stern
Acked-by: Rafael J. Wysocki
---
drivers/scsi/sd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 12f6fdf..8b6e004 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2911,7 +2911,8 @@ static int sd_suspend(struct device *dev, pm_message_t
mesg)
goto done;
}
- if ((mesg.event & PM_EVENT_SLEEP) && sdkp->device->manage_start_stop) {
+ if (((mesg.event & PM_EVENT_SLEEP) || PMSG_IS_AUTO(mesg)) &&
+ sdkp->device->manage_start_stop) {
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
ret = sd_start_stop_device(sdkp, 0);
}
--
1.7.12.21.g871e293
--
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 v4 2/5] Revert "[SCSI] scsi_pm: set device runtime state before parent suspended"
This reverts commit 33a2285d96b5e7b9500612ec623bf4313397bb53.
With commit 88d26136a256576e444db312179e17af6dd0ea87 (PM: Prevent
runtime suspend during system resume), this patch is no longer needed.
Signed-off-by: Aaron Lu
Acked-by: Alan Stern
Acked-by: Rafael J. Wysocki
---
drivers/scsi/scsi_pm.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index dc0ad85..d4201de 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -76,24 +76,23 @@ static int scsi_bus_resume_common(struct device *dev)
{
int err = 0;
- /*
-* Parent device may have runtime suspended as soon as
-* it is woken up during the system resume.
-*
-* Resume it on behalf of child.
-*/
- pm_runtime_get_sync(dev->parent);
-
- if (scsi_is_sdev_device(dev))
+ if (scsi_is_sdev_device(dev)) {
+ /*
+* Parent device may have runtime suspended as soon as
+* it is woken up during the system resume.
+*
+* Resume it on behalf of child.
+*/
+ pm_runtime_get_sync(dev->parent);
err = scsi_dev_type_resume(dev);
+ pm_runtime_put_sync(dev->parent);
+ }
+
if (err == 0) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
}
-
- pm_runtime_put_sync(dev->parent);
-
return err;
}
--
1.7.12.21.g871e293
--
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 v4 3/5] Revert "[SCSI] runtime resume parent for child's system-resume"
This reverts commit 28fd00d42cca178638f51c08efa986a777c24a4b.
With commit 88d26136a256576e444db312179e17af6dd0ea87 (PM: Prevent
runtime suspend during system resume), this patch is no longer needed.
Signed-off-by: Aaron Lu
Acked-by: Alan Stern
Acked-by: Rafael J. Wysocki
---
drivers/scsi/scsi_pm.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index d4201de..9923b26 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -76,17 +76,8 @@ static int scsi_bus_resume_common(struct device *dev)
{
int err = 0;
- if (scsi_is_sdev_device(dev)) {
- /*
-* Parent device may have runtime suspended as soon as
-* it is woken up during the system resume.
-*
-* Resume it on behalf of child.
-*/
- pm_runtime_get_sync(dev->parent);
+ if (scsi_is_sdev_device(dev))
err = scsi_dev_type_resume(dev);
- pm_runtime_put_sync(dev->parent);
- }
if (err == 0) {
pm_runtime_disable(dev);
--
1.7.12.21.g871e293
--
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 v4 4/5] pm: use callbacks from dev_pm_ops for scsi devices
Use of pm_message_t is deprecated and device driver is not supposed
to use that. This patch migrates the SCSI bus level pm callbacks
to call device's pm callbacks defined in its driver's dev_pm_ops.
This is achieved by finding out which device pm callback should be used
in bus callback function, and then pass that callback function pointer
as a param to the scsi_bus_{suspend,resume}_common routine, which will
further pass that callback to scsi_dev_type_{suspend,resume} after
proper handling.
The special case for freeze in scsi_bus_suspend_common is not necessary
since there is no high level SCSI driver has implemented freeze, so no
need to runtime resume the device if it is in runtime suspended state
for system freeze, just return like the system suspend/hibernate case.
Since only sd has implemented drv->suspend/drv->resume, and I'll update
sd driver to use the new callbacks in the following patch, there is no
need to fallback to call drv->suspend/drv->resume if dev_pm_ops is NULL.
Signed-off-by: Aaron Lu
Acked-by: Alan Stern
Acked-by: Rafael J. Wysocki
---
drivers/scsi/scsi_pm.c | 86 +++---
1 file changed, 53 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 9923b26..8f6b12c 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -16,16 +16,14 @@
#include "scsi_priv.h"
-static int scsi_dev_type_suspend(struct device *dev, pm_message_t msg)
+static int scsi_dev_type_suspend(struct device *dev, int (*cb)(struct device
*))
{
- struct device_driver *drv;
int err;
err = scsi_device_quiesce(to_scsi_device(dev));
if (err == 0) {
- drv = dev->driver;
- if (drv && drv->suspend) {
- err = drv->suspend(dev, msg);
+ if (cb) {
+ err = cb(dev);
if (err)
scsi_device_resume(to_scsi_device(dev));
}
@@ -34,14 +32,12 @@ static int scsi_dev_type_suspend(struct device *dev,
pm_message_t msg)
return err;
}
-static int scsi_dev_type_resume(struct device *dev)
+static int scsi_dev_type_resume(struct device *dev, int (*cb)(struct device *))
{
- struct device_driver *drv;
int err = 0;
- drv = dev->driver;
- if (drv && drv->resume)
- err = drv->resume(dev);
+ if (cb)
+ err = cb(dev);
scsi_device_resume(to_scsi_device(dev));
dev_dbg(dev, "scsi resume: %d\n", err);
return err;
@@ -49,35 +45,33 @@ static int scsi_dev_type_resume(struct device *dev)
#ifdef CONFIG_PM_SLEEP
-static int scsi_bus_suspend_common(struct device *dev, pm_message_t msg)
+static int
+scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
{
int err = 0;
if (scsi_is_sdev_device(dev)) {
/*
-* sd is the only high-level SCSI driver to implement runtime
-* PM, and sd treats runtime suspend, system suspend, and
-* system hibernate identically (but not system freeze).
+* All the high-level SCSI drivers that implement runtime
+* PM treat runtime suspend, system suspend, and system
+* hibernate identically.
*/
- if (pm_runtime_suspended(dev)) {
- if (msg.event == PM_EVENT_SUSPEND ||
- msg.event == PM_EVENT_HIBERNATE)
- return 0; /* already suspended */
+ if (pm_runtime_suspended(dev))
+ return 0;
- /* wake up device so that FREEZE will succeed */
- pm_runtime_resume(dev);
- }
- err = scsi_dev_type_suspend(dev, msg);
+ err = scsi_dev_type_suspend(dev, cb);
}
+
return err;
}
-static int scsi_bus_resume_common(struct device *dev)
+static int
+scsi_bus_resume_common(struct device *dev, int (*cb)(struct device *))
{
int err = 0;
if (scsi_is_sdev_device(dev))
- err = scsi_dev_type_resume(dev);
+ err = scsi_dev_type_resume(dev, cb);
if (err == 0) {
pm_runtime_disable(dev);
@@ -102,26 +96,49 @@ static int scsi_bus_prepare(struct device *dev)
static int scsi_bus_suspend(struct device *dev)
{
- return scsi_bus_suspend_common(dev, PMSG_SUSPEND);
+ const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ return scsi_bus_suspend_common(dev, pm ? pm->suspend : NULL);
+}
+
+static int scsi_bus_resume(struct device *dev)
+{
+ const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ return scsi_bus_resume_common(dev, pm ? pm->resume : NULL);
}
static int scsi_bus_freeze(struct device *dev)
{
- return scsi_bus_suspend_common(dev, PMSG_FREEZE);
[PATCH v4 5/5] sd: update sd to use the new pm callbacks
Update sd driver to use the callbacks defined in dev_pm_ops.
sd_freeze is NULL, the bus level callback has taken care of quiescing
the device so there should be nothing needs to be done here.
Consequently, sd_thaw is not needed here either.
suspend, poweroff and runtime suspend share the same routine sd_suspend,
which will sync flush and then stop the drive, this is the same as before.
resume, restore and runtime resume share the same routine sd_resume,
which will start the drive by putting it into active power state, this
is also the same as before.
Signed-off-by: Aaron Lu
Acked-by: Alan Stern
Acked-by: Rafael J. Wysocki
---
drivers/scsi/sd.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8b6e004..6564305 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -104,7 +104,7 @@ static void sd_unlock_native_capacity(struct gendisk *disk);
static int sd_probe(struct device *);
static int sd_remove(struct device *);
static void sd_shutdown(struct device *);
-static int sd_suspend(struct device *, pm_message_t state);
+static int sd_suspend(struct device *);
static int sd_resume(struct device *);
static void sd_rescan(struct device *);
static int sd_done(struct scsi_cmnd *);
@@ -423,15 +423,23 @@ static struct class sd_disk_class = {
.dev_attrs = sd_disk_attrs,
};
+static const struct dev_pm_ops sd_pm_ops = {
+ .suspend= sd_suspend,
+ .resume = sd_resume,
+ .poweroff = sd_suspend,
+ .restore= sd_resume,
+ .runtime_suspend= sd_suspend,
+ .runtime_resume = sd_resume,
+};
+
static struct scsi_driver sd_template = {
.owner = THIS_MODULE,
.gendrv = {
.name = "sd",
.probe = sd_probe,
.remove = sd_remove,
- .suspend= sd_suspend,
- .resume = sd_resume,
.shutdown = sd_shutdown,
+ .pm = &sd_pm_ops,
},
.rescan = sd_rescan,
.done = sd_done,
@@ -2896,7 +2904,7 @@ exit:
scsi_disk_put(sdkp);
}
-static int sd_suspend(struct device *dev, pm_message_t mesg)
+static int sd_suspend(struct device *dev)
{
struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
int ret = 0;
@@ -2911,8 +2919,7 @@ static int sd_suspend(struct device *dev, pm_message_t
mesg)
goto done;
}
- if (((mesg.event & PM_EVENT_SLEEP) || PMSG_IS_AUTO(mesg)) &&
- sdkp->device->manage_start_stop) {
+ if (sdkp->device->manage_start_stop) {
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
ret = sd_start_stop_device(sdkp, 0);
}
--
1.7.12.21.g871e293
--
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 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
> On 11/6/12 7:06 AM, James Bottomley wrote: > > > > Why is this necessary? As I read the reg set assignment code, it finds > > a free bit in the 64 bit register and uses that ... which can never be > > greater than 64 so there's no need for the check. > > This patch just tries to be more defensive for bit(reg_set) with a > broken reg_set value. I agree with you that it's not that necessary. Agree with James, and just need to do NOT operation one time > > > The other two look OK (probably redone as a single patch with a stable > > tag), but I'd like the input of the mvs people since it seems with the > > current code, we only use 32 bit regsets and probably hang if we go over > > that. The bug fix is either to enable the full 64 if it works, or > > possibly cap at 32 ... what works with all released devices? > > Thanks for reviewing. Yeah we'd better to wait for the input from > the mvs people. About patch 3, I check the ffz code and found it will check ~0 conditions. -- 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] virtio-scsi: Fix incorrect lock release order in virtscsi_kick_cmd
On 11/09/2012 02:29 PM, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > This patch fixes a regression bug in virtscsi_kick_cmd() that relinquishes > the acquired spinlocks in the incorrect order using the wrong spin_unlock > macros, namely releasing vq->vq_lock before tgt->tgt_lock while invoking > the calls to virtio_ring.c:virtqueue_add_buf() and friends. > > This bug was originally introduced in v3.5-rc7 code with: > > commit 2bd37f0fde99cbf8b78fb55f1128e8c3a63cf1da > Author: Paolo Bonzini > Date: Wed Jun 13 16:56:34 2012 +0200 > > [SCSI] virtio-scsi: split scatterlist per target > > Go ahead and make sure that vq->vq_lock is relinquished w/ spin_unlock > first, then release tgt->tgt_lock w/ spin_unlock_irqrestore. Did you hit any error? I don't think this order is wrong. Thanks, Wanlong Gao > > Cc: Paolo Bonzini > Cc: James Bottomley > Cc: Christoph Hellwig > Cc: sta...@vger.kernel.org > Signed-off-by: Nicholas Bellinger > --- > drivers/scsi/virtio_scsi.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c > index 595af1a..b2abb8a 100644 > --- a/drivers/scsi/virtio_scsi.c > +++ b/drivers/scsi/virtio_scsi.c > @@ -417,11 +417,11 @@ static int virtscsi_kick_cmd(struct > virtio_scsi_target_state *tgt, > > spin_lock(&vq->vq_lock); > ret = virtqueue_add_buf(vq->vq, tgt->sg, out_num, in_num, cmd, gfp); > - spin_unlock(&tgt->tgt_lock); > + spin_unlock(&vq->vq_lock); > if (ret >= 0) > ret = virtqueue_kick_prepare(vq->vq); > > - spin_unlock_irqrestore(&vq->vq_lock, flags); > + spin_unlock_irqrestore(&tgt->tgt_lock, flags); > > if (ret > 0) > virtqueue_notify(vq->vq); > -- 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
