RE: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR

[Kiwoong Kim]

Hi, Martin.

I think that the patch is correct.
UFS spec says "The Data Segment area is empty" for Read Descriptor.
I have been using similar code with it and it works.
That have been already applied in Android kernel.

Reviewed-by: Kiwoong Kim 

Regards.

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Martin K. Petersen
> Sent: Wednesday, September 28, 2016 2:14 PM
> To: subha...@codeaurora.org
> Cc: Zang Leigang; vinholika...@gmail.com; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-scsi-ow...@vger.kernel.org
> Subject: Re: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR
> 
> > "Subhash" == subhashj   writes:
> 
> Subhash> Looks good to me.
> 
> > -   /* Data segment length */
> > -   ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD(
> > -   0, 0, len >> 8, (u8)len);
> > +   /* Data segment length only need for WRITE_DESC */
> > +   if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
> > +   ucd_req_ptr->header.dword_2 =
> > +   UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len);
> > +   else
> > +   ucd_req_ptr->header.dword_2 = 0;
> 
> What about READ_DESC?
> 
> --
> Martin K. PetersenOracle Linux Engineering
> --
> 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


--
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] cxgb4i: set completion bit in work request

[Martin K. Petersen]

> "Varun" == Varun Prakash  writes:

Varun> If SKCBF_TX_FLAG_COMPL flag is set and unacknowledged credits are
Varun> >= max credits / 2, set completion bit in work request to reclaim
Varun> credits.

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v2] UFS: Date Segment only need for WRITE DESCRIPTOR

[Martin K. Petersen]

> "Subhash" == subhashj   writes:

Subhash> Looks good to me.

> - /* Data segment length */
> - ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD(
> - 0, 0, len >> 8, (u8)len);
> + /* Data segment length only need for WRITE_DESC */
> + if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
> + ucd_req_ptr->header.dword_2 =
> + UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len);
> + else
> + ucd_req_ptr->header.dword_2 = 0;

What about READ_DESC?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: [PATCHv2] MAINTAINERS: Update open-iscsi maintainers

[Martin K. Petersen]

> "Lee" == Lee Duncan  writes:

Lee> Yes, that would be great. Thank you.

Applied to 4.9/scsi-queue.

>> Is it your plan to go through the SCSI tree?

Lee> Yes, the iscsi initiator kernel code updates have been going
Lee> through the Linux SCSI mailing list and repository for a while,
Lee> now.

Yep. Just wanted to make sure.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: ufs: enable no vccq quirk for skhynix device

[Martin K. Petersen]

> "Kyuho" == Kyuho Choi  writes:

Kyuho> This patch enable no vccq quirk for SKHynix devices.  SKHynix ufs
Kyuho> device don't need vccq vrail for device operation.

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: be2iscsi: mark symbols static where possible

[Martin K. Petersen]

> "Baoyou" == Baoyou Xie  writes:

Baoyou> We get 6 warnings when building kernel with W=1:
Baoyou> drivers/scsi/be2iscsi/be_main.c:65:1: warning: no previous
Baoyou> prototype for 'beiscsi_log_enable_disp' [-Wmissing-prototypes]
Baoyou> drivers/scsi/be2iscsi/be_main.c:78:1: warning: no previous
Baoyou> prototype for 'beiscsi_log_enable_change' [-Wmissing-prototypes]
Baoyou> drivers/scsi/be2iscsi/be_main.c:97:1: warning: no previous
Baoyou> prototype for 'beiscsi_log_enable_store' [-Wmissing-prototypes]
Baoyou> drivers/scsi/be2iscsi/be_main.c:116:1: warning: no previous
Baoyou> prototype for 'beiscsi_log_enable_init' [-Wmissing-prototypes]
Baoyou> drivers/scsi/be2iscsi/be_main.c:4587:5: warning: no previous
Baoyou> prototype for 'beiscsi_iotask_v2' [-Wmissing-prototypes]
Baoyou> drivers/scsi/be2iscsi/be_main.c:4976:6: warning: no previous
Baoyou> prototype for 'beiscsi_hba_attrs_init' [-Wmissing-prototypes]

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 5/7] block: Implement support for zoned block devices

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien> Thanks for all the comments. Should I send a fixed-up series or
Damien> just send correction patches later ?

Please repost with the requested changes in place.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 7/7] blk-zoned: implement ioctls

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien> Adds the new BLKREPORTZONE and BLKRESETZONE ioctls for
Damien> respectively obtaining the zone configuration of a zoned block
Damien> device and resetting the write pointer of sequential zones of a
Damien> zoned block device.

Damien> The BLKREPORTZONE ioctl maps directly to a single call of the
Damien> function blkdev_report_zones. The zone information result is
Damien> passed as an array of struct blk_zone identical to the structure
Damien> used internally for processing the REQ_OP_ZONE_REPORT operation.
Damien> The BLKRESETZONE ioctl maps to a single call of the
Damien> blkdev_reset_zones function.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 6/7] sd: Implement support for ZBC devices

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien,

The new stuff looks much cleaner. Thanks for doing that!

This hunk has an unintended side effect:

@@ -2836,14 +2896,14 @@ static int sd_revalidate_disk(struct gendisk *disk)
 * react badly if we do.
 */
if (sdkp->media_present) {
-   sd_read_capacity(sdkp, buffer);
-
if (scsi_device_supports_vpd(sdp)) {
sd_read_block_provisioning(sdkp);
sd_read_block_limits(sdkp);
sd_read_block_characteristics(sdkp);
}
 
+   sd_read_capacity(sdkp, buffer);
+
sd_read_write_protect_flag(sdkp, buffer);
sd_read_cache_type(sdkp, buffer);
sd_read_app_tag_own(sdkp, buffer);

LMBPE from READ CAPACITY(16) will not be set when calling
sd_read_block_provisioning() and thus we bail early (we catch it second
time around). You may want to split that vpd conditional to shuffle the
block_characteristics on top. Or even better: Split the capacity/block
size printing code from sd_read_capacity() into a separate function and
do:

if (sdkp->media_present) {
sd_read_capacity(sdkp, buffer);

if (sd_try_extended_inquiry(sdp)) {
sd_read_block_provisioning(sdkp);
sd_read_block_limits(sdkp);
sd_read_block_characteristics(sdkp);
}

+   sd_print_capacity(sdkp);

sd_read_write_protect_flag(sdkp, buffer);
sd_read_cache_type(sdkp, buffer);
sd_read_app_tag_own(sdkp, buffer);
sd_read_write_same(sdkp, buffer);
}

Typo:

static int sd_zbc_read_zoned_charateristics(struct scsi_disk *sdkp,
 ^^

My second comment is that I don't particularly like the notion of values
being stored and passed in units of block layer sectors in struct
scsi_disk and sd*.  As a rule of thumb I prefer SCSI stuff to be counted
in logical blocks and block layer stuff in 512b sectors. You may want to
entertain having a sectors_to_zone() wrapper to facilitate that
conversion.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 5/7] block: Implement support for zoned block devices

[Damien Le Moal]

Martin,

On 9/28/16 12:54, Martin K. Petersen wrote:
>> "Damien" == Damien Le Moal  writes:
> 
> Purely cosmetic: Looks like whitespace is a bit funky (spaces instead of
> tabs several places). Also a few typos.
> 
> + * blkdev_report_zones - Get zones information
> + * @bdev:Target block device
> + * @sector:  Sector from which to report zones
> + * @zones:  Array of zone structures where to return the zones 
> information
> + * @nr_zones:   Number of zone structures in the zone array
> + * @gfp_mask:Memory allocation flags (for bio_alloc)
> 
> Tabs and spaces: Together at last! ^
> 
> + /*
> +  * Process the report resukt: skip the header and go through the
> 
> result
> 
> + /* Check alignement (handle eventual smaller last zone) */
> 
> alignment

Thanks for all the comments. Should I send a fixed-up series or just
send correction patches later ?

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
damien.lem...@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
www.hgst.com
--
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 v3 5/7] block: Implement support for zoned block devices

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Purely cosmetic: Looks like whitespace is a bit funky (spaces instead of
tabs several places). Also a few typos.

+ * blkdev_report_zones - Get zones information
+ * @bdev:  Target block device
+ * @sector:Sector from which to report zones
+ * @zones:  Array of zone structures where to return the zones information
+ * @nr_zones:   Number of zone structures in the zone array
+ * @gfp_mask:  Memory allocation flags (for bio_alloc)

Tabs and spaces: Together at last! ^

+   /*
+* Process the report resukt: skip the header and go through the

result

+   /* Check alignement (handle eventual smaller last zone) */

alignment

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 3/7] block: update chunk_sectors in blk_stack_limits()

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien> From: Hannes Reinecke  Signed-off-by: Hannes
Damien> Reinecke  Signed-off-by: Damien Le Moal
Damien>  ---

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 4/7] block: Define zoned block device operations

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien> Define REQ_OP_ZONE_REPORT and REQ_OP_ZONE_RESET for handling
Damien> zones of host-managed and host-aware zoned block devices. With
Damien> with these two new operations, the total number of operations
Damien> defined reaches 8 and still fits with the 3 bits definition of
Damien> REQ_OP_BITS.

Nicer!

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 2/7] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

...same is true for the chunk_sectors attribute. Aside from that:

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v3 1/7] block: Add 'zoned' queue limit

[Martin K. Petersen]

> "Damien" == Damien Le Moal  writes:

Damien> The zoned attribute is also exported as a string to applications
Damien> via sysfs. BLK_ZONED_NONE shows as "none", BLK_ZONED_HA as
Damien> "host-aware" and BLK_ZONED_HM as "host-managed".

Looks good. However, you should add the zoned attribute to
Documentation/ABI/testing/sysfs-block.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: [PATCHv2] MAINTAINERS: Update open-iscsi maintainers

[Mike Christie]

On 09/26/2016 05:25 PM, Lee Duncan wrote:
> Chris Leech and I are taking over as open-iscsi maintainers.
> 
> Changes since v1:
>  * Updated URL to open-iscsi.com
>  * Removed git repository, since code in tree
> ---
>  MAINTAINERS | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 01bff8ea28d8..81384a2562e7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6448,10 +6448,10 @@ S:Maintained
>  F:   drivers/firmware/iscsi_ibft*
>  
>  ISCSI
> -M:   Mike Christie 
> +M:   Lee Duncan 
> +M:   Chris Leech 
>  L:   open-is...@googlegroups.com
> -W:   www.open-iscsi.org
> -T:   git 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnc/linux-2.6-iscsi.git
> +W:   www.open-iscsi.com
>  S:   Maintained
>  F:   drivers/scsi/*iscsi*
>  F:   include/scsi/*iscsi*
> 

After over 10 years, I will get to take a vacation where I do not have
to think about iSCSI :)

Reviewed-by: Mike Christie 
--
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 v3 0/7] ZBC / Zoned block device support

[Christoph Hellwig]

The updated series looks fine to me:

Reviewed-by: Christoph Hellwig 

Thanks a lot shaun and Demian for your hard work in the last weeks!
--
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 v3 7/7] blk-zoned: implement ioctls

[Damien Le Moal]

From: Shaun Tancheff 

Adds the new BLKREPORTZONE and BLKRESETZONE ioctls for respectively
obtaining the zone configuration of a zoned block device and resetting
the write pointer of sequential zones of a zoned block device.

The BLKREPORTZONE ioctl maps directly to a single call of the function
blkdev_report_zones. The zone information result is passed as an array
of struct blk_zone identical to the structure used internally for
processing the REQ_OP_ZONE_REPORT operation.  The BLKRESETZONE ioctl
maps to a single call of the blkdev_reset_zones function.

Signed-off-by: Shaun Tancheff 
Signed-off-by: Damien Le Moal 
---
 block/blk-zoned.c | 93 +++
 block/ioctl.c |  4 ++
 include/linux/blkdev.h| 22 ++
 include/uapi/linux/blkzoned.h | 40 +++
 include/uapi/linux/fs.h   |  4 ++
 5 files changed, 163 insertions(+)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 7ebfc86..d354bea 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -255,3 +255,96 @@ int blkdev_reset_zones(struct block_device *bdev,
return 0;
 }
 EXPORT_SYMBOL_GPL(blkdev_reset_zones);
+
+/**
+ * BLKREPORTZONE ioctl processing.
+ * Called from blkdev_ioctl.
+ */
+int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+   void __user *argp = (void __user *)arg;
+   struct request_queue *q;
+   struct blk_zone_report rep;
+   struct blk_zone *zones;
+   int ret;
+
+   if (!argp)
+   return -EINVAL;
+
+   q = bdev_get_queue(bdev);
+   if (!q)
+   return -ENXIO;
+
+   if (!blk_queue_is_zoned(q))
+   return -ENOTTY;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
+   return -EFAULT;
+
+   if (!rep.nr_zones)
+   return -EINVAL;
+
+   zones = kcalloc(rep.nr_zones, sizeof(struct blk_zone), GFP_KERNEL);
+   if (!zones)
+   return -ENOMEM;
+
+   ret = blkdev_report_zones(bdev, rep.sector,
+ zones, &rep.nr_zones,
+ GFP_KERNEL);
+   if (ret)
+   goto out;
+
+   if (copy_to_user(argp, &rep, sizeof(struct blk_zone_report))) {
+   ret = -EFAULT;
+   goto out;
+   }
+
+   if (rep.nr_zones) {
+   if (copy_to_user(argp + sizeof(struct blk_zone_report), zones,
+sizeof(struct blk_zone) * rep.nr_zones))
+   ret = -EFAULT;
+   }
+
+ out:
+   kfree(zones);
+
+   return ret;
+}
+
+/**
+ * BLKRESETZONE ioctl processing.
+ * Called from blkdev_ioctl.
+ */
+int blkdev_reset_zones_ioctl(struct block_device *bdev, fmode_t mode,
+unsigned int cmd, unsigned long arg)
+{
+   void __user *argp = (void __user *)arg;
+   struct request_queue *q;
+   struct blk_zone_range zrange;
+
+   if (!argp)
+   return -EINVAL;
+
+   q = bdev_get_queue(bdev);
+   if (!q)
+   return -ENXIO;
+
+   if (!blk_queue_is_zoned(q))
+   return -ENOTTY;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   if (!(mode & FMODE_WRITE))
+   return -EBADF;
+
+   if (copy_from_user(&zrange, argp, sizeof(struct blk_zone_range)))
+   return -EFAULT;
+
+   return blkdev_reset_zones(bdev, zrange.sector, zrange.nr_sectors,
+ GFP_KERNEL);
+}
diff --git a/block/ioctl.c b/block/ioctl.c
index ed2397f..448f78a 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -513,6 +513,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, 
unsigned cmd,
BLKDEV_DISCARD_SECURE);
case BLKZEROOUT:
return blk_ioctl_zeroout(bdev, mode, arg);
+   case BLKREPORTZONE:
+   return blkdev_report_zones_ioctl(bdev, mode, cmd, arg);
+   case BLKRESETZONE:
+   return blkdev_reset_zones_ioctl(bdev, mode, cmd, arg);
case HDIO_GETGEO:
return blkdev_getgeo(bdev, argp);
case BLKRAGET:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6316972..0a75285 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -315,6 +315,28 @@ extern int blkdev_report_zones(struct block_device *,
unsigned int *, gfp_t);
 extern int blkdev_reset_zones(struct block_device *, sector_t,
sector_t, gfp_t);
+
+extern int blkdev_report_zones_ioctl(struct block_device *, fmode_t,
+unsigned int, unsigned long);
+extern int blkdev_reset_zones_ioctl(struct block_device *, fmode_t,
+   unsigned int, unsigned lon

[PATCH v3 6/7] sd: Implement support for ZBC devices

[Damien Le Moal]

From: Hannes Reinecke 

Implement ZBC support functions to setup zoned disks, both
host-managed and host-aware models. Only zoned disks that satisfy
the following conditions are supported:
1) All zones are the same size, with the exception of an eventual
   last smaller runt zone.
2) For host-managed disks, reads are unrestricted (reads are not
   failed due to zone or write pointer alignement constraints).
Zoned disks that do not satisfy these 2 conditions will be ignored.

The capacity read of the device triggers the zoned block device
checks. As this needs the zone model of the disk, the call to
sd_read_capacity is moved after the call to
sd_read_block_characteristics so that host-aware devices are
properlly detected and initialized. The call to sd_zbc_read_zones
in sd_read_capacity may change the device capacity obtained with
the sd_read_capacity_16 function for devices reporting only the
capacity of conventional zones at the beginning of the LBA range
(i.e. devices with rc_basis set to 0).

Signed-off-by: Hannes Reinecke 

[Damien: * Removed zone cache support
 * Removed mapping of discard to reset write pointer command
 * Modified sd_zbc_read_zones to include checks that the
   device satisfies the kernel constraints
 * Implemeted REPORT ZONES setup and post-processing based
   on code from Shaun Tancheff ]
Signed-off-by: Damien Le Moal 
---
 drivers/scsi/Makefile |   1 +
 drivers/scsi/sd.c |  97 ++--
 drivers/scsi/sd.h |  67 ++
 drivers/scsi/sd_zbc.c | 586 ++
 include/scsi/scsi_proto.h |  17 ++
 5 files changed, 754 insertions(+), 14 deletions(-)
 create mode 100644 drivers/scsi/sd_zbc.c

diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index fc0d9b8..350513c 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -180,6 +180,7 @@ hv_storvsc-y:= storvsc_drv.o
 
 sd_mod-objs:= sd.o
 sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o
+sd_mod-$(CONFIG_BLK_DEV_ZONED) += sd_zbc.o
 
 sr_mod-objs:= sr.o sr_ioctl.o sr_vendor.o
 ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 51e5629..4b3523b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -93,6 +93,7 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
+MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC);
 
 #if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
 #define SD_MINORS  16
@@ -163,7 +164,7 @@ cache_type_store(struct device *dev, struct 
device_attribute *attr,
static const char temp[] = "temporary ";
int len;
 
-   if (sdp->type != TYPE_DISK)
+   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
/* no cache control on RBC devices; theoretically they
 * can do it, but there's probably so many exceptions
 * it's not worth the risk */
@@ -262,7 +263,7 @@ allow_restart_store(struct device *dev, struct 
device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
 
-   if (sdp->type != TYPE_DISK)
+   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;
 
sdp->allow_restart = simple_strtoul(buf, NULL, 10);
@@ -392,6 +393,11 @@ provisioning_mode_store(struct device *dev, struct 
device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
 
+   if (sd_is_zoned(sdkp)) {
+   sd_config_discard(sdkp, SD_LBP_DISABLE);
+   return count;
+   }
+
if (sdp->type != TYPE_DISK)
return -EINVAL;
 
@@ -459,7 +465,7 @@ max_write_same_blocks_store(struct device *dev, struct 
device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
 
-   if (sdp->type != TYPE_DISK)
+   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;
 
err = kstrtoul(buf, 10, &max);
@@ -844,6 +850,13 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
 
BUG_ON(bio_offset(bio) || bio_iovec(bio).bv_len != sdp->sector_size);
 
+   if (sd_is_zoned(sdkp)) {
+   /* sd_zbc_setup_read_write uses block layer sector units */
+   ret = sd_zbc_setup_read_write(sdkp, rq, sector, nr_sectors);
+   if (ret != BLKPREP_OK)
+   return ret;
+   }
+
sector >>= ilog2(sdp->sector_size) - 9;
nr_sectors >>= ilog2(sdp->sector_size) - 9;
 
@@ -963,6 +976,13 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd 
*SCpnt)
SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt, "block=%llu\n",
(unsigned long long)block));
 
+   if (sd_is_zoned(sdkp)) {
+   /* sd_zbc_setup_read_write uses block layer sector units */
+   ret = sd_zbc_setup_read

[PATCH v3 4/7] block: Define zoned block device operations

[Damien Le Moal]

From: Shaun Tancheff 

Define REQ_OP_ZONE_REPORT and REQ_OP_ZONE_RESET for handling zones of
host-managed and host-aware zoned block devices. With with these two
new operations, the total number of operations defined reaches 8 and
still fits with the 3 bits definition of REQ_OP_BITS.

Signed-off-by: Shaun Tancheff 
Signed-off-by: Damien Le Moal 
---
 block/blk-core.c  | 4 
 include/linux/blk_types.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 14d7c07..e4eda5d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1941,6 +1941,10 @@ generic_make_request_checks(struct bio *bio)
case REQ_OP_WRITE_SAME:
if (!bdev_write_same(bio->bi_bdev))
goto not_supported;
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   if (!bdev_is_zoned(bio->bi_bdev))
+   goto not_supported;
break;
default:
break;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index cd395ec..dd50dce 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -243,6 +243,8 @@ enum req_op {
REQ_OP_SECURE_ERASE,/* request to securely erase sectors */
REQ_OP_WRITE_SAME,  /* write same block many times */
REQ_OP_FLUSH,   /* request for cache flush */
+   REQ_OP_ZONE_REPORT, /* Get zone information */
+   REQ_OP_ZONE_RESET,  /* Reset a zone write pointer */
 };
 
 #define REQ_OP_BITS 3
-- 
2.7.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 v3 0/7] ZBC / Zoned block device support

[Damien Le Moal]

This series introduces support for zoned block devices. It integrates
earlier submissions by Hannes Reinecke and Shaun Tancheff. Compared to the
previous series version, the code was significantly simplified by limiting
support to zoned devices satisfying the following conditions:
1) All zones of the device are the same size, with the exception of an
   eventual last smaller runt zone.
2) For host-managed disks, reads must be unrestricted (read commands do not
   fail due to zone or write pointer alignement constraints).
Zoned disks that do not satisfy these 2 conditions are ignored.

These 2 conditions allowed dropping the zone information cache implemented
in the previous version. This simplifies the code and also reduces the memory
consumption at run time. Support for zoned devices now only require one bit
per zone (less than 8KB in total). This bit field is used to write-lock
zones and prevent the concurrent execution of multiple write commands in
the same zone. This avoids write ordering problems at dispatch time, for
both the simple queue and scsi-mq settings.

The new operations introduced to suport zone manipulation was reduced to
only the two main ZBC/ZAC defined commands: REPORT ZONES (REQ_OP_ZONE_REPORT)
and RESET WRITE POINTER (REQ_OP_ZONE_RESET). This brings the total number of
operations defined to 8, which fits in the 3 bits (REQ_OP_BITS) reserved for
operation code in bio->bi_opf and req->cmd_flags.

Most of the ZBC specific code is kept out of sd.c and implemented in the
new file sd_zbc.c. Similarly, at the block layer, most of the zoned block
device code is implemented in the new blk-zoned.c.

For host-managed zoned block devices, the sequential write constraint of
write pointer zones is exposed to the user. Users of the disk (applications,
file systems or device mappers) must sequentially write to zones. This means
that for raw block device accesses from applications, buffered writes are
unreliable and direct I/Os must be used (or buffered writes with O_SYNC).

Access to zone manipulation operations is also provided to applications
through a set of new ioctls. This allows applications operating on raw
block devices (e.g. mkfs.xxx) to discover a device zone layout and
manipulate zone state.

Changes from v2:
* Use kcalloc to allocate zone information array for ioctl
* Use kcalloc to allocate zone information array for ioctl
* Export GPL the functions blkdev_report_zones and blkdev_reset_zones
* Shuffled uapi definitions from patch 7 into patch 5

Damien Le Moal (1):
  block: Add 'zoned' queue limit

Hannes Reinecke (4):
  blk-sysfs: Add 'chunk_sectors' to sysfs attributes
  block: update chunk_sectors in blk_stack_limits()
  block: Implement support for zoned block devices
  sd: Implement support for ZBC devices

Shaun Tancheff (2):
  block: Define zoned block device operations
  blk-zoned: implement ioctls

 block/Kconfig |   8 +
 block/Makefile|   1 +
 block/blk-core.c  |   4 +
 block/blk-settings.c  |   5 +
 block/blk-sysfs.c |  29 +++
 block/blk-zoned.c | 350 +
 block/ioctl.c |   4 +
 drivers/scsi/Makefile |   1 +
 drivers/scsi/sd.c |  97 ++-
 drivers/scsi/sd.h |  67 +
 drivers/scsi/sd_zbc.c | 586 ++
 include/linux/blk_types.h |   2 +
 include/linux/blkdev.h|  99 +++
 include/scsi/scsi_proto.h |  17 ++
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/blkzoned.h | 143 +++
 include/uapi/linux/fs.h   |   4 +
 17 files changed, 1404 insertions(+), 14 deletions(-)
 create mode 100644 block/blk-zoned.c
 create mode 100644 drivers/scsi/sd_zbc.c
 create mode 100644 include/uapi/linux/blkzoned.h

-- 
2.7.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 v3 1/7] block: Add 'zoned' queue limit

[Damien Le Moal]

Add the zoned queue limit to indicate the zoning model of a block device.
Defined values are 0 (BLK_ZONED_NONE) for regular block devices,
1 (BLK_ZONED_HA) for host-aware zone block devices and 2 (BLK_ZONED_HM)
for host-managed zone block devices. The standards defined drive managed
model is not defined here since these block devices do not provide any
command for accessing zone information. Drive managed model devices will
be reported as BLK_ZONED_NONE.

The helper functions blk_queue_zoned_model and bdev_zoned_model return
the zoned limit and the functions blk_queue_is_zoned and bdev_is_zoned
return a boolean for callers to test if a block device is zoned.

The zoned attribute is also exported as a string to applications via
sysfs. BLK_ZONED_NONE shows as "none", BLK_ZONED_HA as "host-aware" and
BLK_ZONED_HM as "host-managed".

Signed-off-by: Damien Le Moal 
---
 block/blk-settings.c   |  1 +
 block/blk-sysfs.c  | 18 ++
 include/linux/blkdev.h | 47 +++
 3 files changed, 66 insertions(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index f679ae1..b1d5b7f 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -107,6 +107,7 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->io_opt = 0;
lim->misaligned = 0;
lim->cluster = 1;
+   lim->zoned = BLK_ZONED_NONE;
 }
 EXPORT_SYMBOL(blk_set_default_limits);
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 9cc8d7c..ff9cd9c 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -257,6 +257,18 @@ QUEUE_SYSFS_BIT_FNS(random, ADD_RANDOM, 0);
 QUEUE_SYSFS_BIT_FNS(iostats, IO_STAT, 0);
 #undef QUEUE_SYSFS_BIT_FNS
 
+static ssize_t queue_zoned_show(struct request_queue *q, char *page)
+{
+   switch (blk_queue_zoned_model(q)) {
+   case BLK_ZONED_HA:
+   return sprintf(page, "host-aware\n");
+   case BLK_ZONED_HM:
+   return sprintf(page, "host-managed\n");
+   default:
+   return sprintf(page, "none\n");
+   }
+}
+
 static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
 {
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -485,6 +497,11 @@ static struct queue_sysfs_entry queue_nonrot_entry = {
.store = queue_store_nonrot,
 };
 
+static struct queue_sysfs_entry queue_zoned_entry = {
+   .attr = {.name = "zoned", .mode = S_IRUGO },
+   .show = queue_zoned_show,
+};
+
 static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = S_IRUGO | S_IWUSR },
.show = queue_nomerges_show,
@@ -546,6 +563,7 @@ static struct attribute *default_attrs[] = {
&queue_discard_zeroes_data_entry.attr,
&queue_write_same_max_entry.attr,
&queue_nonrot_entry.attr,
+   &queue_zoned_entry.attr,
&queue_nomerges_entry.attr,
&queue_rq_affinity_entry.attr,
&queue_iostats_entry.attr,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c47c358..f19e16b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -261,6 +261,15 @@ struct blk_queue_tag {
 #define BLK_SCSI_MAX_CMDS  (256)
 #define BLK_SCSI_CMD_PER_LONG  (BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
 
+/*
+ * Zoned block device models (zoned limit).
+ */
+enum blk_zoned_model {
+   BLK_ZONED_NONE, /* Regular block device */
+   BLK_ZONED_HA,   /* Host-aware zoned block device */
+   BLK_ZONED_HM,   /* Host-managed zoned block device */
+};
+
 struct queue_limits {
unsigned long   bounce_pfn;
unsigned long   seg_boundary_mask;
@@ -290,6 +299,7 @@ struct queue_limits {
unsigned char   cluster;
unsigned char   discard_zeroes_data;
unsigned char   raid_partial_stripes_expensive;
+   enum blk_zoned_modelzoned;
 };
 
 struct request_queue {
@@ -627,6 +637,23 @@ static inline unsigned int blk_queue_cluster(struct 
request_queue *q)
return q->limits.cluster;
 }
 
+static inline enum blk_zoned_model
+blk_queue_zoned_model(struct request_queue *q)
+{
+   return q->limits.zoned;
+}
+
+static inline bool blk_queue_is_zoned(struct request_queue *q)
+{
+   switch (blk_queue_zoned_model(q)) {
+   case BLK_ZONED_HA:
+   case BLK_ZONED_HM:
+   return true;
+   default:
+   return false;
+   }
+}
+
 /*
  * We regard a request as sync, if either a read or a sync write
  */
@@ -1354,6 +1381,26 @@ static inline unsigned int bdev_write_same(struct 
block_device *bdev)
return 0;
 }
 
+static inline enum blk_zoned_model bdev_zoned_model(struct block_device *bdev)
+{
+   struct request_queue *q = bdev_get_queue(bdev);
+
+   if (q)
+   return blk_queue_zoned_model(q);
+
+   return BLK_ZONED_NONE;
+}
+
+static inline bool bdev_is_zoned(struct block_device *bdev)
+{
+   struct request_queue *q = bdev_get_queue(bdev);
+
+   if 

[PATCH v3 5/7] block: Implement support for zoned block devices

[Damien Le Moal]

From: Hannes Reinecke 

Implement zoned block device zone information reporting and reset.
Zone information are reported as struct blk_zone. This implementation
does not differentiate between host-aware and host-managed device
models and is valid for both. Two functions are provided:
blkdev_report_zones for discovering the zone configuration of a
zoned block device, and blkdev_reset_zones for resetting the write
pointer of sequential zones. The helper function blk_queue_zone_size
and bdev_zone_size are also provided for, as the name suggest,
obtaining the zone size (in 512B sectors) of the zones of the device.

Signed-off-by: Hannes Reinecke 

[Damien: * Removed the zone cache
 * Implement report zones operation based on earlier proposal
   by Shaun Tancheff ]
Signed-off-by: Damien Le Moal 
---
 block/Kconfig |   8 ++
 block/Makefile|   1 +
 block/blk-zoned.c | 257 ++
 include/linux/blkdev.h|  30 +
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/blkzoned.h | 103 +
 6 files changed, 400 insertions(+)
 create mode 100644 block/blk-zoned.c
 create mode 100644 include/uapi/linux/blkzoned.h

diff --git a/block/Kconfig b/block/Kconfig
index 1d4d624..6b0ad08 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -89,6 +89,14 @@ config BLK_DEV_INTEGRITY
T10/SCSI Data Integrity Field or the T13/ATA External Path
Protection.  If in doubt, say N.
 
+config BLK_DEV_ZONED
+   bool "Zoned block device support"
+   ---help---
+   Block layer zoned block device support. This option enables
+   support for ZAC/ZBC host-managed and host-aware zoned block devices.
+
+   Say yes here if you have a ZAC or ZBC storage device.
+
 config BLK_DEV_THROTTLING
bool "Block layer bio throttling support"
depends on BLK_CGROUP=y
diff --git a/block/Makefile b/block/Makefile
index 36acdd7..9371bc7 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -22,4 +22,5 @@ obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o
 obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o
 obj-$(CONFIG_BLK_CMDLINE_PARSER)   += cmdline-parser.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
+obj-$(CONFIG_BLK_DEV_ZONED)+= blk-zoned.o
 obj-$(CONFIG_BLK_MQ_PCI)   += blk-mq-pci.o
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
new file mode 100644
index 000..7ebfc86
--- /dev/null
+++ b/block/blk-zoned.c
@@ -0,0 +1,257 @@
+/*
+ * Zoned block device handling
+ *
+ * Copyright (c) 2015, Hannes Reinecke
+ * Copyright (c) 2015, SUSE Linux GmbH
+ *
+ * Copyright (c) 2016, Damien Le Moal
+ * Copyright (c) 2016, Western Digital
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static inline sector_t blk_zone_start(struct request_queue *q,
+ sector_t sector)
+{
+   sector_t zone_mask = blk_queue_zone_size(q) - 1;
+
+   return sector & ~zone_mask;
+}
+
+/*
+ * Check that a zone report belongs to the partition.
+ * If yes, fix its start sector and write pointer, copy it in the
+ * zone information array and return true. Return false otherwise.
+ */
+static bool blkdev_report_zone(struct block_device *bdev,
+  struct blk_zone *rep,
+  struct blk_zone *zone)
+{
+   sector_t offset = get_start_sect(bdev);
+
+   if (rep->start < offset)
+   return false;
+
+   rep->start -= offset;
+   if (rep->start + rep->len > bdev->bd_part->nr_sects)
+   return false;
+
+   if (rep->type == BLK_ZONE_TYPE_CONVENTIONAL)
+   rep->wp = rep->start + rep->len;
+   else
+   rep->wp -= offset;
+   memcpy(zone, rep, sizeof(struct blk_zone));
+
+   return true;
+}
+
+/**
+ * blkdev_report_zones - Get zones information
+ * @bdev:  Target block device
+ * @sector:Sector from which to report zones
+ * @zones:  Array of zone structures where to return the zones information
+ * @nr_zones:   Number of zone structures in the zone array
+ * @gfp_mask:  Memory allocation flags (for bio_alloc)
+ *
+ * Description:
+ *Get zone information starting from the zone containing @sector.
+ *The number of zone information reported may be less than the number
+ *requested by @nr_zones. The number of zones actually reported is
+ *returned in @nr_zones.
+ */
+int blkdev_report_zones(struct block_device *bdev,
+   sector_t sector,
+   struct blk_zone *zones,
+   unsigned int *nr_zones,
+   gfp_t gfp_mask)
+{
+   struct request_queue *q = bdev_get_queue(bdev);
+   struct blk_zone_report_hdr *hdr;
+   unsigned int nrz = *nr_zones;
+   struct page *page;
+   unsigned int nr_rep;
+   size_t rep_bytes;
+   unsigned int nr_pages;
+   struct bio *bio;
+   struct bio_vec *bv;
+   unsign

[PATCH v3 2/7] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Damien Le Moal]

From: Hannes Reinecke 

The queue limits already have a 'chunk_sectors' setting, so
we should be presenting it via sysfs.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Damien Le Moal 
---
 block/blk-sysfs.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index ff9cd9c..488c2e2 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -130,6 +130,11 @@ static ssize_t queue_physical_block_size_show(struct 
request_queue *q, char *pag
return queue_var_show(queue_physical_block_size(q), page);
 }
 
+static ssize_t queue_chunk_sectors_show(struct request_queue *q, char *page)
+{
+   return queue_var_show(q->limits.chunk_sectors, page);
+}
+
 static ssize_t queue_io_min_show(struct request_queue *q, char *page)
 {
return queue_var_show(queue_io_min(q), page);
@@ -455,6 +460,11 @@ static struct queue_sysfs_entry 
queue_physical_block_size_entry = {
.show = queue_physical_block_size_show,
 };
 
+static struct queue_sysfs_entry queue_chunk_sectors_entry = {
+   .attr = {.name = "chunk_sectors", .mode = S_IRUGO },
+   .show = queue_chunk_sectors_show,
+};
+
 static struct queue_sysfs_entry queue_io_min_entry = {
.attr = {.name = "minimum_io_size", .mode = S_IRUGO },
.show = queue_io_min_show,
@@ -555,6 +565,7 @@ static struct attribute *default_attrs[] = {
&queue_hw_sector_size_entry.attr,
&queue_logical_block_size_entry.attr,
&queue_physical_block_size_entry.attr,
+   &queue_chunk_sectors_entry.attr,
&queue_io_min_entry.attr,
&queue_io_opt_entry.attr,
&queue_discard_granularity_entry.attr,
-- 
2.7.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 v3 3/7] block: update chunk_sectors in blk_stack_limits()

[Damien Le Moal]

From: Hannes Reinecke 

Signed-off-by: Hannes Reinecke 
Signed-off-by: Damien Le Moal 
---
 block/blk-settings.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index b1d5b7f..55369a6 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -631,6 +631,10 @@ int blk_stack_limits(struct queue_limits *t, struct 
queue_limits *b,
t->discard_granularity;
}
 
+   if (b->chunk_sectors)
+   t->chunk_sectors = min_not_zero(t->chunk_sectors,
+   b->chunk_sectors);
+
return ret;
 }
 EXPORT_SYMBOL(blk_stack_limits);
-- 
2.7.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

Re: [PATCH 4/3] g_NCR5380: Merge g_NCR5380 and g_NCR5380_mmio

[Finn Thain]

On Sun, 25 Sep 2016, Ondrej Zary wrote:

> Merge the PIO and MMIO code (with the help of ioport_map) in g_NCR5380 and
> delete g_NCR5380_mmio.
> 
> Signed-off-by: Ondrej Zary 
> ---
>  drivers/scsi/Kconfig  |   32 ++---
>  drivers/scsi/g_NCR5380.c  |  257 
> +++--
>  drivers/scsi/g_NCR5380.h  |   33 ++
>  drivers/scsi/g_NCR5380_mmio.c |   10 --
>  4 files changed, 135 insertions(+), 197 deletions(-)
>  delete mode 100644 drivers/scsi/g_NCR5380_mmio.c
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 7d1b431..fd1bca1 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -780,40 +780,22 @@ config SCSI_ISCI
> control unit found in the Intel(R) C600 series chipset.
>  
>  config SCSI_GENERIC_NCR5380
> - tristate "Generic NCR5380/53c400 SCSI PIO support"
> + tristate "Generic NCR5380/53c400 SCSI"
>   depends on ISA && SCSI
>   select SCSI_SPI_ATTRS
>   ---help---
> -   This is a driver for the old NCR 53c80 series of SCSI controllers
> -   on boards using PIO. Most boards such as the Trantor T130 fit this
> -   category, along with a large number of ISA 8bit controllers shipped
> -   for free with SCSI scanners. If you have a PAS16, T128 or DMX3191
> -   you should select the specific driver for that card rather than
> -   generic 5380 support.
> +   This is a driver for the old NCR 53c80 series of SCSI controllers.
> +   Most boards such as the Trantor T130 fit this category, along with
> +   a large number of ISA 8bit controllers shipped for free with SCSI
> +   scanners. If you have a PAS16, T128 or DMX3191 you should select the
> +   specific driver for that card rather than generic 5380 support.

The pas16 and t128 drivers were removed (see 4.9/scsi-queue). The DMX3191 
isn't an ISA card, so it isn't relevant.

>  
> It is explained in section 3.8 of the SCSI-HOWTO, available from
> -   .  If it doesn't work out
> -   of the box, you may have to change some settings in
> -   .
> +   .

I didn't find anything useful at that url.

Therefore, please change the help text as follows:

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 98e5d51..928d937 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -780,21 +780,14 @@ config SCSI_ISCI
  control unit found in the Intel(R) C600 series chipset.
 
 config SCSI_GENERIC_NCR5380
-   tristate "Generic NCR5380/53c400 SCSI PIO support"
+   tristate "Generic NCR5380/53C400 ISA card SCSI controller support"
depends on ISA && SCSI
select SCSI_SPI_ATTRS
---help---
- This is a driver for the old NCR 53c80 series of SCSI controllers
- on boards using PIO. Most boards such as the Trantor T130 fit this
- category, along with a large number of ISA 8bit controllers shipped
- for free with SCSI scanners. If you have a PAS16, T128 or DMX3191
- you should select the specific driver for that card rather than
- generic 5380 support.
-
- It is explained in section 3.8 of the SCSI-HOWTO, available from
- .  If it doesn't work out
- of the box, you may have to change some settings in
- .
+ This is a driver for old ISA card SCSI controllers based on the
+ NCR 5380, 53C80, 53C400 and 53C400A, and compatible DTC devices.
+ Most boards such as the Trantor T130 fit this category, along
+ with various 8-bit and 16-bit ISA cards bundled with SCSI scanners.
 
  To compile this driver as a module, choose M here: the
  module will be called g_NCR5380.


The rest of the patch looks good (aside from the missing module alias that 
Chrisoph mentioned).

Thanks.

-- 
--
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 3/3] g_NCR5380: Stop using scsi_module.c

[Finn Thain]

On Tue, 27 Sep 2016, Ondrej Zary wrote:

> On Monday 26 September 2016, Finn Thain wrote:
> 
> [...]
> 
> > > @@ -364,7 +304,7 @@ static int generic_NCR5380_release_resources(struct
> > > Scsi_Host *instance) release_mem_region(instance->base,
> > > hostdata->iomem_size);
> > >   }
> > >  #endif
> > > - return 0;
> > > + scsi_host_put(instance);
> >
> > The sequence of operations here should be the same as the error path
> > above.
> 
> I put scsi_host_put() call at the end because the release_mem_region 
> code (in the MMIO case) dereferences the hostdata pointer. I don't think 
> it's safe to do after scsi_host_put().

It's funny you say that, I already fixed a few of those use-after-free 
bugs in the other 5380 drivers. To avoid the bug, you'd need to use a 
temporary variable, like the fixes I did elsewhere.

Don't worry about changing it, this part of the driver gets revisited in 
my existing patch queue anyway.

> 
> [...]
> 
> > > +static int pnp_registered;
> > > +#endif /* !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP) */
> > > +
> > > +static int __init generic_NCR5380_init(void)
> > > +{
> > > + int ret = 0;
> > > +
> > > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > > + ret = pnp_register_driver(&generic_NCR5380_pnp_driver);
> > > + if (!ret)
> > > + pnp_registered = 1;
> > >  #endif
> > > + ret = isa_register_driver(&generic_NCR5380_isa_driver, MAX_CARDS);
> > > + if (!ret)
> > > + isa_registered = 1;
> > > +
> > > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > > + if (pnp_registered)
> > > + ret = 0;
> > > +#endif
> > > + if (isa_registered)
> > > + ret = 0;
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void __exit generic_NCR5380_exit(void)
> > > +{
> > > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > > + if (pnp_registered)
> > > + pnp_unregister_driver(&generic_NCR5380_pnp_driver);
> > > +#endif
> > > + if (isa_registered)
> > > + isa_unregister_driver(&generic_NCR5380_isa_driver);
> > > +}
> > > +
> >
> > I find that hard to follow. This should be equivalent and simpler:
> >
> > static int __init generic_NCR5380_init(void)
> > {
> > isa_ret = isa_register_driver(&generic_NCR5380_isa_driver, MAX_CARDS);
> > #if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > pnp_ret = pnp_register_driver(&generic_NCR5380_pnp_driver);
> > return pnp_ret ? isa_ret : 0;
> > #endif
> > return isa_ret;
> > }
> >
> > static void __exit generic_NCR5380_exit(void)
> > {
> > if (!isa_ret)
> > isa_unregister_driver(&generic_NCR5380_isa_driver);
> > #if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > if (!pnp_ret)
> > pnp_unregister_driver(&generic_NCR5380_pnp_driver);
> > #endif
> > }
> 
> Doesn't make it any better, IMHO. Good that it's shorter but not cleaner - 
> especially this:
> > return pnp_ret ? isa_ret : 0;

The problem with that line is that pnp_ret is always discarded, same as in 
your version. I think my version makes that clear, which is what matters 
to me.

> 
> Also looking at the _exit function, meaning of isa_ret and pnp_ret is not 
> obvious there.
> 
> Maybe we should have a module_isa_pnp_driver() macro.
> 
> 

FWIW I would hope that the hypothetical definition of this 
`module_isa_pnp_driver' macro would have the same properties as my 
version: shorter, uses no temporaries and uses one less #ifdef, and is 
generally easier for me to read.

-- 
--
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: UFS API in the kernel

[subhashj]

Hi Joao,


On 2016-09-26 18:10, Kiwoong Kim wrote:

Hi.

If you want to declare some things for user interface,
is it be better to put those thing include/uapi/linux/ than 
include/linux?


Agreed with Mr. Pinto's opinion with respect to implementing additional 
ioctls.


Yes, "scsi_host_template" allows the LLD's to export their ioctl 
callback and then you can use the sg interface to issue UFS specific 
ioctls. We had implemented similar ioctl for our use, here is the 
reference code: 
https://source.codeaurora.org/quic/la/kernel/msm-3.18/tree/drivers/scsi/ufs/ufshcd.c?h=LA.HB.1.1.1.c2#n6791. 
This was mainly done to  export the UFS query request interface to user 
space. Declarations where exported under include/uapi/scsi/ufs/ . If you 
want to build upon this already existing functionality, i can post the 
formal patch on mailing list.


Regards,
Subhash




Regards.


-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of Shaun Tancheff
Sent: Tuesday, September 27, 2016 4:23 AM
To: Joao Pinto
Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: Re: UFS API in the kernel

On Thu, Sep 22, 2016 at 10:21 AM, Joao Pinto 
wrote:
> Hi!
>
> I am designing an application that has the goal to be an utility for
> Unipro and UFS testing purposes. This application is going to run on
> top of a recent Linux Kernel containing the new UFS stack (including the
new DWC drivers).
>
> I am considering doing the following:
> a) Create a new config item called CONFIG_UFS_CHARDEV which is going
> to create a char device responsible to make some IOCTL available for
> user-space applications
> b) Create a linux/ufs.h header file that contains data structures
> declarations that will be needed in user-space applications

I am not very familiar with UFS devices, that said you should have an 
sgX

chardev being created already so you can handle SG_IO requests.
There also appear to be some sysfs entries being created.

So between sg and sysfs you should be able to handle any user-space 
out of

band requests without resorting to making a new chardev.

Adding more sysfs entries, if you need them, should be fine.

You may find it easier to expand on the existing interfaces than to 
get

consensus on a new driver and ioctls.

Hope this helps,
Shaun

> Could you please advise me about what the correct approach should be
> to make it as standard as possible and usable in the future?
>
> Thank you very much for your help!
>
> regards,
> Joao
> --
> 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
> https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_ma
> jordomo-2Dinfo.html&d=DQICaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ug
> l8V50qIHLe856QW0qfG3WVYGOrWzA&m=vJFB6pCywWtdvkgHz9Vc0jQz0xzeyZlr-7eCWY
> u88nM&s=yiQLPFpqmMrbqLZz1Jb3aNqOje2dRMLJHEzUDobwcXc&e=



--
Shaun Tancheff
--
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



--
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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][SCSI] scsi: ufs: get a TM service response from the correct offset

[subhashj]

Hi Kiwoong Kim,

This is a good catch, Looks good to me.
Reviewed-by: Subhash Jadavani 

Thanks,
Subhash

On 2016-09-08 16:22, Kiwoong Kim wrote:

From: Kiwoong Kim 

When any UFS host controller receives a TM(Task Management)
response from a UFS device,
UFS driver has been recognize like receiving a message of
"Task Management Function Complete"(00h) in all cases, so far.
That means there is no pending task for a tag of the TM request
sent before in the UFS device.
That's because the byte offset 6 in TM response which has been used
to get a TM service response so far
represents just whether or not a TM transmission passes.

Regarding UFS spec, the correct byte offset to
get TM service response is 15, not 6.

I tested that UFS driver responds properly for the TM response
From a UFS device with an reference board with exynos8890, as follow:
No pending task -> Task Management Function Complete (00h)
Pending task -> Task Management Function Succeeded (08h)

Signed-off-by: Kiwoong Kim 
Signed-off-by: HeonGwang Chu 
Tested-by: : Kiwoong Kim 
---
 drivers/scsi/ufs/ufs.h|1 +
 drivers/scsi/ufs/ufshcd.c |4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 42c459a..89c121e 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -295,6 +295,7 @@ enum {
MASK_QUERY_DATA_SEG_LEN = 0x,
MASK_RSP_UPIU_DATA_SEG_LEN  = 0x,
MASK_RSP_EXCEPTION_EVENT= 0x1,
+   MASK_TM_SERVICE_RESP= 0xFF,
 };

 /* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e8a706b..c641cd3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3013,8 +3013,8 @@ static int ufshcd_task_req_compl(struct ufs_hba
*hba, u32 index, u8 *resp)
if (ocs_value == OCS_SUCCESS) {
task_rsp_upiup = (struct utp_upiu_task_rsp *)
task_req_descp[index].task_rsp_upiu;
-   task_result = 
be32_to_cpu(task_rsp_upiup->header.dword_1);
-   task_result = ((task_result & MASK_TASK_RESPONSE) >> 
8);
+   task_result = 
be32_to_cpu(task_rsp_upiup->output_param1);

+   task_result = task_result & MASK_TM_SERVICE_RESP;
if (resp)
*resp = (u8)task_result;
} else {
--
1.7.9.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

--
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 v2] UFS: Date Segment only need for WRITE DESCRIPTOR

[subhashj]

Looks good to me.
Reviewed-by: Subhash Jadavani 

On 2016-08-25 02:39, Zang Leigang wrote:
Some device may cause a compatibility issue while receiving a Query 
UPIU

with Data Segment which does not expected.

Signed-off-by: Zang Leigang 
---
 drivers/scsi/ufs/ufshcd.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f08d41a..9b21d88 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1266,9 +1266,12 @@ static void
ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
ucd_req_ptr->header.dword_1 = UPIU_HEADER_DWORD(
0, query->request.query_func, 0, 0);

-   /* Data segment length */
-   ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD(
-   0, 0, len >> 8, (u8)len);
+   /* Data segment length only need for WRITE_DESC */
+   if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
+   ucd_req_ptr->header.dword_2 =
+   UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len);
+   else
+   ucd_req_ptr->header.dword_2 = 0;

/* Copy the Query Request buffer as is */
memcpy(&ucd_req_ptr->qr, &query->request.upiu_req,

--
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 v3 7/7] blk-zoned: implement ioctls

[Christoph Hellwig]

Damien, can you do a repost of the whole series with the
header reshuffled included?  Except for that and the trivial
kcalloc bit the series looks fine to me:

Reviewed-by: Christoph Hellwig 
--
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 v2 3/7] block: update chunk_sectors in blk_stack_limits()

[Shaun Tancheff]

On Mon, Sep 26, 2016 at 6:14 AM, Damien Le Moal  wrote:
> From: Hannes Reinecke 
>
> Signed-off-by: Hannes Reinecke 
> Signed-off-by: Damien Le Moal 
> ---
>  block/blk-settings.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index b1d5b7f..55369a6 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -631,6 +631,10 @@ int blk_stack_limits(struct queue_limits *t, struct 
> queue_limits *b,
> t->discard_granularity;
> }
>
> +   if (b->chunk_sectors)
> +   t->chunk_sectors = min_not_zero(t->chunk_sectors,
> +   b->chunk_sectors);
> +
> return ret;
>  }
>  EXPORT_SYMBOL(blk_stack_limits);
> --
> 2.7.4

Reviewed-by: Shaun Tancheff 
Tested-by: Shaun Tancheff 


> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Shaun Tancheff
--
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 v2 2/7] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Shaun Tancheff]

On Mon, Sep 26, 2016 at 6:14 AM, Damien Le Moal  wrote:
> From: Hannes Reinecke 
>
> The queue limits already have a 'chunk_sectors' setting, so
> we should be presenting it via sysfs.
>
> Signed-off-by: Hannes Reinecke 
> Signed-off-by: Damien Le Moal 
> ---
>  block/blk-sysfs.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index ff9cd9c..488c2e2 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -130,6 +130,11 @@ static ssize_t queue_physical_block_size_show(struct 
> request_queue *q, char *pag
> return queue_var_show(queue_physical_block_size(q), page);
>  }
>
> +static ssize_t queue_chunk_sectors_show(struct request_queue *q, char *page)
> +{
> +   return queue_var_show(q->limits.chunk_sectors, page);
> +}
> +
>  static ssize_t queue_io_min_show(struct request_queue *q, char *page)
>  {
> return queue_var_show(queue_io_min(q), page);
> @@ -455,6 +460,11 @@ static struct queue_sysfs_entry 
> queue_physical_block_size_entry = {
> .show = queue_physical_block_size_show,
>  };
>
> +static struct queue_sysfs_entry queue_chunk_sectors_entry = {
> +   .attr = {.name = "chunk_sectors", .mode = S_IRUGO },
> +   .show = queue_chunk_sectors_show,
> +};
> +
>  static struct queue_sysfs_entry queue_io_min_entry = {
> .attr = {.name = "minimum_io_size", .mode = S_IRUGO },
> .show = queue_io_min_show,
> @@ -555,6 +565,7 @@ static struct attribute *default_attrs[] = {
> &queue_hw_sector_size_entry.attr,
> &queue_logical_block_size_entry.attr,
> &queue_physical_block_size_entry.attr,
> +   &queue_chunk_sectors_entry.attr,
> &queue_io_min_entry.attr,
> &queue_io_opt_entry.attr,
> &queue_discard_granularity_entry.attr,
> --
> 2.7.4

Reviewed-by: Shaun Tancheff 
Tested-by: Shaun Tancheff 


> --
> 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



-- 
Shaun Tancheff
--
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 v2 1/7] block: Add 'zoned' queue limit

[Shaun Tancheff]

On Mon, Sep 26, 2016 at 6:14 AM, Damien Le Moal  wrote:
> Add the zoned queue limit to indicate the zoning model of a block device.
> Defined values are 0 (BLK_ZONED_NONE) for regular block devices,
> 1 (BLK_ZONED_HA) for host-aware zone block devices and 2 (BLK_ZONED_HM)
> for host-managed zone block devices. The standards defined drive managed
> model is not defined here since these block devices do not provide any
> command for accessing zone information. Drive managed model devices will
> be reported as BLK_ZONED_NONE.
>
> The helper functions blk_queue_zoned_model and bdev_zoned_model return
> the zoned limit and the functions blk_queue_is_zoned and bdev_is_zoned
> return a boolean for callers to test if a block device is zoned.
>
> The zoned attribute is also exported as a string to applications via
> sysfs. BLK_ZONED_NONE shows as "none", BLK_ZONED_HA as "host-aware" and
> BLK_ZONED_HM as "host-managed".
>
> Signed-off-by: Damien Le Moal 
> ---
>  block/blk-settings.c   |  1 +
>  block/blk-sysfs.c  | 18 ++
>  include/linux/blkdev.h | 47 +++
>  3 files changed, 66 insertions(+)
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index f679ae1..b1d5b7f 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -107,6 +107,7 @@ void blk_set_default_limits(struct queue_limits *lim)
> lim->io_opt = 0;
> lim->misaligned = 0;
> lim->cluster = 1;
> +   lim->zoned = BLK_ZONED_NONE;
>  }
>  EXPORT_SYMBOL(blk_set_default_limits);
>
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 9cc8d7c..ff9cd9c 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -257,6 +257,18 @@ QUEUE_SYSFS_BIT_FNS(random, ADD_RANDOM, 0);
>  QUEUE_SYSFS_BIT_FNS(iostats, IO_STAT, 0);
>  #undef QUEUE_SYSFS_BIT_FNS
>
> +static ssize_t queue_zoned_show(struct request_queue *q, char *page)
> +{
> +   switch (blk_queue_zoned_model(q)) {
> +   case BLK_ZONED_HA:
> +   return sprintf(page, "host-aware\n");
> +   case BLK_ZONED_HM:
> +   return sprintf(page, "host-managed\n");
> +   default:
> +   return sprintf(page, "none\n");
> +   }
> +}
> +
>  static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
>  {
> return queue_var_show((blk_queue_nomerges(q) << 1) |
> @@ -485,6 +497,11 @@ static struct queue_sysfs_entry queue_nonrot_entry = {
> .store = queue_store_nonrot,
>  };
>
> +static struct queue_sysfs_entry queue_zoned_entry = {
> +   .attr = {.name = "zoned", .mode = S_IRUGO },
> +   .show = queue_zoned_show,
> +};
> +
>  static struct queue_sysfs_entry queue_nomerges_entry = {
> .attr = {.name = "nomerges", .mode = S_IRUGO | S_IWUSR },
> .show = queue_nomerges_show,
> @@ -546,6 +563,7 @@ static struct attribute *default_attrs[] = {
> &queue_discard_zeroes_data_entry.attr,
> &queue_write_same_max_entry.attr,
> &queue_nonrot_entry.attr,
> +   &queue_zoned_entry.attr,
> &queue_nomerges_entry.attr,
> &queue_rq_affinity_entry.attr,
> &queue_iostats_entry.attr,
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index c47c358..f19e16b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -261,6 +261,15 @@ struct blk_queue_tag {
>  #define BLK_SCSI_MAX_CMDS  (256)
>  #define BLK_SCSI_CMD_PER_LONG  (BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
>
> +/*
> + * Zoned block device models (zoned limit).
> + */
> +enum blk_zoned_model {
> +   BLK_ZONED_NONE, /* Regular block device */
> +   BLK_ZONED_HA,   /* Host-aware zoned block device */
> +   BLK_ZONED_HM,   /* Host-managed zoned block device */
> +};
> +
>  struct queue_limits {
> unsigned long   bounce_pfn;
> unsigned long   seg_boundary_mask;
> @@ -290,6 +299,7 @@ struct queue_limits {
> unsigned char   cluster;
> unsigned char   discard_zeroes_data;
> unsigned char   raid_partial_stripes_expensive;
> +   enum blk_zoned_modelzoned;
>  };
>
>  struct request_queue {
> @@ -627,6 +637,23 @@ static inline unsigned int blk_queue_cluster(struct 
> request_queue *q)
> return q->limits.cluster;
>  }
>
> +static inline enum blk_zoned_model
> +blk_queue_zoned_model(struct request_queue *q)
> +{
> +   return q->limits.zoned;
> +}
> +
> +static inline bool blk_queue_is_zoned(struct request_queue *q)
> +{
> +   switch (blk_queue_zoned_model(q)) {
> +   case BLK_ZONED_HA:
> +   case BLK_ZONED_HM:
> +   return true;
> +   default:
> +   return false;
> +   }
> +}
> +
>  /*
>   * We regard a request as sync, if either a read or a sync write
>   */
> @@ -1354,6 +1381,26 @@ static inline unsigned int bdev_write_same(struct 
> block_device *bdev)
> return 0;
>  }
>
> +static inline enum blk_zoned_model bdev_zoned_model(struct bloc

Re: [PATCH v2 6/7] sd: Implement support for ZBC devices

[Shaun Tancheff]

On Mon, Sep 26, 2016 at 6:14 AM, Damien Le Moal  wrote:
> From: Hannes Reinecke 
>
> Implement ZBC support functions to setup zoned disks, both
> host-managed and host-aware models. Only zoned disks that satisfy
> the following conditions are supported:
> 1) All zones are the same size, with the exception of an eventual
>last smaller runt zone.
> 2) For host-managed disks, reads are unrestricted (reads are not
>failed due to zone or write pointer alignement constraints).
> Zoned disks that do not satisfy these 2 conditions will be ignored.
>
> The capacity read of the device triggers the zoned block device
> checks. As this needs the zone model of the disk, the call to
> sd_read_capacity is moved after the call to
> sd_read_block_characteristics so that host-aware devices are
> properlly detected and initialized. The call to sd_zbc_read_zones
> in sd_read_capacity may change the device capacity obtained with
> the sd_read_capacity_16 function for devices reporting only the
> capacity of conventional zones at the beginning of the LBA range
> (i.e. devices with rc_basis set to 0).
>
> Signed-off-by: Hannes Reinecke 
>
> [Damien: * Removed zone cache support
>  * Removed mapping of discard to reset write pointer command
>  * Modified sd_zbc_read_zones to include checks that the
>device satisfies the kernel constraints
>  * Implemeted REPORT ZONES setup and post-processing based
>on code from Shaun Tancheff ]
> Signed-off-by: Damien Le Moal 
> ---
>  drivers/scsi/Makefile |   1 +
>  drivers/scsi/sd.c |  97 ++--
>  drivers/scsi/sd.h |  67 ++
>  drivers/scsi/sd_zbc.c | 586 
> ++
>  include/scsi/scsi_proto.h |  17 ++
>  5 files changed, 754 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/scsi/sd_zbc.c
>
> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
> index fc0d9b8..350513c 100644
> --- a/drivers/scsi/Makefile
> +++ b/drivers/scsi/Makefile
> @@ -180,6 +180,7 @@ hv_storvsc-y:= storvsc_drv.o
>
>  sd_mod-objs:= sd.o
>  sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o
> +sd_mod-$(CONFIG_BLK_DEV_ZONED) += sd_zbc.o
>
>  sr_mod-objs:= sr.o sr_ioctl.o sr_vendor.o
>  ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 51e5629..4b3523b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -93,6 +93,7 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR);
>  MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK);
>  MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
>  MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
> +MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC);
>
>  #if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
>  #define SD_MINORS  16
> @@ -163,7 +164,7 @@ cache_type_store(struct device *dev, struct 
> device_attribute *attr,
> static const char temp[] = "temporary ";
> int len;
>
> -   if (sdp->type != TYPE_DISK)
> +   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
> /* no cache control on RBC devices; theoretically they
>  * can do it, but there's probably so many exceptions
>  * it's not worth the risk */
> @@ -262,7 +263,7 @@ allow_restart_store(struct device *dev, struct 
> device_attribute *attr,
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
>
> -   if (sdp->type != TYPE_DISK)
> +   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
> return -EINVAL;
>
> sdp->allow_restart = simple_strtoul(buf, NULL, 10);
> @@ -392,6 +393,11 @@ provisioning_mode_store(struct device *dev, struct 
> device_attribute *attr,
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
>
> +   if (sd_is_zoned(sdkp)) {
> +   sd_config_discard(sdkp, SD_LBP_DISABLE);
> +   return count;
> +   }
> +
> if (sdp->type != TYPE_DISK)
> return -EINVAL;
>
> @@ -459,7 +465,7 @@ max_write_same_blocks_store(struct device *dev, struct 
> device_attribute *attr,
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
>
> -   if (sdp->type != TYPE_DISK)
> +   if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
> return -EINVAL;
>
> err = kstrtoul(buf, 10, &max);
> @@ -844,6 +850,13 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd 
> *cmd)
>
> BUG_ON(bio_offset(bio) || bio_iovec(bio).bv_len != sdp->sector_size);
>
> +   if (sd_is_zoned(sdkp)) {
> +   /* sd_zbc_setup_read_write uses block layer sector units */
> +   ret = sd_zbc_setup_read_write(sdkp, rq, sector, nr_sectors);
> +   if (ret != BLKPREP_OK)
> +   return ret;
> +   }
> +
> sector >>= ilog2(sdp->sector_size) - 9;
> nr_sectors >>= ilog2(sdp->sector_size) - 9;
>
> @@ -963,6 +976,13 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd 
> *SCpnt)
> SCSI_LOG

Re: [PATCH] scsi: ufs: add support for BLKSECDISCARD

[subhashj]

Hi Pawel,

Please find some comments inline.

On 2016-07-26 04:56, Pawel Wodkowski wrote:

Add BLKSECDISCAD feature support if LU is provisioned for TPRZ
(bProvisioningType = 3).


Did you mean sending purge when bProvisioningType is set to 02h (TPRZ = 
0)? why do we want to send the purge if TPRZ is 1?




To perform BLKSECDISCAD driver issue purge operation after each discard
SCSI command with REQ_SECURE flag set, and delay calling scsi_done()
till purge finish. This operation might long so block requests from 
SCSI

layer in ufshcd_queueucommand() and then unblock it after purge finish.


We had seen purge taking few mins to complete with some of the UFS 
device vendors.
Did you run any experiments to major the time taken for purge to 
complete?




Signed-off-by: Pawel Wodkowski 
---
 drivers/scsi/ufs/ufs.h|  19 +
 drivers/scsi/ufs/ufshcd.c | 187 
+-

 drivers/scsi/ufs/ufshcd.h |   6 ++
 3 files changed, 208 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b291fa6ed2ad..2f769974fda1 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -132,12 +132,14 @@ enum flag_idn {
QUERY_FLAG_IDN_FDEVICEINIT  = 0x01,
QUERY_FLAG_IDN_PWR_ON_WPE   = 0x03,
QUERY_FLAG_IDN_BKOPS_EN = 0x04,
+   QUERY_FLAG_IDN_PURGE_EN = 0x06,
 };

 /* Attribute idn for Query requests */
 enum attr_idn {
QUERY_ATTR_IDN_ACTIVE_ICC_LVL   = 0x03,
QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
+   QUERY_ATTR_IDN_PURGE_STATUS = 0x06,
QUERY_ATTR_IDN_EE_CONTROL   = 0x0D,
QUERY_ATTR_IDN_EE_STATUS= 0x0E,
 };
@@ -247,6 +249,13 @@ enum {
UFSHCD_AMP  = 3,
 };

+/* Provisioning type */
+enum unit_desc_param_provisioning_type {
+   THIN_PROVISIONING_DISABLED  = 0x00,
+   THIN_PROVISIONING_ENABLED_TPRZ_0= 0x02,
+   THIN_PROVISIONING_ENABLED_TPRZ_1= 0x03,
+};
+
 #define POWER_DESC_MAX_SIZE0x62
 #define POWER_DESC_MAX_ACTV_ICC_LVLS   16

@@ -279,6 +288,16 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
 };

+/* Purge operation status */
+enum purge_status {
+   PURGE_STATUS_IDLE = 0x0,
+   PURGE_STATUS_IN_PROGRESS  = 0x1,
+   PURGE_STATUS_STOP_BY_HOST = 0x2,
+   PURGE_STATUS_SUCCESS  = 0x3,
+   PURGE_STATUS_QUEUE_NOT_EMPTY  = 0x4,
+   PURGE_STATUS_GENERAL_FAIL = 0x5
+};
+
 /* UTP QUERY Transaction Specific Fields OpCode */
 enum query_opcode {
UPIU_QUERY_OPCODE_NOP   = 0x0,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f8fa72c31a9d..4ca15a6f294c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -70,6 +70,9 @@
 /* Task management command timeout */
 #define TM_CMD_TIMEOUT 100 /* msecs */

+/* Purge operation timeout */
+#define PURGE_TIMEOUT 9000 /* msecs */
+
 /* maximum number of retries for a general UIC command  */
 #define UFS_UIC_COMMAND_RETRIES 3

@@ -1382,11 +1385,13 @@ static int ufshcd_queuecommand(struct
Scsi_Host *host, struct scsi_cmnd *cmd)
struct ufshcd_lrb *lrbp;
struct ufs_hba *hba;
unsigned long flags;
+   bool secure;
int tag;
int err = 0;

hba = shost_priv(host);

+   secure = !!(cmd->request->cmd_flags & REQ_SECURE);
tag = cmd->request->tag;
if (!ufshcd_valid_tag(hba, tag)) {
dev_err(hba->dev,
@@ -1420,6 +1425,17 @@ static int ufshcd_queuecommand(struct Scsi_Host
*host, struct scsi_cmnd *cmd)
cmd->scsi_done(cmd);
goto out_unlock;
}
+
+   if (secure) {
+   if (hba->is_purge_in_progress) {
+   secure = false;
+   err = SCSI_MLQUEUE_HOST_BUSY;
+   goto out_unlock;
+   }
+
+   hba->is_purge_in_progress = true;
+   }
+
spin_unlock_irqrestore(hba->host->host_lock, flags);

/* acquire the tag to make sure device cmds don't use it */
@@ -1465,9 +1481,19 @@ static int ufshcd_queuecommand(struct Scsi_Host
*host, struct scsi_cmnd *cmd)
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
ufshcd_send_command(hba, tag);
+
+   if (secure) {
+   hba->purge_timeout = jiffies + msecs_to_jiffies(PURGE_TIMEOUT);
+
+   scsi_block_requests(hba->host);
+   }
+
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
 out:
+   if (err && secure && hba->is_purge_in_progress)
+   hba->is_purge_in_progress = false;
+
return err;
 }

@@ -1641,7 +1667,7 @@ static inline void ufshcd_put_dev_cmd_tag(struct
ufs_hba *hba, int tag)
  * ufshcd_exec_dev_cmd - API for sending device management requests
  * @hba - UFS hba
  * @cmd_type - specifi

Re: [PATCH] scsi: ufs: enable no vccq quirk for skhynix device

[subhashj]

Looks good to me.
Reviewed-by: Subhash Jadavani 

On 2016-09-26 07:58, Kyuho Choi wrote:

This patch enable no vccq quirk for SKHynix devices.
SKHynix ufs device don't need vccq vrail for device operation.

Signed-off-by: Kyuho Choi 
---
 drivers/scsi/ufs/ufs_quirks.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufs_quirks.h 
b/drivers/scsi/ufs/ufs_quirks.h

index ee4ab85..22f881e 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -25,6 +25,7 @@

 #define UFS_VENDOR_TOSHIBA 0x198
 #define UFS_VENDOR_SAMSUNG 0x1CE
+#define UFS_VENDOR_SKHYNIX 0x1AD

 /**
  * ufs_device_info - ufs device details
@@ -145,6 +146,7 @@ static struct ufs_dev_fix ufs_fixups[] = {
UFS_DEVICE_QUIRK_PA_TACTIVATE),
UFS_FIX(UFS_VENDOR_TOSHIBA, "THGLF2G9D8KBADG",
UFS_DEVICE_QUIRK_PA_TACTIVATE),
+   UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL, UFS_DEVICE_NO_VCCQ),

END_FIX
 };

--
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 v2 0/3] g_NCR5380: Modernization

[Ondrej Zary]

This small patch series removes deprecated code from g_NCR5380 driver
and converts it from scsi_module.c to scsi_add_host().

Tested with:
HP C2502 (53C400A chip)
Canon FG2-5202 (53C400 chip, memory-mapped)
DTC-3181L (DTCT-436P chip, PnP)

---
Changes in v2: 
 - updated Documentation/scsi/g_NCR5380.txt
 - kept old-style module parameters for compatibility
 - added missing NCR5380_exit() call
 - fixed error propagation from init
 - simplified driver registration

-- 
Ondrej Zary
--
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] g_NCR5380: Stop using scsi_module.c

[Ondrej Zary]

Convert g_NCR5380 to use scsi_add_host instead of scsi_module.c
Use pnp_driver and isa_driver to manage cards.

In order to support multiple cards, new module parameter format is
introduced. The old parameters are kept for compatibility.

Signed-off-by: Ondrej Zary 
---
 Documentation/scsi/g_NCR5380.txt |   24 ++-
 drivers/scsi/g_NCR5380.c |  335 ++
 drivers/scsi/g_NCR5380.h |8 -
 3 files changed, 215 insertions(+), 152 deletions(-)

diff --git a/Documentation/scsi/g_NCR5380.txt b/Documentation/scsi/g_NCR5380.txt
index 843cbae..e2c1879 100644
--- a/Documentation/scsi/g_NCR5380.txt
+++ b/Documentation/scsi/g_NCR5380.txt
@@ -28,6 +28,16 @@ time. More info to come in the future.
 This driver works as a module.
 When included as a module, parameters can be passed on the insmod/modprobe
 command line:
+  irq=xx[,...] the interrupt(s)
+  base=xx[,...]the port or base address(es) (for port or memory 
mapped, resp.)
+  card=xx[,...]card type(s):
+   0 = NCR5380,
+   1 = NCR53C400,
+   2 = NCR53C400A,
+   3 = Domex Technology Corp 3181E (DTC3181E)
+   4 = Hewlett Packard C2502
+
+These old-style parameters can support only one card:
   ncr_irq=xx   the interrupt
   ncr_addr=xx  the port or base address (for port or memory
mapped, resp.)
@@ -36,11 +46,19 @@ command line:
   ncr_53c400a=1 to set up for a NCR53C400A board
   dtc_3181e=1  to set up for a Domex Technology Corp 3181E board
   hp_c2502=1   to set up for a Hewlett Packard C2502 board
+
 e.g.
-modprobe g_NCR5380 ncr_irq=5 ncr_addr=0x350 ncr_5380=1
+OLD: modprobe g_NCR5380 ncr_irq=5 ncr_addr=0x350 ncr_5380=1
+NEW: modprobe g_NCR5380 irq=5 base=0x350 card=0
   for a port mapped NCR5380 board or
-modprobe g_NCR5380 ncr_irq=255 ncr_addr=0xc8000 ncr_53c400=1
-  for a memory mapped NCR53C400 board with interrupts disabled.
+
+OLD: modprobe g_NCR5380 ncr_irq=255 ncr_addr=0xc8000 ncr_53c400=1
+NEW: modprobe g_NCR5380 irq=255 base=0xc8000 card=1
+  for a memory mapped NCR53C400 board with interrupts disabled or
+
+NEW: modprobe g_NCR5380 irq=0,7 base=0x240,0x300 card=3,4
+  for two cards: DTC3181 (in non-PnP mode) at 0x240 with no IRQ
+ and HP C2502 at 0x300 with IRQ 7
 
 (255 should be specified for no or DMA interrupt, 254 to autoprobe for an 
  IRQ line if overridden on the command line.)
diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 5162de6..cbf0103 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -30,24 +30,41 @@
 #include "NCR5380.h"
 #include 
 #include 
-#include 
+#include 
+#include 
 #include 
 
+#define MAX_CARDS 8
+
+/* old-style parameters for compatibility */
 static int ncr_irq;
-static int ncr_dma;
 static int ncr_addr;
 static int ncr_5380;
 static int ncr_53c400;
 static int ncr_53c400a;
 static int dtc_3181e;
 static int hp_c2502;
+module_param(ncr_irq, int, 0);
+module_param(ncr_addr, int, 0);
+module_param(ncr_5380, int, 0);
+module_param(ncr_53c400, int, 0);
+module_param(ncr_53c400a, int, 0);
+module_param(dtc_3181e, int, 0);
+module_param(hp_c2502, int, 0);
+
+static int irq[] = { 0, 0, 0, 0, 0, 0, 0, 0 };
+module_param_array(irq, int, NULL, 0);
+MODULE_PARM_DESC(irq, "IRQ number(s)");
 
-static struct card {
-   NCR5380_map_type NCR5380_map_name;
-   int irq;
-   int dma;
-   int board;  /* Use NCR53c400, Ricoh, etc. extensions ? */
-} card;
+static int base[] = { 0, 0, 0, 0, 0, 0, 0, 0 };
+module_param_array(base, int, NULL, 0);
+MODULE_PARM_DESC(base, "base address(es)");
+
+static int card[] = { -1, -1, -1, -1, -1, -1, -1, -1 };
+module_param_array(card, int, NULL, 0);
+MODULE_PARM_DESC(card, "card type (0=NCR5380, 1=NCR53C400, 2=NCR53C400A, 
3=DTC3181E, 4=HP C2502)");
+
+MODULE_LICENSE("GPL");
 
 #ifndef SCSI_G_NCR5380_MEM
 /*
@@ -73,17 +90,8 @@ static void magic_configure(int idx, u8 irq, u8 magic[])
 }
 #endif
 
-/**
- * generic_NCR5380_detect  -   look for NCR5380 controllers
- * @tpnt: the scsi template
- *
- * Scan for the present of NCR5380, NCR53C400, NCR53C400A, DTC3181E
- * and DTC436(ISAPnP) controllers.
- *
- * Locks: none
- */
-
-static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
+static int generic_NCR5380_init_one(struct scsi_host_template *tpnt,
+   struct device *pdev, int base, int irq, int board)
 {
unsigned int *ports;
u8 *magic = NULL;
@@ -92,80 +100,29 @@ static int __init generic_NCR5380_detect(struct 
scsi_host_template *tpnt)
int port_idx = -1;
unsigned long region_size;
 #endif
-   static unsigned int __initdata ncr_53c400a_ports[] = {
+   static unsigned int ncr_53c400a_ports[] = {
0x280, 0x290, 0x300, 0x310, 0x330, 0x340, 0x348, 0x350, 0
};
-   static unsigned int __initdata dtc_3181e_ports[] = {
+   static unsigned int dtc_3181e_ports[] 

[PATCH 1/3] g_NCR5380: Remove deprecated __setup

[Ondrej Zary]

Remove deprecated __setup for parsing command line parameters.
g_NCR5380.* parameters could be used instead.

This might break existing setups with g_NCR5380 built-in (if there are
any). But it has to go in order to remove the overrides[] array.

Signed-off-by: Ondrej Zary 
---
 Documentation/scsi/g_NCR5380.txt |   10 ---
 drivers/scsi/g_NCR5380.c |  135 --
 2 files changed, 145 deletions(-)

diff --git a/Documentation/scsi/g_NCR5380.txt b/Documentation/scsi/g_NCR5380.txt
index fd88015..843cbae 100644
--- a/Documentation/scsi/g_NCR5380.txt
+++ b/Documentation/scsi/g_NCR5380.txt
@@ -21,16 +21,6 @@ NCR53c400 card, the Trantor T130B in its default 
configuration:
 The NCR53c400 does not support DMA but it does have Pseudo-DMA which is
 supported by the driver.
 
-If the default configuration does not work for you, you can use the kernel
-command lines (eg using the lilo append command):
-   ncr5380=addr,irq
-   ncr53c400=addr,irq
-   ncr53c400a=addr,irq
-   dtc3181e=addr,irq
-
-The driver does not probe for any addresses or ports other than those in
-the OVERRIDE or given to the kernel as above.
-
 This driver provides some information on what it has detected in
 /proc/scsi/g_NCR5380/x where x is the scsi card number as detected at boot
 time. More info to come in the future.
diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 516bd6c..7e50b44e 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -56,136 +56,6 @@ static struct override {
 
 #define NO_OVERRIDES ARRAY_SIZE(overrides)
 
-#ifndef MODULE
-
-/**
- * internal_setup  -   handle lilo command string override
- * @board: BOARD_* identifier for the board
- * @str: unused
- * @ints: numeric parameters
- *
- * Do LILO command line initialization of the overrides array. Display
- * errors when needed
- *
- * Locks: none
- */
-
-static void __init internal_setup(int board, char *str, int *ints)
-{
-   static int commandline_current;
-   switch (board) {
-   case BOARD_NCR5380:
-   if (ints[0] != 2 && ints[0] != 3) {
-   printk(KERN_ERR "generic_NCR5380_setup : usage 
ncr5380=" STRVAL(NCR5380_map_name) ",irq,dma\n");
-   return;
-   }
-   break;
-   case BOARD_NCR53C400:
-   if (ints[0] != 2) {
-   printk(KERN_ERR "generic_NCR53C400_setup : usage 
ncr53c400=" STRVAL(NCR5380_map_name) ",irq\n");
-   return;
-   }
-   break;
-   case BOARD_NCR53C400A:
-   if (ints[0] != 2) {
-   printk(KERN_ERR "generic_NCR53C400A_setup : usage 
ncr53c400a=" STRVAL(NCR5380_map_name) ",irq\n");
-   return;
-   }
-   break;
-   case BOARD_DTC3181E:
-   if (ints[0] != 2) {
-   printk("generic_DTC3181E_setup : usage dtc3181e=" 
STRVAL(NCR5380_map_name) ",irq\n");
-   return;
-   }
-   break;
-   }
-
-   if (commandline_current < NO_OVERRIDES) {
-   overrides[commandline_current].NCR5380_map_name = 
(NCR5380_map_type) ints[1];
-   overrides[commandline_current].irq = ints[2];
-   if (ints[0] == 3)
-   overrides[commandline_current].dma = ints[3];
-   else
-   overrides[commandline_current].dma = DMA_NONE;
-   overrides[commandline_current].board = board;
-   ++commandline_current;
-   }
-}
-
-
-/**
- * do_NCR53C80_setup   -   set up entry point
- * @str: unused
- *
- * Setup function invoked at boot to parse the ncr5380= command
- * line.
- */
-
-static int __init do_NCR5380_setup(char *str)
-{
-   int ints[10];
-
-   get_options(str, ARRAY_SIZE(ints), ints);
-   internal_setup(BOARD_NCR5380, str, ints);
-   return 1;
-}
-
-/**
- * do_NCR53C400_setup  -   set up entry point
- * @str: unused
- * @ints: integer parameters from kernel setup code
- *
- * Setup function invoked at boot to parse the ncr53c400= command
- * line.
- */
-
-static int __init do_NCR53C400_setup(char *str)
-{
-   int ints[10];
-
-   get_options(str, ARRAY_SIZE(ints), ints);
-   internal_setup(BOARD_NCR53C400, str, ints);
-   return 1;
-}
-
-/**
- * do_NCR53C400A_setup -   set up entry point
- * @str: unused
- * @ints: integer parameters from kernel setup code
- *
- * Setup function invoked at boot to parse the ncr53c400a= command
- * line.
- */
-
-static int __init do_NCR53C400A_setup(char *str)
-{
-   int ints[10];
-
-   get_options(str, ARRAY_SIZE(ints), ints);
-   internal_setup(BOARD_NCR53C400A, str, ints);
-   return 1;
-}
-
-/**
- * do_DTC3181E_setup   -   set up entry poin

[PATCH 2/3] g_NCR5380: Reduce overrides[] from array to struct

[Ondrej Zary]

Remove compile-time card type definition GENERIC_NCR5380_OVERRIDE.
Then remove all code iterating the overrides[] array and reduce it to
struct card.

Signed-off-by: Ondrej Zary 
---
 drivers/scsi/g_NCR5380.c |  351 ++
 1 file changed, 167 insertions(+), 184 deletions(-)

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 7e50b44e..5162de6 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -42,19 +42,12 @@ static int ncr_53c400a;
 static int dtc_3181e;
 static int hp_c2502;
 
-static struct override {
+static struct card {
NCR5380_map_type NCR5380_map_name;
int irq;
int dma;
int board;  /* Use NCR53c400, Ricoh, etc. extensions ? */
-} overrides
-#ifdef GENERIC_NCR5380_OVERRIDE
-[] __initdata = GENERIC_NCR5380_OVERRIDE;
-#else
-[1] __initdata = { { 0,},};
-#endif
-
-#define NO_OVERRIDES ARRAY_SIZE(overrides)
+} card;
 
 #ifndef SCSI_G_NCR5380_MEM
 /*
@@ -85,16 +78,13 @@ static void magic_configure(int idx, u8 irq, u8 magic[])
  * @tpnt: the scsi template
  *
  * Scan for the present of NCR5380, NCR53C400, NCR53C400A, DTC3181E
- * and DTC436(ISAPnP) controllers. If overrides have been set we use
- * them.
+ * and DTC436(ISAPnP) controllers.
  *
  * Locks: none
  */
 
 static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
 {
-   static int current_override;
-   int count;
unsigned int *ports;
u8 *magic = NULL;
 #ifndef SCSI_G_NCR5380_MEM
@@ -124,28 +114,25 @@ static int __init generic_NCR5380_detect(struct 
scsi_host_template *tpnt)
 #endif
 
if (ncr_irq)
-   overrides[0].irq = ncr_irq;
+   card.irq = ncr_irq;
if (ncr_dma)
-   overrides[0].dma = ncr_dma;
+   card.dma = ncr_dma;
if (ncr_addr)
-   overrides[0].NCR5380_map_name = (NCR5380_map_type) ncr_addr;
+   card.NCR5380_map_name = (NCR5380_map_type) ncr_addr;
if (ncr_5380)
-   overrides[0].board = BOARD_NCR5380;
+   card.board = BOARD_NCR5380;
else if (ncr_53c400)
-   overrides[0].board = BOARD_NCR53C400;
+   card.board = BOARD_NCR53C400;
else if (ncr_53c400a)
-   overrides[0].board = BOARD_NCR53C400A;
+   card.board = BOARD_NCR53C400A;
else if (dtc_3181e)
-   overrides[0].board = BOARD_DTC3181E;
+   card.board = BOARD_DTC3181E;
else if (hp_c2502)
-   overrides[0].board = BOARD_HP_C2502;
+   card.board = BOARD_HP_C2502;
 #ifndef SCSI_G_NCR5380_MEM
-   if (!current_override && isapnp_present()) {
+   if (isapnp_present()) {
struct pnp_dev *dev = NULL;
-   count = 0;
while ((dev = pnp_find_dev(NULL, ISAPNP_VENDOR('D', 'T', 'C'), 
ISAPNP_FUNCTION(0x436e), dev))) {
-   if (count >= NO_OVERRIDES)
-   break;
if (pnp_device_attach(dev) < 0)
continue;
if (pnp_activate_dev(dev) < 0) {
@@ -159,202 +146,198 @@ static int __init generic_NCR5380_detect(struct 
scsi_host_template *tpnt)
continue;
}
if (pnp_irq_valid(dev, 0))
-   overrides[count].irq = pnp_irq(dev, 0);
+   card.irq = pnp_irq(dev, 0);
else
-   overrides[count].irq = NO_IRQ;
+   card.irq = NO_IRQ;
if (pnp_dma_valid(dev, 0))
-   overrides[count].dma = pnp_dma(dev, 0);
+   card.dma = pnp_dma(dev, 0);
else
-   overrides[count].dma = DMA_NONE;
-   overrides[count].NCR5380_map_name = (NCR5380_map_type) 
pnp_port_start(dev, 0);
-   overrides[count].board = BOARD_DTC3181E;
-   count++;
+   card.dma = DMA_NONE;
+   card.NCR5380_map_name = (NCR5380_map_type) 
pnp_port_start(dev, 0);
+   card.board = BOARD_DTC3181E;
+   break;
}
}
 #endif
 
-   for (count = 0; current_override < NO_OVERRIDES; ++current_override) {
-   if (!(overrides[current_override].NCR5380_map_name))
-   continue;
+   if (!(card.NCR5380_map_name))
+   return 0;
 
-   ports = NULL;
-   flags = 0;
-   switch (overrides[current_override].board) {
-   case BOARD_NCR5380:
-   flags = FLAG_NO_PSEUDO_DMA | FLAG_DMA_FIXUP;
-   break;
-   case BOARD_NCR53C400A:
-  

[PATCH v3 7/7] blk-zoned: implement ioctls

[Shaun Tancheff]

Adds the new BLKREPORTZONE and BLKRESETZONE ioctls for respectively
obtaining the zone configuration of a zoned block device and resetting
the write pointer of sequential zones of a zoned block device.

The BLKREPORTZONE ioctl maps directly to a single call of the function
blkdev_report_zones. The zone information result is passed as an array
of struct blk_zone identical to the structure used internally for
processing the REQ_OP_ZONE_REPORT operation.  The BLKRESETZONE ioctl
maps to a single call of the blkdev_reset_zones function.

Signed-off-by: Shaun Tancheff 
Signed-off-by: Damien Le Moal 
---
Changes since v2:
 - Changed kzalloc() to kcalloc() per Christoph
 - Added ioctl specific bits to uapi as blkzoned.h is now added in an earlier
   patch.

 block/blk-zoned.c | 93 +++
 block/ioctl.c |  4 ++
 include/linux/blkdev.h| 22 ++
 include/uapi/linux/blkzoned.h | 40 +++
 include/uapi/linux/fs.h   |  4 ++
 5 files changed, 163 insertions(+)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index bc4159d..91f7347 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -240,3 +240,96 @@ int blkdev_reset_zones(struct block_device *bdev,
return 0;
 }
 EXPORT_SYMBOL_GPL(blkdev_reset_zones);
+
+/**
+ * BLKREPORTZONE ioctl processing.
+ * Called from blkdev_ioctl.
+ */
+int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+   void __user *argp = (void __user *)arg;
+   struct request_queue *q;
+   struct blk_zone_report rep;
+   struct blk_zone *zones;
+   int ret;
+
+   if (!argp)
+   return -EINVAL;
+
+   q = bdev_get_queue(bdev);
+   if (!q)
+   return -ENXIO;
+
+   if (!blk_queue_is_zoned(q))
+   return -ENOTTY;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
+   return -EFAULT;
+
+   if (!rep.nr_zones)
+   return -EINVAL;
+
+   zones = kcalloc(rep.nr_zones, sizeof(struct blk_zone), GFP_KERNEL);
+   if (!zones)
+   return -ENOMEM;
+
+   ret = blkdev_report_zones(bdev, rep.sector,
+ zones, &rep.nr_zones,
+ GFP_KERNEL);
+   if (ret)
+   goto out;
+
+   if (copy_to_user(argp, &rep, sizeof(struct blk_zone_report))) {
+   ret = -EFAULT;
+   goto out;
+   }
+
+   if (rep.nr_zones) {
+   if (copy_to_user(argp + sizeof(struct blk_zone_report), zones,
+sizeof(struct blk_zone) * rep.nr_zones))
+   ret = -EFAULT;
+   }
+
+ out:
+   kfree(zones);
+
+   return ret;
+}
+
+/**
+ * BLKRESETZONE ioctl processing.
+ * Called from blkdev_ioctl.
+ */
+int blkdev_reset_zones_ioctl(struct block_device *bdev, fmode_t mode,
+unsigned int cmd, unsigned long arg)
+{
+   void __user *argp = (void __user *)arg;
+   struct request_queue *q;
+   struct blk_zone_range zrange;
+
+   if (!argp)
+   return -EINVAL;
+
+   q = bdev_get_queue(bdev);
+   if (!q)
+   return -ENXIO;
+
+   if (!blk_queue_is_zoned(q))
+   return -ENOTTY;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   if (!(mode & FMODE_WRITE))
+   return -EBADF;
+
+   if (copy_from_user(&zrange, argp, sizeof(struct blk_zone_range)))
+   return -EFAULT;
+
+   return blkdev_reset_zones(bdev, zrange.sector, zrange.nr_sectors,
+ GFP_KERNEL);
+}
diff --git a/block/ioctl.c b/block/ioctl.c
index ed2397f..448f78a 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -513,6 +513,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, 
unsigned cmd,
BLKDEV_DISCARD_SECURE);
case BLKZEROOUT:
return blk_ioctl_zeroout(bdev, mode, arg);
+   case BLKREPORTZONE:
+   return blkdev_report_zones_ioctl(bdev, mode, cmd, arg);
+   case BLKRESETZONE:
+   return blkdev_reset_zones_ioctl(bdev, mode, cmd, arg);
case HDIO_GETGEO:
return blkdev_getgeo(bdev, argp);
case BLKRAGET:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6316972..0a75285 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -315,6 +315,28 @@ extern int blkdev_report_zones(struct block_device *,
unsigned int *, gfp_t);
 extern int blkdev_reset_zones(struct block_device *, sector_t,
sector_t, gfp_t);
+
+extern int blkdev_report_zones_ioctl(struct block_device *, fmode_t,
+unsigned int, unsigned long);

[PATCH v3 5/7] block: Implement support for zoned block devices

[Shaun Tancheff]

From: Hannes Reinecke 

Implement zoned block device zone information reporting and reset.
Zone information are reported as struct blk_zone. This implementation
does not differentiate between host-aware and host-managed device
models and is valid for both. Two functions are provided:
blkdev_report_zones for discovering the zone configuration of a
zoned block device, and blkdev_reset_zones for resetting the write
pointer of sequential zones. The helper function blk_queue_zone_size
and bdev_zone_size are also provided for, as the name suggest,
obtaining the zone size (in 512B sectors) of the zones of the device.

Signed-off-by: Hannes Reinecke 

[Damien: * Removed the zone cache
 * Implement report zones operation based on earlier proposal
   by Shaun Tancheff ]
Signed-off-by: Damien Le Moal 
---

Changes from v2:
 - Added EXPORT_SYMBOL_GPL() per Damien
 - Added uapi blkzoned.h earlier and put shared enums/struct directly
   into blkzoned.h

 block/Kconfig |   8 ++
 block/Makefile|   1 +
 block/blk-zoned.c | 242 ++
 include/linux/blkdev.h|  30 ++
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/blkzoned.h | 103 ++
 6 files changed, 385 insertions(+)
 create mode 100644 block/blk-zoned.c
 create mode 100644 include/uapi/linux/blkzoned.h

diff --git a/block/Kconfig b/block/Kconfig
index 1d4d624..6b0ad08 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -89,6 +89,14 @@ config BLK_DEV_INTEGRITY
T10/SCSI Data Integrity Field or the T13/ATA External Path
Protection.  If in doubt, say N.
 
+config BLK_DEV_ZONED
+   bool "Zoned block device support"
+   ---help---
+   Block layer zoned block device support. This option enables
+   support for ZAC/ZBC host-managed and host-aware zoned block devices.
+
+   Say yes here if you have a ZAC or ZBC storage device.
+
 config BLK_DEV_THROTTLING
bool "Block layer bio throttling support"
depends on BLK_CGROUP=y
diff --git a/block/Makefile b/block/Makefile
index 36acdd7..9371bc7 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -22,4 +22,5 @@ obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o
 obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o
 obj-$(CONFIG_BLK_CMDLINE_PARSER)   += cmdline-parser.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
+obj-$(CONFIG_BLK_DEV_ZONED)+= blk-zoned.o
 obj-$(CONFIG_BLK_MQ_PCI)   += blk-mq-pci.o
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
new file mode 100644
index 000..bc4159d
--- /dev/null
+++ b/block/blk-zoned.c
@@ -0,0 +1,242 @@
+/*
+ * Zoned block device handling
+ *
+ * Copyright (c) 2015, Hannes Reinecke
+ * Copyright (c) 2015, SUSE Linux GmbH
+ *
+ * Copyright (c) 2016, Damien Le Moal
+ * Copyright (c) 2016, Western Digital
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static inline sector_t blk_zone_start(struct request_queue *q,
+ sector_t sector)
+{
+   sector_t zone_mask = blk_queue_zone_size(q) - 1;
+
+   return sector & ~zone_mask;
+}
+
+static inline void blkdev_report_to_zone(struct block_device *bdev,
+void *rep,
+struct blk_zone *zone)
+{
+   sector_t offset = get_start_sect(bdev);
+
+   memcpy(zone, rep, sizeof(struct blk_zone));
+   zone->start -= offset;
+   if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
+   zone->wp = zone->start + zone->len;
+   else
+   zone->wp -= offset;
+}
+
+/**
+ * blkdev_report_zones - Get zones information
+ * @bdev:  Target block device
+ * @sector:Sector from which to report zones
+ * @zones:  Array of zone structures where to return the zones information
+ * @nr_zones:   Number of zone structures in the zone array
+ * @gfp_mask:  Memory allocation flags (for bio_alloc)
+ *
+ * Description:
+ *Get zone information starting from the zone containing @sector.
+ *The number of zone information reported may be less than the number
+ *requested by @nr_zones. The number of zones actually reported is
+ *returned in @nr_zones.
+ */
+int blkdev_report_zones(struct block_device *bdev,
+   sector_t sector,
+   struct blk_zone *zones,
+   unsigned int *nr_zones,
+   gfp_t gfp_mask)
+{
+   struct request_queue *q = bdev_get_queue(bdev);
+   struct blk_zone_report_hdr *hdr;
+   unsigned int nrz = *nr_zones;
+   struct page *page;
+   unsigned int nr_rep;
+   size_t rep_bytes;
+   unsigned int nr_pages;
+   struct bio *bio;
+   struct bio_vec *bv;
+   unsigned int i, nz;
+   unsigned int ofst;
+   void *addr;
+   int ret = 0;
+
+   if (!q)
+   return -ENXIO;
+
+   if (!blk_queue_is_zoned(q))
+   return -EOPNOTSUPP;

Re: [dm-devel] dm-mpath: always return reservation conflict

[James Bottomley]

On Tue, 2016-09-27 at 08:34 +0200, Hannes Reinecke wrote:
> On 09/26/2016 09:06 PM, James Bottomley wrote:
> > On Mon, 2016-09-26 at 09:52 -0700, Christoph Hellwig wrote:
> > > Getting back to this after Hannes recovered from his vacation
> > > and I had a chat with him..
> > > 
> > > On Mon, Aug 15, 2016 at 09:40:39AM -0400, Mike Snitzer wrote:
> > > > Seems we still need a more sophisticated approach.  But I'm 
> > > > left wondering: if we didn't do it would anything notice? 
> > > >  Sadly, the same big question from the original thread from a
> > > > year ago:
> > > 
> > > Yes.  I have a customer looking to push the pNFS SCSI layout into
> > > a product, and the major show stopper right now is that we can
> > > trivially get into failver loops without this (or and equivalent)
> > > fix.
> > > 
> > > A year ago SCSI layout was still work in progress in the IETF,
> > > people use the similar block layout instead that doesn't use
> > > PRs and we also didn't have the in-kernel PR API, so you 
> > > effectively couldn't use PRs with multipathing.
> > > 
> > > > https://patchwork.kernel.org/patch/6797111/
> > > > 
> > > > > So this is throw-away for now (and I'll get Hannes' patch 
> > > > > applied for 4.8-rc3, with the tweak of returning -EBADE
> > > > > immediately):
> > > > 
> > > > Unfortunately, I'm _not_ staging Hannes' patch until I have 
> > > > James Bottomley's Ack (given his original issues with the patch
> > > > haven't been explained away AFAICT).
> > > 
> > > I've added James to the Cc.  His argument was that the old 
> > > behavior could be implemented to use some non-standard use of 
> > > reservations without a specific example.  I don't really think 
> > > his example even is practical - once we use dm-mpath it 
> > > exclusively claims the underling block devices, so any sort of 
> > > selective reservations would have had to happen before even
> > > starting dm-multipath.
> > 
> > Well, now that you've made me reread the thread from 14 months ago 
> > that wasn't quite my objection.  The objection hinged on the fact 
> > that anything that uses path specific reservations would now fail
> > instead of retrying on a different path.  I thought the IBM SVC did 
> > this and Hannes implied he'd be able to check this ... did anyone 
> > check?  If we've checked and there's no issue with the SVC, then I 
> > don't have any other objections.
> > 
> > >   So a dynamic SAN controller would have to tear down and rebuild 
> > > the dm-multipath setup at all the time.
> > 
> > That was the job of the SVC: it sat in the middle of the SAN and
> > controlled which node saw what storage.
> > 
> > https://www.ibm.com/support/knowledgecenter/STPVGU/com.ibm.storage.
> > svc.console.720.doc/svc_svcovr_1bcfiq.html
> > 
> > The SVC can issue its own reservations in those circumstances. 
> >  What I'm not at all clear on is whether they'll interact badly 
> > with the dm-mp reservations.
> > 
> In the end SVC is (for us) just another storage array.
> If and what SVC does in the background is of no interest to us.

How can that be true?  It sits *on* the san and manages devices, it
doesn't sit between the initators and the devices.  It applies
reservations to devices under management, but every node usually sees
everything else, so devices under SVC management are visible to all
initators unless you zone them off.

The last SVC manual I saw included a procedure for manually releasing
stuck SVC reservations from an initator, which illustrates the
expectation.

> OTOH I'd be very surprised if the SVC would be allowing us to see
> remnants of its internal working (like persistent reservation 
> errors); in doing so third-party applications would be able to see 
> and possibly modify these persistent reservations and the SVC would 
> find itself in a very fragile operating scenario.

Because unless you zone the fibre, that's precisely what you do see.

> Also interactions with GPFS (which uses it's own set of reservations)
> will become very tricky.
> 
> So I sincerely doubt we'll ever see SVC-originated persistent
> reservations errors.
> 
> And as a side-note, this particular patch is included in SLES since
> 2011. With no noticeable side-effect.

OK, so can you actually say that someone has tested this scenario?  If
not, do you have the capacity to test it?

James

--
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 v2 7/7] blk-zoned: implement ioctls

[Christoph Hellwig]

On Mon, Sep 26, 2016 at 06:12:24PM -0500, Shaun Tancheff wrote:
> Except our source locations are disjoint (stack and kcalloc'd).

Indeed.
--
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 9/9] [RFC] nvme: Fix a race condition

[Bart Van Assche]

On 09/27/2016 09:56 AM, James Bottomley wrote:

On Tue, 2016-09-27 at 09:43 -0700, Bart Van Assche wrote:

On 09/27/2016 09:31 AM, Steve Wise wrote:

@@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
struct nvme_ns *ns;
+   struct request_queue *q;

mutex_lock(&ctrl->namespaces_mutex);
list_for_each_entry(ns, &ctrl->namespaces, list) {
-   blk_mq_cancel_requeue_work(ns->queue);
-   blk_mq_stop_hw_queues(ns->queue);
+   q = ns->queue;
+   blk_quiesce_queue(q);
+   blk_mq_cancel_requeue_work(q);
+   blk_mq_stop_hw_queues(q);
+   blk_resume_queue(q);
}
mutex_unlock(&ctrl->namespaces_mutex);


Hey Bart, should nvme_stop_queues() really be resuming the blk
queue?


Hello Steve,

Would you perhaps prefer that blk_resume_queue(q) is called from
nvme_start_queues()? I think that would make the NVMe code harder to
review. The above code won't cause any unexpected side effects if an
NVMe namespace is removed after nvme_stop_queues() has been called
and before nvme_start_queues() is called. Moving the
blk_resume_queue(q) call into nvme_start_queues() will only work as
expected if no namespaces are added nor removed between the
nvme_stop_queues() and nvme_start_queues() calls. I'm not familiar
enough with the NVMe code to know whether or not this change is safe
...


It's something that looks obviously wrong, so explain why you need to
do it, preferably in a comment above the function.


Hello James and Steve,

I will add a comment.

Please note that the above patch does not change the behavior of 
nvme_stop_queues() except that it causes nvme_stop_queues() to wait 
until any ongoing nvme_queue_rq() calls have finished. 
blk_resume_queue() does not affect the value of the BLK_MQ_S_STOPPED bit 
that has been set by blk_mq_stop_hw_queues(). All it does is to resume 
pending blk_queue_enter() calls and to ensure that future 
blk_queue_enter() calls do not block. Even after blk_resume_queue() has 
been called if a new request is queued queue_rq() won't be invoked 
because the BLK_MQ_S_STOPPED bit is still set. Patch "dm: Fix a race 
condition related to stopping and starting queues" realizes a similar 
change in the dm driver and that change has been tested extensively.


Bart.
--
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

[Bug 172831] Adaptec ASR7805 in RAID (Expose RAW) mode recreate block devices and broke MDRAID

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=172831

Dave Carroll  changed:

   What|Removed |Added

 CC||david.carr...@microsemi.com

--- Comment #9 from Dave Carroll  ---
Hi Badalyan,

We have entered a ticket for this request into our backlog. We will let you
know as this progresses.

Thanks, -Dave

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 9/9] [RFC] nvme: Fix a race condition

[Steve Wise]

> On 09/27/2016 09:31 AM, Steve Wise wrote:
> >> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> >>  void nvme_stop_queues(struct nvme_ctrl *ctrl)
> >>  {
> >>struct nvme_ns *ns;
> >> +  struct request_queue *q;
> >>
> >>mutex_lock(&ctrl->namespaces_mutex);
> >>list_for_each_entry(ns, &ctrl->namespaces, list) {
> >> -  blk_mq_cancel_requeue_work(ns->queue);
> >> -  blk_mq_stop_hw_queues(ns->queue);
> >> +  q = ns->queue;
> >> +  blk_quiesce_queue(q);
> >> +  blk_mq_cancel_requeue_work(q);
> >> +  blk_mq_stop_hw_queues(q);
> >> +  blk_resume_queue(q);
> >>}
> >>mutex_unlock(&ctrl->namespaces_mutex);
> >
> > Hey Bart, should nvme_stop_queues() really be resuming the blk queue?
> 
> Hello Steve,
> 
> Would you perhaps prefer that blk_resume_queue(q) is called from
> nvme_start_queues()? I think that would make the NVMe code harder to
> review. 

I'm still learning the blk code (and nvme code :)), but I would think
blk_resume_queue() would cause requests to start being submit on the NVME
queues, which I believe shouldn't happen when they are stopped.  I'm currently
debugging a problem where requests are submitted to the nvme-rdma driver while
it has supposedly stopped all the nvme and blk mqs.  I tried your series at
Christoph's request to see if it resolved my problem, but it didn't.  

> The above code won't cause any unexpected side effects if an
> NVMe namespace is removed after nvme_stop_queues() has been called and
> before nvme_start_queues() is called. Moving the blk_resume_queue(q)
> call into nvme_start_queues() will only work as expected if no
> namespaces are added nor removed between the nvme_stop_queues() and
> nvme_start_queues() calls. I'm not familiar enough with the NVMe code to
> know whether or not this change is safe ...
> 

I'll have to look and see if new namespaces can be added/deleted while a nvme
controller is in the RECONNECTING state.   In the meantime, I'm going to move
the blk_resume_queue() to nvme_start_queues() and see if it helps my problem.

Christoph:  Thoughts?

Steve.

--
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

[Bug 172841] aacraid does not support TRIM in Raw (Pass Through) devices

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=172841

Dave Carroll  changed:

   What|Removed |Added

 CC||david.carr...@microsemi.com

--- Comment #1 from Dave Carroll  ---
Hi Badalyan,

We are aware of the issue, and this is on our list of topics for a future
release.

I'd like to put this is acknowledged.

Thanks, -Dave

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 9/9] [RFC] nvme: Fix a race condition

[James Bottomley]

On Tue, 2016-09-27 at 09:43 -0700, Bart Van Assche wrote:
> On 09/27/2016 09:31 AM, Steve Wise wrote:
> > > @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> > >  void nvme_stop_queues(struct nvme_ctrl *ctrl)
> > >  {
> > >   struct nvme_ns *ns;
> > > + struct request_queue *q;
> > > 
> > >   mutex_lock(&ctrl->namespaces_mutex);
> > >   list_for_each_entry(ns, &ctrl->namespaces, list) {
> > > - blk_mq_cancel_requeue_work(ns->queue);
> > > - blk_mq_stop_hw_queues(ns->queue);
> > > + q = ns->queue;
> > > + blk_quiesce_queue(q);
> > > + blk_mq_cancel_requeue_work(q);
> > > + blk_mq_stop_hw_queues(q);
> > > + blk_resume_queue(q);
> > >   }
> > >   mutex_unlock(&ctrl->namespaces_mutex);
> > 
> > Hey Bart, should nvme_stop_queues() really be resuming the blk
> > queue?
> 
> Hello Steve,
> 
> Would you perhaps prefer that blk_resume_queue(q) is called from 
> nvme_start_queues()? I think that would make the NVMe code harder to 
> review. The above code won't cause any unexpected side effects if an 
> NVMe namespace is removed after nvme_stop_queues() has been called 
> and before nvme_start_queues() is called. Moving the 
> blk_resume_queue(q) call into nvme_start_queues() will only work as 
> expected if no namespaces are added nor removed between the 
> nvme_stop_queues() and nvme_start_queues() calls. I'm not familiar 
> enough with the NVMe code to know whether or not this change is safe
> ...

It's something that looks obviously wrong, so explain why you need to
do it, preferably in a comment above the function.

James


--
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 9/9] [RFC] nvme: Fix a race condition

[Bart Van Assche]

On 09/27/2016 09:31 AM, Steve Wise wrote:

@@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
struct nvme_ns *ns;
+   struct request_queue *q;

mutex_lock(&ctrl->namespaces_mutex);
list_for_each_entry(ns, &ctrl->namespaces, list) {
-   blk_mq_cancel_requeue_work(ns->queue);
-   blk_mq_stop_hw_queues(ns->queue);
+   q = ns->queue;
+   blk_quiesce_queue(q);
+   blk_mq_cancel_requeue_work(q);
+   blk_mq_stop_hw_queues(q);
+   blk_resume_queue(q);
}
mutex_unlock(&ctrl->namespaces_mutex);


Hey Bart, should nvme_stop_queues() really be resuming the blk queue?


Hello Steve,

Would you perhaps prefer that blk_resume_queue(q) is called from 
nvme_start_queues()? I think that would make the NVMe code harder to 
review. The above code won't cause any unexpected side effects if an 
NVMe namespace is removed after nvme_stop_queues() has been called and 
before nvme_start_queues() is called. Moving the blk_resume_queue(q) 
call into nvme_start_queues() will only work as expected if no 
namespaces are added nor removed between the nvme_stop_queues() and 
nvme_start_queues() calls. I'm not familiar enough with the NVMe code to 
know whether or not this change is safe ...


Bart.
--
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 9/9] [RFC] nvme: Fix a race condition

[Steve Wise]

> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
>  void nvme_stop_queues(struct nvme_ctrl *ctrl)
>  {
>   struct nvme_ns *ns;
> + struct request_queue *q;
> 
>   mutex_lock(&ctrl->namespaces_mutex);
>   list_for_each_entry(ns, &ctrl->namespaces, list) {
> - blk_mq_cancel_requeue_work(ns->queue);
> - blk_mq_stop_hw_queues(ns->queue);
> + q = ns->queue;
> + blk_quiesce_queue(q);
> + blk_mq_cancel_requeue_work(q);
> + blk_mq_stop_hw_queues(q);
> + blk_resume_queue(q);
>   }
>   mutex_unlock(&ctrl->namespaces_mutex);

Hey Bart, should nvme_stop_queues() really be resuming the blk queue?



--
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: [PATCHv2] MAINTAINERS: Update open-iscsi maintainers

[Lee Duncan]

Hello Martin:

On 09/26/2016 06:26 PM, Martin K. Petersen wrote:
>> "Lee" == Lee Duncan  writes:
> 
> Lee,
> 
> Lee> Chris Leech and I are taking over as open-iscsi maintainers.
> 
> Do you want me to queue the MAINTAINER update?

Yes, that would be great. Thank you.

> 
> Lee>  * Removed git repository, since code in tree
> 
> Is it your plan to go through the SCSI tree?
> 

Yes, the iscsi initiator kernel code updates have been going through the
Linux SCSI mailing list and repository for a while, now. The open-iscsi
kernel code git repository currently mentioned in the MAINTAINERS file
no longer exists.
-- 
Lee Duncan
--
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 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path

[Bart Van Assche]

On 09/27/2016 07:42 AM, Bart Van Assche wrote:

Jens, regarding non-blk-mq mode and q_usage_counter: do you prefer that
I rework patch 8/9 such that blk_quiesce_queue() and blk_resume_queue()
are only used in blk-mq mode or are you OK with adding a
blk_queue_enter() call in get_request() and a blk_queue_exit() call to
__blk_put_request()?


(replying to my own e-mail)

Although it is easy to make q_usage_counter count non-blk-mq requests, 
extending the blk_quiesce_queue() waiting mechanism to the non-blk-mq 
path is non-trivial. To limit the number of changes in this patch series 
I will drop the non-blk-mq changes from this patch series.


Bart.
--
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: [Question] Calling request_firmware under the spinlocks in file advansys.c

[Hannes Reinecke]

On 09/27/2016 01:26 PM, Vaishali Thakkar wrote:
> 
> 
> On Tuesday 13 September 2016 02:48 PM, Vaishali Thakkar wrote:
>> Hi,
>>
>> In the file drivers/scsi/advansys.c we are calling function AdvISR at 2 
>> instances
>> [in the function advansys_reset and advansys_interrupt] while holding 
>> spinlock.
>> Function AdvISR eventually calls request_firmware following this sequence of
>> routines: 
>>
>> AdvISR -> adv_async_callback -> AdvResetChipAndSB -> AdvInitAsc3550Driver ->
>> request_firmware
>>
>> According to the definition of request_firmware it should be called from user
>> context where sleeping is allowed. And usually sleeping under the spin lock 
>> is
>> not allowed. Is it really necessary to call AdvISR under spinlocks here? Are
>> we taking care of sleeping related concern of request_firmware or am I
>> overlooking something here?
> 
> Hi,
> 
> Any comments on this?
> 
AdvISR is the main interrupt handling routine, for which we definitely
will want to have interupts disabled. Plus the SCSI parallel drivers
(ab-) use the interrupt routine to do all sorts of things, and are
thereby well versed to keep interrupts disabled for an extented amount
of time.
However, we don't really have a good way of handling a request firmware
here; we probably would need to implement a workqueue to handle this
properly ...

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-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 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path

[Bart Van Assche]

On 09/27/16 06:22, Ming Lei wrote:

On Tue, Sep 27, 2016 at 2:27 AM, Bart Van Assche
 wrote:

Signed-off-by: Bart Van Assche 
---
 block/blk-core.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 8cc8006..5ecc7ab 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -689,7 +689,10 @@ void blk_freeze_queue_start(struct request_queue *q)
freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
if (freeze_depth == 1) {
percpu_ref_kill(&q->q_usage_counter);
-   blk_mq_run_hw_queues(q, false);
+   if (q->mq_ops)
+   blk_mq_run_hw_queues(q, false);
+   else if (q->request_fn)
+   blk_run_queue(q);


Just wondering if you have a non-blk-mq drivers which need this change,
cause we only hold .q_usage_counter for sync bio.


Hello Ming Lei,

Patch 8/9 calls blk_quiesce_queue() and blk_resume_queue() from a code 
path that is used in both blk-mq and non-blk-mq mode. Although it 
wouldn't be hard to modify that patch such that it only uses these two 
functions in blk-mq mode, that wouldn't be very elegant.


Jens, regarding non-blk-mq mode and q_usage_counter: do you prefer that 
I rework patch 8/9 such that blk_quiesce_queue() and blk_resume_queue() 
are only used in blk-mq mode or are you OK with adding a 
blk_queue_enter() call in get_request() and a blk_queue_exit() call to 
__blk_put_request()?


Bart.
--
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 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path

[Ming Lei]

On Tue, Sep 27, 2016 at 2:27 AM, Bart Van Assche
 wrote:
> Signed-off-by: Bart Van Assche 
> ---
>  block/blk-core.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 8cc8006..5ecc7ab 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -689,7 +689,10 @@ void blk_freeze_queue_start(struct request_queue *q)
> freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
> if (freeze_depth == 1) {
> percpu_ref_kill(&q->q_usage_counter);
> -   blk_mq_run_hw_queues(q, false);
> +   if (q->mq_ops)
> +   blk_mq_run_hw_queues(q, false);
> +   else if (q->request_fn)
> +   blk_run_queue(q);

Just wondering if you have a non-blk-mq drivers which need this change,
cause we only hold .q_usage_counter for sync bio.

> }
>  }
>
> @@ -700,17 +703,11 @@ void blk_freeze_queue_wait(struct request_queue *q)
>
>  /*
>   * Guarantee no request is in use, so we can change any data structure of
> - * the queue afterward.
> + * the queue afterward. Increases q->mq_freeze_depth and waits until
> + * q->q_usage_counter drops to zero.
>   */
>  void blk_freeze_queue(struct request_queue *q)
>  {
> -   /*
> -* In the !blk_mq case we are only calling this to kill the
> -* q_usage_counter, otherwise this increases the freeze depth
> -* and waits for it to return to zero.  For this reason there is
> -* no blk_unfreeze_queue(), and blk_freeze_queue() is not
> -* exported to drivers as the only user for unfreeze is blk_mq.
> -*/
> blk_freeze_queue_start(q);
> blk_freeze_queue_wait(q);
>  }
> --
> 2.10.0
>
> --
> 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



-- 
Ming Lei
--
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 3/3] g_NCR5380: Stop using scsi_module.c

[Ondrej Zary]

On Monday 26 September 2016, Finn Thain wrote:

[...]

> Instead of this:
> > +   if (scsi_add_host(instance, pdev)) {
> > +   free_irq(irq, instance);
> > +   goto out_unregister;
> > +   }
> > +   scsi_scan_host(instance);
> > +   return instance;
>
> please use the following (note the missing NCR5380_exit() call):
>
> + if (scsi_add_host(instance, pdev))
> + goto out_free_irq;
> + scsi_scan_host(instance);
> + return(instance);
> +
> +out_free_irq:
> + if (instance->irq != NO_IRQ)
> + free_irq(instance->irq, instance);
> + NCR5380_exit(instance);
>
> >  out_unregister:
> > -   scsi_unregister(instance);
> > +   scsi_host_put(instance);
> >  out_release:
> >  #ifndef SCSI_G_NCR5380_MEM
> > -   release_region(card.NCR5380_map_name, region_size);
> > +   release_region(base, region_size);
> >  #else
> > iounmap(iomem);
> > release_mem_region(base, iomem_size);
> >  #endif
> > -   return 0;
> > +   return NULL;
> >  }
> >
> > -/**
> > - * generic_NCR5380_release_resources   -   free resources
> > - * @instance: host adapter to clean up
> > - *
> > - * Free the generic interface resources from this adapter.
> > - *
> > - * Locks: none
> > - */
> > -
> > -static int generic_NCR5380_release_resources(struct Scsi_Host *instance)
> > +static void generic_NCR5380_release_resources(struct Scsi_Host
> > *instance) {
> > +   scsi_remove_host(instance);
> > if (instance->irq != NO_IRQ)
> > free_irq(instance->irq, instance);
> > NCR5380_exit(instance);
> > @@ -364,7 +304,7 @@ static int generic_NCR5380_release_resources(struct
> > Scsi_Host *instance) release_mem_region(instance->base,
> > hostdata->iomem_size);
> > }
> >  #endif
> > -   return 0;
> > +   scsi_host_put(instance);
>
> The sequence of operations here should be the same as the error path
> above.

I put scsi_host_put() call at the end because the release_mem_region code (in 
the MMIO case) dereferences the hostdata pointer. I don't think it's safe to 
do after scsi_host_put().

[...]

> > +static int pnp_registered;
> > +#endif /* !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP) */
> > +
> > +static int __init generic_NCR5380_init(void)
> > +{
> > +   int ret = 0;
> > +
> > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > +   ret = pnp_register_driver(&generic_NCR5380_pnp_driver);
> > +   if (!ret)
> > +   pnp_registered = 1;
> >  #endif
> > +   ret = isa_register_driver(&generic_NCR5380_isa_driver, MAX_CARDS);
> > +   if (!ret)
> > +   isa_registered = 1;
> > +
> > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > +   if (pnp_registered)
> > +   ret = 0;
> > +#endif
> > +   if (isa_registered)
> > +   ret = 0;
> > +
> > +   return ret;
> > +}
> > +
> > +static void __exit generic_NCR5380_exit(void)
> > +{
> > +#if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
> > +   if (pnp_registered)
> > +   pnp_unregister_driver(&generic_NCR5380_pnp_driver);
> > +#endif
> > +   if (isa_registered)
> > +   isa_unregister_driver(&generic_NCR5380_isa_driver);
> > +}
> > +
>
> I find that hard to follow. This should be equivalent and simpler:
>
> static int __init generic_NCR5380_init(void)
> {
>   isa_ret = isa_register_driver(&generic_NCR5380_isa_driver, MAX_CARDS);
> #if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
>   pnp_ret = pnp_register_driver(&generic_NCR5380_pnp_driver);
>   return pnp_ret ? isa_ret : 0;
> #endif
>   return isa_ret;
> }
>
> static void __exit generic_NCR5380_exit(void)
> {
>   if (!isa_ret)
>   isa_unregister_driver(&generic_NCR5380_isa_driver);
> #if !defined(SCSI_G_NCR5380_MEM) && defined(CONFIG_PNP)
>   if (!pnp_ret)
>   pnp_unregister_driver(&generic_NCR5380_pnp_driver);
> #endif
> }

Doesn't make it any better, IMHO. Good that it's shorter but not cleaner - 
especially this:
>   return pnp_ret ? isa_ret : 0;

Also looking at the _exit function, meaning of isa_ret and pnp_ret is not 
obvious there.

Maybe we should have a module_isa_pnp_driver() macro.

-- 
Ondrej Zary
--
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: [Question] Calling request_firmware under the spinlocks in file advansys.c

[Vaishali Thakkar]

On Tuesday 13 September 2016 02:48 PM, Vaishali Thakkar wrote:
> Hi,
> 
> In the file drivers/scsi/advansys.c we are calling function AdvISR at 2 
> instances
> [in the function advansys_reset and advansys_interrupt] while holding 
> spinlock.
> Function AdvISR eventually calls request_firmware following this sequence of
> routines: 
> 
> AdvISR -> adv_async_callback -> AdvResetChipAndSB -> AdvInitAsc3550Driver ->
> request_firmware
> 
> According to the definition of request_firmware it should be called from user
> context where sleeping is allowed. And usually sleeping under the spin lock is
> not allowed. Is it really necessary to call AdvISR under spinlocks here? Are
> we taking care of sleeping related concern of request_firmware or am I
> overlooking something here?

Hi,

Any comments on this?

Thanks

> Thank you.
> 

-- 
Vaishali
--
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 02/28] be2iscsi: Replace _bh with _irqsave/irqrestore

[Jitendra Bhivare]

> -Original Message-
> From: Jitendra Bhivare [mailto:jitendra.bhiv...@broadcom.com]
> Sent: Friday, September 23, 2016 8:38 PM
> To: 'Mike Christie'; 'Martin K. Petersen'
> Cc: 'linux-scsi@vger.kernel.org'
> Subject: RE: [PATCH 02/28] be2iscsi: Replace _bh with _irqsave/irqrestore
>
> Hi Mike,
>
> I could reproduce hard lockup using for-next kernel only in
> iscsi_eh_cmd_timeout path due to spin_lock_irqsave taken in
> blk_timeout_work
> Please refer stack trace below.
>
> The _bh version used for frwd_lock and back_lock does not seem to be
> causing
> any issue similar to seen with be2iscsi after replacing _bh versions in
> be2iscsi.
>
> I am testing it further, to confirm in all possible scenarios... NOPs,
> error
> recovery, resets and reconnects.
> On my setup, I affined all EQ interrupts on a single CPU.
> Along with heavy IO, few of the invocations of fio were pinned to run on
> same
> CPU.
>
> Any call to unlock_bh with another spin_lock already held, invoking
> do_softirq,
> might cause deadlock if bottom half used by driver calls function which
> needs
> that another spin_lock.
> Is there a code which prevents this issue?
>
The only place, I think, libiscsi can have issues is__iscsi_complete_pdu
which should be protected under _bh version for frwd and back locks.
If the function is executed in process context there is a good chance the
base version of spin_lock used for frwd/back locks could cause deadlocks
when lld uses _bh version in alloc_pdu path
(__iscsi_complete_pdu->iscsi_send_nopout... alloc_pdu).

Thanks,

JB
--
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 4/9] block: Move blk_freeze_queue() and blk_unfreeze_queue() code

[Johannes Thumshirn]

On Tue, Sep 27, 2016 at 08:26:19AM +0200, Hannes Reinecke wrote:
> On 09/26/2016 08:27 PM, Bart Van Assche wrote:
> > Move the blk_freeze_queue() and blk_unfreeze_queue() implementations
> > from block/blk-mq.c to block/blk-core.c. Drop "_mq" from the name of
> > the functions that have been moved.
> > 
> > Signed-off-by: Bart Van Assche 
> > ---
> >  block/blk-core.c | 45 +
> >  block/blk-mq.c   | 41 +++--
> >  block/blk.h  |  3 +++
> >  3 files changed, 51 insertions(+), 38 deletions(-)
> > 
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index b75d688..8cc8006 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -682,6 +682,51 @@ static void blk_queue_usage_counter_release(struct 
> > percpu_ref *ref)
> > wake_up_all(&q->mq_freeze_wq);
> >  }
> >  
> > +void blk_freeze_queue_start(struct request_queue *q)
> > +{
> > +   int freeze_depth;
> > +
> > +   freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
> > +   if (freeze_depth == 1) {
> > +   percpu_ref_kill(&q->q_usage_counter);
> > +   blk_mq_run_hw_queues(q, false);
> > +   }
> > +}
> > +
> As you dropped the 'mq_' prefix, maybe you should rename the counter to
> 'freeze_depth', to?

See PATCH 6/9

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path

[Johannes Thumshirn]

On Mon, Sep 26, 2016 at 11:27:49AM -0700, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] block: Rename mq_freeze_wq and mq_freeze_depth

[Johannes Thumshirn]

On Mon, Sep 26, 2016 at 11:28:08AM -0700, Bart Van Assche wrote:
> Since these two structure members are now used in blk-mq and !blk-mq
> paths, remove the mq_prefix. This patch does not change any
> functionality.
> 
> Signed-off-by: Bart Van Assche 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] dm: Fix a race condition related to stopping and starting queues

[Johannes Thumshirn]

On Mon, Sep 26, 2016 at 11:26:50AM -0700, Bart Van Assche wrote:
> Ensure that all ongoing dm_mq_queue_rq() and dm_mq_requeue_request()
> calls have stopped before setting the "queue stopped" flag. This
> allows to remove the "queue stopped" test from dm_mq_queue_rq() and
> dm_mq_requeue_request(). This patch fixes a race condition because
> dm_mq_queue_rq() is called without holding the queue lock and hence
> BLK_MQ_S_STOPPED can be set at any time while dm_mq_queue_rq() is
> in progress.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Mike Snitzer 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] blk-mq: Introduce blk_mq_queue_stopped()

[Johannes Thumshirn]

On Mon, Sep 26, 2016 at 11:26:26AM -0700, Bart Van Assche wrote:
> The function blk_queue_stopped() allows to test whether or not a
> traditional request queue has been stopped. Introduce a helper
> function that allows block drivers to query easily whether or not
> one or more hardware contexts of a blk-mq queue have been stopped.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html