* [PATCH 1/7] scsi: target: Rename target_configure_unmap_from_queue
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2025-05-14 11:11 ` John Garry
2024-10-08 2:03 ` [PATCH 2/7] scsi: target: Add atomic se_device fields Mike Christie
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
Rename target_configure_unmap_from_queue to
target_configure_unmap_from_bdev since it now takes a bdev.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_device.c | 6 +++---
drivers/target/target_core_file.c | 4 ++--
drivers/target/target_core_iblock.c | 4 ++--
include/target/target_core_backend.h | 4 ++--
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 7d43d92c44d4..b709a64c0ff3 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -797,8 +797,8 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
* Check if the underlying struct block_device supports discard and if yes
* configure the UNMAP parameters.
*/
-bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
- struct block_device *bdev)
+bool target_configure_unmap_from_bdev(struct se_dev_attrib *attrib,
+ struct block_device *bdev)
{
int block_size = bdev_logical_block_size(bdev);
@@ -816,7 +816,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
bdev_discard_alignment(bdev) / block_size;
return true;
}
-EXPORT_SYMBOL(target_configure_unmap_from_queue);
+EXPORT_SYMBOL(target_configure_unmap_from_bdev);
/*
* Convert from blocksize advertised to the initiator to the 512 byte
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 2d78ef74633c..b2610073e8cc 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -92,8 +92,8 @@ static bool fd_configure_unmap(struct se_device *dev)
struct inode *inode = file->f_mapping->host;
if (S_ISBLK(inode->i_mode))
- return target_configure_unmap_from_queue(&dev->dev_attrib,
- I_BDEV(inode));
+ return target_configure_unmap_from_bdev(&dev->dev_attrib,
+ I_BDEV(inode));
/* Limit UNMAP emulation to 8k Number of LBAs (NoLB) */
dev->dev_attrib.max_unmap_lba_count = 0x2000;
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index c8dc92a7d63e..3a191bc35e04 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -83,8 +83,8 @@ static bool iblock_configure_unmap(struct se_device *dev)
{
struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
- return target_configure_unmap_from_queue(&dev->dev_attrib,
- ib_dev->ibd_bd);
+ return target_configure_unmap_from_bdev(&dev->dev_attrib,
+ ib_dev->ibd_bd);
}
static int iblock_configure_device(struct se_device *dev)
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index 4063a701081b..d394306f8f49 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -121,8 +121,8 @@ sense_reason_t passthrough_parse_cdb(struct se_cmd *cmd,
bool target_sense_desc_format(struct se_device *dev);
sector_t target_to_linux_sector(struct se_device *dev, sector_t lb);
-bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
- struct block_device *bdev);
+bool target_configure_unmap_from_bdev(struct se_dev_attrib *attrib,
+ struct block_device *bdev);
static inline bool target_dev_configured(struct se_device *se_dev)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 2/7] scsi: target: Add atomic se_device fields
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
2024-10-08 2:03 ` [PATCH 1/7] scsi: target: Rename target_configure_unmap_from_queue Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2025-05-14 10:26 ` John Garry
2024-10-08 2:03 ` [PATCH 3/7] scsi: target: Add helper to setup atomic values from block_device Mike Christie
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This adds atomic fields to the se_device and exports them in configfs.
Initially only target_core_iblock will be supported and we will inherit
all the settings from the block layer except the alignment which userspace
will set.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_configfs.c | 42 +++++++++++++++++++++++++++
include/target/target_core_base.h | 6 ++++
2 files changed, 48 insertions(+)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..f3c7da650f65 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
+DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
#define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
static ssize_t _name##_store(struct config_item *item, const char *page,\
@@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct config_item *item, const char *page,
return count;
}
+static ssize_t atomic_alignment_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct se_dev_attrib *da = to_attrib(item);
+ u32 val;
+ int ret;
+
+ ret = kstrtou32(page, 0, &val);
+ if (ret < 0)
+ return ret;
+
+ if (da->da_dev->export_count) {
+ pr_err("dev[%p]: Unable to change SE Device atomic_alignment while export_count is %d\n",
+ da->da_dev, da->da_dev->export_count);
+ return -EINVAL;
+ }
+
+ da->atomic_alignment = val;
+
+ pr_debug("dev[%p]: SE Device atomic_alignment changed to %u\n",
+ da->da_dev, val);
+ return count;
+}
+
CONFIGFS_ATTR(, emulate_model_alias);
CONFIGFS_ATTR(, emulate_dpo);
CONFIGFS_ATTR(, emulate_fua_write);
@@ -1302,6 +1332,12 @@ CONFIGFS_ATTR(, max_write_same_len);
CONFIGFS_ATTR(, alua_support);
CONFIGFS_ATTR(, pgr_support);
CONFIGFS_ATTR(, submit_type);
+CONFIGFS_ATTR(, atomic_alignment);
+CONFIGFS_ATTR_RO(, atomic_supported);
+CONFIGFS_ATTR_RO(, atomic_max_len);
+CONFIGFS_ATTR_RO(, atomic_granularity);
+CONFIGFS_ATTR_RO(, atomic_max_with_boundary);
+CONFIGFS_ATTR_RO(, atomic_max_boundary);
/*
* dev_attrib attributes for devices using the target core SBC/SPC
@@ -1345,6 +1381,12 @@ struct configfs_attribute *sbc_attrib_attrs[] = {
&attr_pgr_support,
&attr_emulate_rsoc,
&attr_submit_type,
+ &attr_atomic_supported,
+ &attr_atomic_alignment,
+ &attr_atomic_max_len,
+ &attr_atomic_granularity,
+ &attr_atomic_max_with_boundary,
+ &attr_atomic_max_boundary,
NULL,
};
EXPORT_SYMBOL(sbc_attrib_attrs);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 97099a5e3f6c..6c87bd018983 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -715,6 +715,7 @@ struct se_dev_attrib {
bool is_nonrot;
bool emulate_rest_reord;
bool unmap_zeroes_data;
+ bool atomic_supported;
u32 hw_block_size;
u32 block_size;
u32 hw_max_sectors;
@@ -726,6 +727,11 @@ struct se_dev_attrib {
u32 unmap_granularity;
u32 unmap_granularity_alignment;
u32 max_write_same_len;
+ u32 atomic_max_len;
+ u32 atomic_alignment;
+ u32 atomic_granularity;
+ u32 atomic_max_with_boundary;
+ u32 atomic_max_boundary;
u8 submit_type;
struct se_device *da_dev;
struct config_group da_group;
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2024-10-08 2:03 ` [PATCH 2/7] scsi: target: Add atomic se_device fields Mike Christie
@ 2025-05-14 10:26 ` John Garry
2025-07-20 22:15 ` Mike Christie
0 siblings, 1 reply; 23+ messages in thread
From: John Garry @ 2025-05-14 10:26 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
On 08/10/2024 03:03, Mike Christie wrote:
> This adds atomic fields to the se_device and exports them in configfs.
>
> Initially only target_core_iblock will be supported and we will inherit
> all the settings from the block layer except the alignment which userspace
> will set.
>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
> drivers/target/target_core_configfs.c | 42 +++++++++++++++++++++++++++
> include/target/target_core_base.h | 6 ++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index c40217f44b1b..f3c7da650f65 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
> DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
> DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
> DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
>
> #define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
> static ssize_t _name##_store(struct config_item *item, const char *page,\
> @@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct config_item *item, const char *page,
> return count;
> }
>
> +static ssize_t atomic_alignment_store(struct config_item *item,
> + const char *page, size_t count)
What is special about alignment that we need to be able to configure this?
Won't that be derived from the block device queue_limits (like granularity)?
> +{
> + struct se_dev_attrib *da = to_attrib(item);
> + u32 val;
> + int ret;
> +
> + ret = kstrtou32(page, 0, &val);
> + if (ret < 0)
> + return ret;
> +
> + if (da->da_dev->export_count) {
> + pr_err("dev[%p]: Unable to change SE Device atomic_alignment while export_count is %d\n",
> + da->da_dev, da->da_dev->export_count);
> + return -EINVAL;
> + }
> +
> + da->atomic_alignment = val;
> +
> + pr_debug("dev[%p]: SE Device atomic_alignment changed to %u\n",
> + da->da_dev, val);
> + return count;
> +}
> +
> CONFIGFS_ATTR(, emulate_model_alias);
> CONFIGFS_ATTR(, emulate_dpo);
> CONFIGFS_ATTR(, emulate_fua_write);
> @@ -1302,6 +1332,12 @@ CONFIGFS_ATTR(, max_write_same_len);
> CONFIGFS_ATTR(, alua_support);
> CONFIGFS_ATTR(, pgr_support);
> CONFIGFS_ATTR(, submit_type);
> +CONFIGFS_ATTR(, atomic_alignment);
> +CONFIGFS_ATTR_RO(, atomic_supported);
isn't a non-zero atomic_max_len the same thing as this?
> +CONFIGFS_ATTR_RO(, atomic_max_len);
> +CONFIGFS_ATTR_RO(, atomic_granularity);
> +CONFIGFS_ATTR_RO(, atomic_max_with_boundary);
> +CONFIGFS_ATTR_RO(, atomic_max_boundary);
>
> /*
> * dev_attrib attributes for devices using the target core SBC/SPC
> @@ -1345,6 +1381,12 @@ struct configfs_attribute *sbc_attrib_attrs[] = {
> &attr_pgr_support,
> &attr_emulate_rsoc,
> &attr_submit_type,
> + &attr_atomic_supported,
> + &attr_atomic_alignment,
> + &attr_atomic_max_len,
> + &attr_atomic_granularity,
> + &attr_atomic_max_with_boundary,
> + &attr_atomic_max_boundary,
> NULL,
> };
> EXPORT_SYMBOL(sbc_attrib_attrs);
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 97099a5e3f6c..6c87bd018983 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -715,6 +715,7 @@ struct se_dev_attrib {
> bool is_nonrot;
> bool emulate_rest_reord;
> bool unmap_zeroes_data;
> + bool atomic_supported;
> u32 hw_block_size;
> u32 block_size;
> u32 hw_max_sectors;
> @@ -726,6 +727,11 @@ struct se_dev_attrib {
> u32 unmap_granularity;
> u32 unmap_granularity_alignment;
> u32 max_write_same_len;
> + u32 atomic_max_len;
> + u32 atomic_alignment;
> + u32 atomic_granularity;
> + u32 atomic_max_with_boundary;
> + u32 atomic_max_boundary;
these will always be zero, right? I don't see much point in even storing
them.
> u8 submit_type;
> struct se_device *da_dev;
> struct config_group da_group;
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-05-14 10:26 ` John Garry
@ 2025-07-20 22:15 ` Mike Christie
2025-07-23 11:28 ` John Garry
0 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2025-07-20 22:15 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 5/14/25 5:26 AM, John Garry wrote:
> On 08/10/2024 03:03, Mike Christie wrote:
>> This adds atomic fields to the se_device and exports them in configfs.
>>
>> Initially only target_core_iblock will be supported and we will inherit
>> all the settings from the block layer except the alignment which userspace
>> will set.
>>
>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>> ---
>> drivers/target/target_core_configfs.c | 42 +++++++++++++++++++++++++++
>> include/target/target_core_base.h | 6 ++++
>> 2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
>> index c40217f44b1b..f3c7da650f65 100644
>> --- a/drivers/target/target_core_configfs.c
>> +++ b/drivers/target/target_core_configfs.c
>> @@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
>> DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
>> DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
>> DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
>> #define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
>> static ssize_t _name##_store(struct config_item *item, const char *page,\
>> @@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct config_item *item, const char *page,
>> return count;
>> }
>> +static ssize_t atomic_alignment_store(struct config_item *item,
>> + const char *page, size_t count)
>
> What is special about alignment that we need to be able to configure this?
>
> Won't that be derived from the block device queue_limits (like granularity)?
What if you are not using a physical block device like using LIO's ramdisk mode
to perform testing?
>
>> +{
>> + struct se_dev_attrib *da = to_attrib(item);
>> + u32 val;
>> + int ret;
>> +
>> + ret = kstrtou32(page, 0, &val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (da->da_dev->export_count) {
>> + pr_err("dev[%p]: Unable to change SE Device atomic_alignment while export_count is %d\n",
>> + da->da_dev, da->da_dev->export_count);
>> + return -EINVAL;
>> + }
>> +
>> + da->atomic_alignment = val;
>> +
>> + pr_debug("dev[%p]: SE Device atomic_alignment changed to %u\n",
>> + da->da_dev, val);
>> + return count;
>> +}
>> +
>> CONFIGFS_ATTR(, emulate_model_alias);
>> CONFIGFS_ATTR(, emulate_dpo);
>> CONFIGFS_ATTR(, emulate_fua_write);
>> @@ -1302,6 +1332,12 @@ CONFIGFS_ATTR(, max_write_same_len);
>> CONFIGFS_ATTR(, alua_support);
>> CONFIGFS_ATTR(, pgr_support);
>> CONFIGFS_ATTR(, submit_type);
>> +CONFIGFS_ATTR(, atomic_alignment);
>> +CONFIGFS_ATTR_RO(, atomic_supported);
>
> isn't a non-zero atomic_max_len the same thing as this?
I separated it because like above I was thinking maybe users will want to
use this to do some testing. So I tried to make it more generic.
>
>> +CONFIGFS_ATTR_RO(, atomic_max_len);
>> +CONFIGFS_ATTR_RO(, atomic_granularity);
>> +CONFIGFS_ATTR_RO(, atomic_max_with_boundary);
>> +CONFIGFS_ATTR_RO(, atomic_max_boundary);
>> /*
>> * dev_attrib attributes for devices using the target core SBC/SPC
>> @@ -1345,6 +1381,12 @@ struct configfs_attribute *sbc_attrib_attrs[] = {
>> &attr_pgr_support,
>> &attr_emulate_rsoc,
>> &attr_submit_type,
>> + &attr_atomic_supported,
>> + &attr_atomic_alignment,
>> + &attr_atomic_max_len,
>> + &attr_atomic_granularity,
>> + &attr_atomic_max_with_boundary,
>> + &attr_atomic_max_boundary,
>> NULL,
>> };
>> EXPORT_SYMBOL(sbc_attrib_attrs);
>> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
>> index 97099a5e3f6c..6c87bd018983 100644
>> --- a/include/target/target_core_base.h
>> +++ b/include/target/target_core_base.h
>> @@ -715,6 +715,7 @@ struct se_dev_attrib {
>> bool is_nonrot;
>> bool emulate_rest_reord;
>> bool unmap_zeroes_data;
>> + bool atomic_supported;
>> u32 hw_block_size;
>> u32 block_size;
>> u32 hw_max_sectors;
>> @@ -726,6 +727,11 @@ struct se_dev_attrib {
>> u32 unmap_granularity;
>> u32 unmap_granularity_alignment;
>> u32 max_write_same_len;
>> + u32 atomic_max_len;
>> + u32 atomic_alignment;
>> + u32 atomic_granularity;
>> + u32 atomic_max_with_boundary;
>> + u32 atomic_max_boundary;
>
> these will always be zero, right? I don't see much point in even storing them.
>
If you use target_core_iblock yes. If you use some other backend then
it will be what's set in configfs.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-07-20 22:15 ` Mike Christie
@ 2025-07-23 11:28 ` John Garry
2025-07-23 15:10 ` michael.christie
0 siblings, 1 reply; 23+ messages in thread
From: John Garry @ 2025-07-23 11:28 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
On 20/07/2025 23:15, Mike Christie wrote:
> On 5/14/25 5:26 AM, John Garry wrote:
>> On 08/10/2024 03:03, Mike Christie wrote:
>>> This adds atomic fields to the se_device and exports them in configfs.
>>>
>>> Initially only target_core_iblock will be supported and we will inherit
>>> all the settings from the block layer except the alignment which userspace
>>> will set.
>>>
>>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>>> ---
>>> drivers/target/target_core_configfs.c | 42 +++++++++++++++++++++++++++
>>> include/target/target_core_base.h | 6 ++++
>>> 2 files changed, 48 insertions(+)
>>>
>>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
>>> index c40217f44b1b..f3c7da650f65 100644
>>> --- a/drivers/target/target_core_configfs.c
>>> +++ b/drivers/target/target_core_configfs.c
>>> @@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
>>> DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
>>> DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
>>> DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
>>> #define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
>>> static ssize_t _name##_store(struct config_item *item, const char *page,\
>>> @@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct config_item *item, const char *page,
>>> return count;
>>> }
>>> +static ssize_t atomic_alignment_store(struct config_item *item,
>>> + const char *page, size_t count)
>>
>> What is special about alignment that we need to be able to configure this?
>>
>> Won't that be derived from the block device queue_limits (like granularity)?
>
> What if you are not using a physical block device like using LIO's ramdisk mode
> to perform testing?
But would this non-physical device still need to support atomics? I
would assume so.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-07-23 11:28 ` John Garry
@ 2025-07-23 15:10 ` michael.christie
2025-07-23 15:48 ` michael.christie
0 siblings, 1 reply; 23+ messages in thread
From: michael.christie @ 2025-07-23 15:10 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 7/23/25 6:28 AM, John Garry wrote:
> On 20/07/2025 23:15, Mike Christie wrote:
>> On 5/14/25 5:26 AM, John Garry wrote:
>>> On 08/10/2024 03:03, Mike Christie wrote:
>>>> This adds atomic fields to the se_device and exports them in configfs.
>>>>
>>>> Initially only target_core_iblock will be supported and we will inherit
>>>> all the settings from the block layer except the alignment which
>>>> userspace
>>>> will set.
>>>>
>>>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>>>> ---
>>>> drivers/target/target_core_configfs.c | 42 ++++++++++++++++++++++
>>>> +++++
>>>> include/target/target_core_base.h | 6 ++++
>>>> 2 files changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/
>>>> target_core_configfs.c
>>>> index c40217f44b1b..f3c7da650f65 100644
>>>> --- a/drivers/target/target_core_configfs.c
>>>> +++ b/drivers/target/target_core_configfs.c
>>>> @@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
>>>> DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
>>>> DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
>>>> DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
>>>> #define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
>>>> static ssize_t _name##_store(struct config_item *item, const char
>>>> *page,\
>>>> @@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct
>>>> config_item *item, const char *page,
>>>> return count;
>>>> }
>>>> +static ssize_t atomic_alignment_store(struct config_item *item,
>>>> + const char *page, size_t count)
>>>
>>> What is special about alignment that we need to be able to configure
>>> this?
>>>
>>> Won't that be derived from the block device queue_limits (like
>>> granularity)?
>>
>> What if you are not using a physical block device like using LIO's
>> ramdisk mode
>> to perform testing?
>
> But would this non-physical device still need to support atomics? I
> would assume so.
It depends. For just a testing mode not necessarily. It can be relaxed
so you can test different scenarios like you want to test when atomics
are not supported correctly. For real support yes.
But just to be clear for either case, there won't always be a
request_queue/block_device. For example, rd and tcmu don't use
target_configure_write_atomic_from_bdev, so the atomic values can be
whatever they want to support.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-07-23 15:10 ` michael.christie
@ 2025-07-23 15:48 ` michael.christie
2025-07-23 16:21 ` John Garry
0 siblings, 1 reply; 23+ messages in thread
From: michael.christie @ 2025-07-23 15:48 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 7/23/25 10:10 AM, michael.christie@oracle.com wrote:
> On 7/23/25 6:28 AM, John Garry wrote:
>> On 20/07/2025 23:15, Mike Christie wrote:
>>> On 5/14/25 5:26 AM, John Garry wrote:
>>>> On 08/10/2024 03:03, Mike Christie wrote:
>>>>> This adds atomic fields to the se_device and exports them in configfs.
>>>>>
>>>>> Initially only target_core_iblock will be supported and we will inherit
>>>>> all the settings from the block layer except the alignment which
>>>>> userspace
>>>>> will set.
>>>>>
>>>>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>>>>> ---
>>>>> drivers/target/target_core_configfs.c | 42 ++++++++++++++++++++++
>>>>> +++++
>>>>> include/target/target_core_base.h | 6 ++++
>>>>> 2 files changed, 48 insertions(+)
>>>>>
>>>>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/
>>>>> target_core_configfs.c
>>>>> index c40217f44b1b..f3c7da650f65 100644
>>>>> --- a/drivers/target/target_core_configfs.c
>>>>> +++ b/drivers/target/target_core_configfs.c
>>>>> @@ -578,6 +578,12 @@ DEF_CONFIGFS_ATTRIB_SHOW(unmap_zeroes_data);
>>>>> DEF_CONFIGFS_ATTRIB_SHOW(max_write_same_len);
>>>>> DEF_CONFIGFS_ATTRIB_SHOW(emulate_rsoc);
>>>>> DEF_CONFIGFS_ATTRIB_SHOW(submit_type);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_supported);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_alignment);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_len);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_granularity);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_with_boundary);
>>>>> +DEF_CONFIGFS_ATTRIB_SHOW(atomic_max_boundary);
>>>>> #define DEF_CONFIGFS_ATTRIB_STORE_U32(_name) \
>>>>> static ssize_t _name##_store(struct config_item *item, const char
>>>>> *page,\
>>>>> @@ -1266,6 +1272,30 @@ static ssize_t submit_type_store(struct
>>>>> config_item *item, const char *page,
>>>>> return count;
>>>>> }
>>>>> +static ssize_t atomic_alignment_store(struct config_item *item,
>>>>> + const char *page, size_t count)
>>>>
>>>> What is special about alignment that we need to be able to configure
>>>> this?
>>>>
>>>> Won't that be derived from the block device queue_limits (like
>>>> granularity)?
>>>
>>> What if you are not using a physical block device like using LIO's
>>> ramdisk mode
>>> to perform testing?
>>
>> But would this non-physical device still need to support atomics? I
>> would assume so.
>
> It depends. For just a testing mode not necessarily. It can be relaxed
> so you can test different scenarios like you want to test when atomics
> are not supported correctly. For real support yes.
>
> But just to be clear for either case, there won't always be a
> request_queue/block_device. For example, rd and tcmu don't use
> target_configure_write_atomic_from_bdev, so the atomic values can be
> whatever they want to support.
I think I misunderstood the last comment.
I'm trying to make sure I can support a broken device for testing so I
tried to make as flexible as possible.
With the code as is though I think your concern in the last comment is
that a user could set some valid settings, use rd/tcmu, and think they
were safe when they are not. If so, then I agree that can happen.
Or are you saying even for broken device simulation that we will never
need to set values like alignment above?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-07-23 15:48 ` michael.christie
@ 2025-07-23 16:21 ` John Garry
2025-07-23 19:38 ` michael.christie
0 siblings, 1 reply; 23+ messages in thread
From: John Garry @ 2025-07-23 16:21 UTC (permalink / raw)
To: michael.christie, martin.petersen, linux-scsi, target-devel
On 23/07/2025 16:48, michael.christie@oracle.com wrote:
>>>> What if you are not using a physical block device like using LIO's
>>>> ramdisk mode
>>>> to perform testing?
>>> But would this non-physical device still need to support atomics? I
>>> would assume so.
>> It depends. For just a testing mode not necessarily. It can be relaxed
>> so you can test different scenarios like you want to test when atomics
>> are not supported correctly. For real support yes.
>>
>> But just to be clear for either case, there won't always be a
>> request_queue/block_device. For example, rd and tcmu don't use
>> target_configure_write_atomic_from_bdev, so the atomic values can be
>> whatever they want to support.
If that is acceptable, then I am curious why only make alignment
configurable (and not the other params)? From below, it seems that you
have some special device to support.
> I think I misunderstood the last comment.
>
> I'm trying to make sure I can support a broken device for testing so I
> tried to make as flexible as possible.
>
> With the code as is though I think your concern in the last comment is
> that a user could set some valid settings, use rd/tcmu, and think they
> were safe when they are not. If so, then I agree that can happen.
>
> Or are you saying even for broken device simulation that we will never
> need to set values like alignment above?
I suppose that we can set alignment for broken devices, yes.
I'll make a couple of points about scsi alignment as it is a bit
special. Firstly the alignment should be compatible with the atomic
write unit min. atomic write unit min comes from the granularity/pbs. So
the granularity/pbs needs to be compatible with alignment.
A further point is that if a partition is not aligned on the atomic
write unit min boundary, then atomic writes are disabled for that bdev
partition.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] scsi: target: Add atomic se_device fields
2025-07-23 16:21 ` John Garry
@ 2025-07-23 19:38 ` michael.christie
0 siblings, 0 replies; 23+ messages in thread
From: michael.christie @ 2025-07-23 19:38 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 7/23/25 11:21 AM, John Garry wrote:
> On 23/07/2025 16:48, michael.christie@oracle.com wrote:
>>>>> What if you are not using a physical block device like using LIO's
>>>>> ramdisk mode
>>>>> to perform testing?
>>>> But would this non-physical device still need to support atomics? I
>>>> would assume so.
>>> It depends. For just a testing mode not necessarily. It can be relaxed
>>> so you can test different scenarios like you want to test when atomics
>>> are not supported correctly. For real support yes.
>>>
>>> But just to be clear for either case, there won't always be a
>>> request_queue/block_device. For example, rd and tcmu don't use
>>> target_configure_write_atomic_from_bdev, so the atomic values can be
>>> whatever they want to support.
>
> If that is acceptable, then I am curious why only make alignment
> configurable (and not the other params)? From below, it seems that you
> have some special device to support.
>
Oh I completely misunderstood you :) I see what you were asking. I think
it was just a mistake.
>> I think I misunderstood the last comment.
>>
>> I'm trying to make sure I can support a broken device for testing so I
>> tried to make as flexible as possible.
>>
>> With the code as is though I think your concern in the last comment is
>> that a user could set some valid settings, use rd/tcmu, and think they
>> were safe when they are not. If so, then I agree that can happen.
>>
>> Or are you saying even for broken device simulation that we will never
>> need to set values like alignment above?
>
> I suppose that we can set alignment for broken devices, yes.
>
> I'll make a couple of points about scsi alignment as it is a bit
> special. Firstly the alignment should be compatible with the atomic
> write unit min. atomic write unit min comes from the granularity/pbs. So
> the granularity/pbs needs to be compatible with alignment.
>
> A further point is that if a partition is not aligned on the atomic
> write unit min boundary, then atomic writes are disabled for that bdev
> partition.
I didn't completely understand these 2 paragraphs. Are you saying you
want me to enforce this type of thing somewhere. How does the info above
come into play if we are testing against other OSs for example?
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/7] scsi: target: Add helper to setup atomic values from block_device
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
2024-10-08 2:03 ` [PATCH 1/7] scsi: target: Rename target_configure_unmap_from_queue Mike Christie
2024-10-08 2:03 ` [PATCH 2/7] scsi: target: Add atomic se_device fields Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2025-05-14 10:29 ` John Garry
2024-10-08 2:03 ` [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler Mike Christie
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This adds a helper function that sets up the atomic value based on a
block_device similar to what we do for unmap.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_device.c | 18 ++++++++++++++++++
include/target/target_core_backend.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index b709a64c0ff3..4741de3d1061 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -793,6 +793,24 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
return dev;
}
+void target_configure_write_atomic_from_bdev(struct se_dev_attrib *attrib,
+ struct block_device *bdev)
+{
+ struct request_queue *q = bdev_get_queue(bdev);
+ int block_size = bdev_logical_block_size(bdev);
+
+ if (!bdev_can_atomic_write(bdev))
+ return;
+
+ attrib->atomic_supported = true;
+ attrib->atomic_max_len = queue_atomic_write_max_bytes(q) / block_size;
+ attrib->atomic_granularity = queue_atomic_write_unit_min_bytes(q) /
+ block_size;
+ attrib->atomic_max_with_boundary = 0;
+ attrib->atomic_max_boundary = 0;
+}
+EXPORT_SYMBOL_GPL(target_configure_write_atomic_from_bdev);
+
/*
* Check if the underlying struct block_device supports discard and if yes
* configure the UNMAP parameters.
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index d394306f8f49..e32de80854b6 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -123,6 +123,8 @@ bool target_sense_desc_format(struct se_device *dev);
sector_t target_to_linux_sector(struct se_device *dev, sector_t lb);
bool target_configure_unmap_from_bdev(struct se_dev_attrib *attrib,
struct block_device *bdev);
+void target_configure_write_atomic_from_bdev(struct se_dev_attrib *attrib,
+ struct block_device *bdev);
static inline bool target_dev_configured(struct se_device *se_dev)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/7] scsi: target: Add helper to setup atomic values from block_device
2024-10-08 2:03 ` [PATCH 3/7] scsi: target: Add helper to setup atomic values from block_device Mike Christie
@ 2025-05-14 10:29 ` John Garry
0 siblings, 0 replies; 23+ messages in thread
From: John Garry @ 2025-05-14 10:29 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
On 08/10/2024 03:03, Mike Christie wrote:
> This adds a helper function that sets up the atomic value based on a
> block_device similar to what we do for unmap.
looks ok, apart from earlier query on alignment
>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
> drivers/target/target_core_device.c | 18 ++++++++++++++++++
> include/target/target_core_backend.h | 2 ++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index b709a64c0ff3..4741de3d1061 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -793,6 +793,24 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
> return dev;
> }
>
> +void target_configure_write_atomic_from_bdev(struct se_dev_attrib *attrib,
> + struct block_device *bdev)
> +{
> + struct request_queue *q = bdev_get_queue(bdev);
> + int block_size = bdev_logical_block_size(bdev);
> +
> + if (!bdev_can_atomic_write(bdev))
> + return;
> +
> + attrib->atomic_supported = true;
> + attrib->atomic_max_len = queue_atomic_write_max_bytes(q) / block_size;
> + attrib->atomic_granularity = queue_atomic_write_unit_min_bytes(q) /
> + block_size;
> + attrib->atomic_max_with_boundary = 0;
> + attrib->atomic_max_boundary = 0;
> +}
> +EXPORT_SYMBOL_GPL(target_configure_write_atomic_from_bdev);
> +
> /*
> * Check if the underlying struct block_device supports discard and if yes
> * configure the UNMAP parameters.
> diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
> index d394306f8f49..e32de80854b6 100644
> --- a/include/target/target_core_backend.h
> +++ b/include/target/target_core_backend.h
> @@ -123,6 +123,8 @@ bool target_sense_desc_format(struct se_device *dev);
> sector_t target_to_linux_sector(struct se_device *dev, sector_t lb);
> bool target_configure_unmap_from_bdev(struct se_dev_attrib *attrib,
> struct block_device *bdev);
> +void target_configure_write_atomic_from_bdev(struct se_dev_attrib *attrib,
> + struct block_device *bdev);
>
> static inline bool target_dev_configured(struct se_device *se_dev)
> {
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
` (2 preceding siblings ...)
2024-10-08 2:03 ` [PATCH 3/7] scsi: target: Add helper to setup atomic values from block_device Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2024-10-09 6:10 ` kernel test robot
2025-05-14 10:47 ` John Garry
2024-10-08 2:03 ` [PATCH 5/7] scsi: target: Report atomic values in INQUIRY Mike Christie
` (2 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This adds the core LIO code to process the WRITE_ATOMIC_16 command.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_sbc.c | 48 +++++++++++++++++++++++++++++++
include/target/target_core_base.h | 1 +
2 files changed, 49 insertions(+)
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index fe8beb7dbab1..0ae0d4ec0ac3 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -764,6 +764,46 @@ sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
return 0;
}
+static int
+sbc_check_atomic(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
+{
+ struct se_dev_attrib *attrib = &dev->dev_attrib;
+ u16 boundary, transfer_len;
+ u32 lba;
+
+ if (!attrib->atomic_supported)
+ return TCM_UNSUPPORTED_SCSI_OPCODE;
+
+ lba = transport_lba_32(cdb);
+ boundary = get_unaligned_be16(&cdb[10]);
+ transfer_len = get_unaligned_be16(&cdb[12]);
+
+ if (!boundary) {
+ if (transfer_len > attrib->atomic_max_len)
+ return TCM_INVALID_CDB_FIELD;
+ } else {
+ if (transfer_len > attrib->atomic_max_with_boundary)
+ return TCM_INVALID_CDB_FIELD;
+
+ if (boundary > attrib->atomic_max_boundary)
+ return TCM_INVALID_CDB_FIELD;
+ }
+
+ if (attrib->atomic_granularity) {
+ if (transfer_len % attrib->atomic_granularity)
+ return TCM_INVALID_CDB_FIELD;
+
+ if (boundary && boundary % attrib->atomic_granularity)
+ return TCM_INVALID_CDB_FIELD;
+ }
+
+ if (dev->dev_attrib.atomic_alignment &&
+ lba % dev->dev_attrib.atomic_alignment)
+ return TCM_INVALID_CDB_FIELD;
+
+ return 0;
+}
+
sense_reason_t
sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
{
@@ -861,6 +901,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
break;
case WRITE_16:
case WRITE_VERIFY_16:
+ case WRITE_ATOMIC_16:
sectors = transport_get_sectors_16(cdb);
cmd->t_task_lba = transport_lba_64(cdb);
@@ -872,6 +913,13 @@ sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
return ret;
cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
+ if (cdb[0] == WRITE_ATOMIC_16) {
+ cmd->se_cmd_flags |= SCF_ATOMIC;
+
+ ret = sbc_check_atomic(dev, cmd, cdb);
+ if (ret)
+ return ret;
+ }
cmd->execute_cmd = sbc_execute_rw;
break;
case VARIABLE_LENGTH_CMD:
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 6c87bd018983..7b8ed21bea03 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -157,6 +157,7 @@ enum se_cmd_flags_table {
SCF_USE_CPUID = (1 << 16),
SCF_TASK_ATTR_SET = (1 << 17),
SCF_TREAT_READ_AS_NORMAL = (1 << 18),
+ SCF_ATOMIC = (1 << 19),
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler
2024-10-08 2:03 ` [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler Mike Christie
@ 2024-10-09 6:10 ` kernel test robot
2025-05-14 10:47 ` John Garry
1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2024-10-09 6:10 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
Cc: oe-kbuild-all, Mike Christie
Hi Mike,
kernel test robot noticed the following build warnings:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.12-rc2 next-20241008]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mike-Christie/scsi-target-Rename-target_configure_unmap_from_queue/20241008-101218
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/20241008020437.78788-5-michael.christie%40oracle.com
patch subject: [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler
config: x86_64-randconfig-123-20241009 (https://download.01.org/0day-ci/archive/20241009/202410091340.dQ10fC3u-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091340.dQ10fC3u-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410091340.dQ10fC3u-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/target/target_core_sbc.c:775:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:775:24: sparse: expected int
drivers/target/target_core_sbc.c:775:24: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:783:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:783:32: sparse: expected int
drivers/target/target_core_sbc.c:783:32: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:786:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:786:32: sparse: expected int
drivers/target/target_core_sbc.c:786:32: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:789:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:789:32: sparse: expected int
drivers/target/target_core_sbc.c:789:32: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:794:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:794:32: sparse: expected int
drivers/target/target_core_sbc.c:794:32: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:797:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:797:32: sparse: expected int
drivers/target/target_core_sbc.c:797:32: sparse: got restricted sense_reason_t
drivers/target/target_core_sbc.c:802:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted sense_reason_t @@
drivers/target/target_core_sbc.c:802:24: sparse: expected int
drivers/target/target_core_sbc.c:802:24: sparse: got restricted sense_reason_t
>> drivers/target/target_core_sbc.c:919:29: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted sense_reason_t [assigned] [usertype] ret @@ got int @@
drivers/target/target_core_sbc.c:919:29: sparse: expected restricted sense_reason_t [assigned] [usertype] ret
drivers/target/target_core_sbc.c:919:29: sparse: got int
vim +775 drivers/target/target_core_sbc.c
766
767 static int
768 sbc_check_atomic(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
769 {
770 struct se_dev_attrib *attrib = &dev->dev_attrib;
771 u16 boundary, transfer_len;
772 u32 lba;
773
774 if (!attrib->atomic_supported)
> 775 return TCM_UNSUPPORTED_SCSI_OPCODE;
776
777 lba = transport_lba_32(cdb);
778 boundary = get_unaligned_be16(&cdb[10]);
779 transfer_len = get_unaligned_be16(&cdb[12]);
780
781 if (!boundary) {
782 if (transfer_len > attrib->atomic_max_len)
783 return TCM_INVALID_CDB_FIELD;
784 } else {
785 if (transfer_len > attrib->atomic_max_with_boundary)
786 return TCM_INVALID_CDB_FIELD;
787
788 if (boundary > attrib->atomic_max_boundary)
789 return TCM_INVALID_CDB_FIELD;
790 }
791
792 if (attrib->atomic_granularity) {
793 if (transfer_len % attrib->atomic_granularity)
794 return TCM_INVALID_CDB_FIELD;
795
796 if (boundary && boundary % attrib->atomic_granularity)
797 return TCM_INVALID_CDB_FIELD;
798 }
799
800 if (dev->dev_attrib.atomic_alignment &&
801 lba % dev->dev_attrib.atomic_alignment)
802 return TCM_INVALID_CDB_FIELD;
803
804 return 0;
805 }
806
807 sense_reason_t
808 sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
809 {
810 struct se_device *dev = cmd->se_dev;
811 unsigned char *cdb = cmd->t_task_cdb;
812 unsigned int size;
813 u32 sectors = 0;
814 sense_reason_t ret;
815
816 cmd->protocol_data = ops;
817
818 switch (cdb[0]) {
819 case READ_6:
820 sectors = transport_get_sectors_6(cdb);
821 cmd->t_task_lba = transport_lba_21(cdb);
822 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
823 cmd->execute_cmd = sbc_execute_rw;
824 break;
825 case READ_10:
826 sectors = transport_get_sectors_10(cdb);
827 cmd->t_task_lba = transport_lba_32(cdb);
828
829 if (sbc_check_dpofua(dev, cmd, cdb))
830 return TCM_INVALID_CDB_FIELD;
831
832 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, false);
833 if (ret)
834 return ret;
835
836 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
837 cmd->execute_cmd = sbc_execute_rw;
838 break;
839 case READ_12:
840 sectors = transport_get_sectors_12(cdb);
841 cmd->t_task_lba = transport_lba_32(cdb);
842
843 if (sbc_check_dpofua(dev, cmd, cdb))
844 return TCM_INVALID_CDB_FIELD;
845
846 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, false);
847 if (ret)
848 return ret;
849
850 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
851 cmd->execute_cmd = sbc_execute_rw;
852 break;
853 case READ_16:
854 sectors = transport_get_sectors_16(cdb);
855 cmd->t_task_lba = transport_lba_64(cdb);
856
857 if (sbc_check_dpofua(dev, cmd, cdb))
858 return TCM_INVALID_CDB_FIELD;
859
860 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, false);
861 if (ret)
862 return ret;
863
864 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
865 cmd->execute_cmd = sbc_execute_rw;
866 break;
867 case WRITE_6:
868 sectors = transport_get_sectors_6(cdb);
869 cmd->t_task_lba = transport_lba_21(cdb);
870 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
871 cmd->execute_cmd = sbc_execute_rw;
872 break;
873 case WRITE_10:
874 case WRITE_VERIFY:
875 sectors = transport_get_sectors_10(cdb);
876 cmd->t_task_lba = transport_lba_32(cdb);
877
878 if (sbc_check_dpofua(dev, cmd, cdb))
879 return TCM_INVALID_CDB_FIELD;
880
881 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, true);
882 if (ret)
883 return ret;
884
885 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
886 cmd->execute_cmd = sbc_execute_rw;
887 break;
888 case WRITE_12:
889 sectors = transport_get_sectors_12(cdb);
890 cmd->t_task_lba = transport_lba_32(cdb);
891
892 if (sbc_check_dpofua(dev, cmd, cdb))
893 return TCM_INVALID_CDB_FIELD;
894
895 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, true);
896 if (ret)
897 return ret;
898
899 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
900 cmd->execute_cmd = sbc_execute_rw;
901 break;
902 case WRITE_16:
903 case WRITE_VERIFY_16:
904 case WRITE_ATOMIC_16:
905 sectors = transport_get_sectors_16(cdb);
906 cmd->t_task_lba = transport_lba_64(cdb);
907
908 if (sbc_check_dpofua(dev, cmd, cdb))
909 return TCM_INVALID_CDB_FIELD;
910
911 ret = sbc_check_prot(dev, cmd, cdb[1] >> 5, sectors, true);
912 if (ret)
913 return ret;
914
915 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
916 if (cdb[0] == WRITE_ATOMIC_16) {
917 cmd->se_cmd_flags |= SCF_ATOMIC;
918
> 919 ret = sbc_check_atomic(dev, cmd, cdb);
920 if (ret)
921 return ret;
922 }
923 cmd->execute_cmd = sbc_execute_rw;
924 break;
925 case VARIABLE_LENGTH_CMD:
926 {
927 u16 service_action = get_unaligned_be16(&cdb[8]);
928 switch (service_action) {
929 case WRITE_SAME_32:
930 sectors = transport_get_sectors_32(cdb);
931 if (!sectors) {
932 pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not"
933 " supported\n");
934 return TCM_INVALID_CDB_FIELD;
935 }
936
937 size = sbc_get_size(cmd, 1);
938 cmd->t_task_lba = get_unaligned_be64(&cdb[12]);
939
940 ret = sbc_setup_write_same(cmd, cdb[10], ops);
941 if (ret)
942 return ret;
943 break;
944 default:
945 pr_err("VARIABLE_LENGTH_CMD service action"
946 " 0x%04x not supported\n", service_action);
947 return TCM_UNSUPPORTED_SCSI_OPCODE;
948 }
949 break;
950 }
951 case COMPARE_AND_WRITE:
952 if (!dev->dev_attrib.emulate_caw) {
953 pr_err_ratelimited("se_device %s/%s (vpd_unit_serial %s) reject COMPARE_AND_WRITE\n",
954 dev->se_hba->backend->ops->name,
955 config_item_name(&dev->dev_group.cg_item),
956 dev->t10_wwn.unit_serial);
957 return TCM_UNSUPPORTED_SCSI_OPCODE;
958 }
959 sectors = cdb[13];
960 /*
961 * Currently enforce COMPARE_AND_WRITE for a single sector
962 */
963 if (sectors > 1) {
964 pr_err("COMPARE_AND_WRITE contains NoLB: %u greater"
965 " than 1\n", sectors);
966 return TCM_INVALID_CDB_FIELD;
967 }
968 if (sbc_check_dpofua(dev, cmd, cdb))
969 return TCM_INVALID_CDB_FIELD;
970
971 /*
972 * Double size because we have two buffers, note that
973 * zero is not an error..
974 */
975 size = 2 * sbc_get_size(cmd, sectors);
976 cmd->t_task_lba = get_unaligned_be64(&cdb[2]);
977 cmd->t_task_nolb = sectors;
978 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB | SCF_COMPARE_AND_WRITE;
979 cmd->execute_cmd = sbc_compare_and_write;
980 cmd->transport_complete_callback = compare_and_write_callback;
981 break;
982 case READ_CAPACITY:
983 size = READ_CAP_LEN;
984 cmd->execute_cmd = sbc_emulate_readcapacity;
985 break;
986 case SERVICE_ACTION_IN_16:
987 switch (cmd->t_task_cdb[1] & 0x1f) {
988 case SAI_READ_CAPACITY_16:
989 cmd->execute_cmd = sbc_emulate_readcapacity_16;
990 break;
991 case SAI_REPORT_REFERRALS:
992 cmd->execute_cmd = target_emulate_report_referrals;
993 break;
994 default:
995 pr_err("Unsupported SA: 0x%02x\n",
996 cmd->t_task_cdb[1] & 0x1f);
997 return TCM_INVALID_CDB_FIELD;
998 }
999 size = get_unaligned_be32(&cdb[10]);
1000 break;
1001 case SYNCHRONIZE_CACHE:
1002 case SYNCHRONIZE_CACHE_16:
1003 if (cdb[0] == SYNCHRONIZE_CACHE) {
1004 sectors = transport_get_sectors_10(cdb);
1005 cmd->t_task_lba = transport_lba_32(cdb);
1006 } else {
1007 sectors = transport_get_sectors_16(cdb);
1008 cmd->t_task_lba = transport_lba_64(cdb);
1009 }
1010 if (ops->execute_sync_cache) {
1011 cmd->execute_cmd = ops->execute_sync_cache;
1012 goto check_lba;
1013 }
1014 size = 0;
1015 cmd->execute_cmd = sbc_emulate_noop;
1016 break;
1017 case UNMAP:
1018 if (!ops->execute_unmap)
1019 return TCM_UNSUPPORTED_SCSI_OPCODE;
1020
1021 if (!dev->dev_attrib.emulate_tpu) {
1022 pr_err("Got UNMAP, but backend device has"
1023 " emulate_tpu disabled\n");
1024 return TCM_UNSUPPORTED_SCSI_OPCODE;
1025 }
1026 size = get_unaligned_be16(&cdb[7]);
1027 cmd->execute_cmd = sbc_execute_unmap;
1028 break;
1029 case WRITE_SAME_16:
1030 sectors = transport_get_sectors_16(cdb);
1031 if (!sectors) {
1032 pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not supported\n");
1033 return TCM_INVALID_CDB_FIELD;
1034 }
1035
1036 size = sbc_get_size(cmd, 1);
1037 cmd->t_task_lba = get_unaligned_be64(&cdb[2]);
1038
1039 ret = sbc_setup_write_same(cmd, cdb[1], ops);
1040 if (ret)
1041 return ret;
1042 break;
1043 case WRITE_SAME:
1044 sectors = transport_get_sectors_10(cdb);
1045 if (!sectors) {
1046 pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not supported\n");
1047 return TCM_INVALID_CDB_FIELD;
1048 }
1049
1050 size = sbc_get_size(cmd, 1);
1051 cmd->t_task_lba = get_unaligned_be32(&cdb[2]);
1052
1053 /*
1054 * Follow sbcr26 with WRITE_SAME (10) and check for the existence
1055 * of byte 1 bit 3 UNMAP instead of original reserved field
1056 */
1057 ret = sbc_setup_write_same(cmd, cdb[1], ops);
1058 if (ret)
1059 return ret;
1060 break;
1061 case VERIFY:
1062 case VERIFY_16:
1063 size = 0;
1064 if (cdb[0] == VERIFY) {
1065 sectors = transport_get_sectors_10(cdb);
1066 cmd->t_task_lba = transport_lba_32(cdb);
1067 } else {
1068 sectors = transport_get_sectors_16(cdb);
1069 cmd->t_task_lba = transport_lba_64(cdb);
1070 }
1071 cmd->execute_cmd = sbc_emulate_noop;
1072 goto check_lba;
1073 case REZERO_UNIT:
1074 case SEEK_6:
1075 case SEEK_10:
1076 /*
1077 * There are still clients out there which use these old SCSI-2
1078 * commands. This mainly happens when running VMs with legacy
1079 * guest systems, connected via SCSI command pass-through to
1080 * iSCSI targets. Make them happy and return status GOOD.
1081 */
1082 size = 0;
1083 cmd->execute_cmd = sbc_emulate_noop;
1084 break;
1085 case START_STOP:
1086 size = 0;
1087 cmd->execute_cmd = sbc_emulate_startstop;
1088 break;
1089 default:
1090 ret = spc_parse_cdb(cmd, &size);
1091 if (ret)
1092 return ret;
1093 }
1094
1095 /* reject any command that we don't have a handler for */
1096 if (!cmd->execute_cmd)
1097 return TCM_UNSUPPORTED_SCSI_OPCODE;
1098
1099 if (cmd->se_cmd_flags & SCF_SCSI_DATA_CDB) {
1100 unsigned long long end_lba;
1101 check_lba:
1102 end_lba = dev->transport->get_blocks(dev) + 1;
1103 if (((cmd->t_task_lba + sectors) < cmd->t_task_lba) ||
1104 ((cmd->t_task_lba + sectors) > end_lba)) {
1105 pr_err("cmd exceeds last lba %llu "
1106 "(lba %llu, sectors %u)\n",
1107 end_lba, cmd->t_task_lba, sectors);
1108 return TCM_ADDRESS_OUT_OF_RANGE;
1109 }
1110
1111 if (!(cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE))
1112 size = sbc_get_size(cmd, sectors);
1113 }
1114
1115 return target_cmd_size_check(cmd, size);
1116 }
1117 EXPORT_SYMBOL(sbc_parse_cdb);
1118
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler
2024-10-08 2:03 ` [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler Mike Christie
2024-10-09 6:10 ` kernel test robot
@ 2025-05-14 10:47 ` John Garry
2025-07-20 22:44 ` Mike Christie
1 sibling, 1 reply; 23+ messages in thread
From: John Garry @ 2025-05-14 10:47 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
On 08/10/2024 03:03, Mike Christie wrote:
> This adds the core LIO code to process the WRITE_ATOMIC_16 command.
>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
> drivers/target/target_core_sbc.c | 48 +++++++++++++++++++++++++++++++
> include/target/target_core_base.h | 1 +
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index fe8beb7dbab1..0ae0d4ec0ac3 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -764,6 +764,46 @@ sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
> return 0;
> }
>
> +static int
> +sbc_check_atomic(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
> +{
> + struct se_dev_attrib *attrib = &dev->dev_attrib;
> + u16 boundary, transfer_len;
> + u32 lba;
> +
> + if (!attrib->atomic_supported)
> + return TCM_UNSUPPORTED_SCSI_OPCODE;
> +
> + lba = transport_lba_32(cdb);
should this be 64, not 32?
> + boundary = get_unaligned_be16(&cdb[10]);
> + transfer_len = get_unaligned_be16(&cdb[12]);
> +
> + if (!boundary) {
> + if (transfer_len > attrib->atomic_max_len)
> + return TCM_INVALID_CDB_FIELD;
from sbc4r22 4.29.1, the key is ILLEGAL_REQUEST and sense code is
INVALID CDB FIELD, but is it possible to have the field pointer set to
the TRANSFER LENGTH in the CDB?
> + } else {
earlier we have no boundary support, so we should reject any IO with a
boundary set, right? From a glance at sbc4r22 4.29.1, that's my take
> + if (transfer_len > attrib->atomic_max_with_boundary)
> + return TCM_INVALID_CDB_FIELD;
> +
> + if (boundary > attrib->atomic_max_boundary)
> + return TCM_INVALID_CDB_FIELD;
> + }
> +
> + if (attrib->atomic_granularity) {
> + if (transfer_len % attrib->atomic_granularity)
> + return TCM_INVALID_CDB_FIELD;
> +
> + if (boundary && boundary % attrib->atomic_granularity)
> + return TCM_INVALID_CDB_FIELD;
> + }
> +
> + if (dev->dev_attrib.atomic_alignment &&
> + lba % dev->dev_attrib.atomic_alignment)
> + return TCM_INVALID_CDB_FIELD;
> +
> + return 0;
> +}
> +
> sense_reason_t
> sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
> {
> @@ -861,6 +901,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
> break;
> case WRITE_16:
> case WRITE_VERIFY_16:
> + case WRITE_ATOMIC_16:
> sectors = transport_get_sectors_16(cdb);
> cmd->t_task_lba = transport_lba_64(cdb);
>
> @@ -872,6 +913,13 @@ sbc_parse_cdb(struct se_cmd *cmd, struct exec_cmd_ops *ops)
> return ret;
>
> cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
> + if (cdb[0] == WRITE_ATOMIC_16) {
> + cmd->se_cmd_flags |= SCF_ATOMIC;
> +
> + ret = sbc_check_atomic(dev, cmd, cdb);
> + if (ret)
> + return ret;
> + }
> cmd->execute_cmd = sbc_execute_rw;
> break;
> case VARIABLE_LENGTH_CMD:
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 6c87bd018983..7b8ed21bea03 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -157,6 +157,7 @@ enum se_cmd_flags_table {
> SCF_USE_CPUID = (1 << 16),
> SCF_TASK_ATTR_SET = (1 << 17),
> SCF_TREAT_READ_AS_NORMAL = (1 << 18),
> + SCF_ATOMIC = (1 << 19),
> };
>
> /*
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler
2025-05-14 10:47 ` John Garry
@ 2025-07-20 22:44 ` Mike Christie
0 siblings, 0 replies; 23+ messages in thread
From: Mike Christie @ 2025-07-20 22:44 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 5/14/25 5:47 AM, John Garry wrote:
> On 08/10/2024 03:03, Mike Christie wrote:
>> This adds the core LIO code to process the WRITE_ATOMIC_16 command.
>>
>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>> ---
>> drivers/target/target_core_sbc.c | 48 +++++++++++++++++++++++++++++++
>> include/target/target_core_base.h | 1 +
>> 2 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
>> index fe8beb7dbab1..0ae0d4ec0ac3 100644
>> --- a/drivers/target/target_core_sbc.c
>> +++ b/drivers/target/target_core_sbc.c
>> @@ -764,6 +764,46 @@ sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
>> return 0;
>> }
>> +static int
>> +sbc_check_atomic(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
>> +{
>> + struct se_dev_attrib *attrib = &dev->dev_attrib;
>> + u16 boundary, transfer_len;
>> + u32 lba;
>> +
>> + if (!attrib->atomic_supported)
>> + return TCM_UNSUPPORTED_SCSI_OPCODE;
>> +
>> + lba = transport_lba_32(cdb);
>
> should this be 64, not 32?
>
Yeah will fix.
>> + boundary = get_unaligned_be16(&cdb[10]);
>> + transfer_len = get_unaligned_be16(&cdb[12]);
>> +
>> + if (!boundary) {
>> + if (transfer_len > attrib->atomic_max_len)
>> + return TCM_INVALID_CDB_FIELD;
>
> from sbc4r22 4.29.1, the key is ILLEGAL_REQUEST and sense code is INVALID CDB FIELD, but is it possible to have the field pointer set to the TRANSFER LENGTH in the CDB?
I can. Will do.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 5/7] scsi: target: Report atomic values in INQUIRY
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
` (3 preceding siblings ...)
2024-10-08 2:03 ` [PATCH 4/7] scsi: target: Add WRITE_ATOMIC_16 handler Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2025-05-14 10:56 ` John Garry
2024-10-08 2:03 ` [PATCH 6/7] scsi: target: Add WRITE_ATOMIC_16 support to RSOC Mike Christie
2024-10-08 2:03 ` [PATCH 7/7] scsi: target: Add atomic support to target_core_iblock Mike Christie
6 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This reports the atomic values in the Block Limits VPD page.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_spc.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index ea14a3835681..ce5eec5c8b8a 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -521,7 +521,6 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
have_tp = 1;
buf[0] = dev->transport->get_device_type(dev);
- buf[3] = have_tp ? 0x3c : 0x10;
/* Set WSNZ to 1 */
buf[4] = 0x01;
@@ -562,11 +561,10 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
else
put_unaligned_be32(dev->dev_attrib.optimal_sectors, &buf[12]);
- /*
- * Exit now if we don't support TP.
- */
+ put_unaligned_be16(12, &buf[2]);
+
if (!have_tp)
- goto max_write_same;
+ goto try_atomic;
/*
* Set MAXIMUM UNMAP LBA COUNT
@@ -595,9 +593,29 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
/*
* MAXIMUM WRITE SAME LENGTH
*/
-max_write_same:
put_unaligned_be64(dev->dev_attrib.max_write_same_len, &buf[36]);
+ put_unaligned_be16(40, &buf[2]);
+
+try_atomic:
+ /*
+ * ATOMIC
+ */
+ if (!dev->dev_attrib.atomic_supported)
+ goto done;
+
+ if (dev->dev_attrib.atomic_max_len < io_max_blocks)
+ put_unaligned_be32(dev->dev_attrib.atomic_max_len, &buf[44]);
+ else
+ put_unaligned_be32(io_max_blocks, &buf[44]);
+
+ put_unaligned_be32(dev->dev_attrib.atomic_alignment, &buf[48]);
+ put_unaligned_be32(dev->dev_attrib.atomic_granularity, &buf[52]);
+ put_unaligned_be32(dev->dev_attrib.atomic_max_with_boundary, &buf[56]);
+ put_unaligned_be32(dev->dev_attrib.atomic_max_boundary, &buf[60]);
+
+ put_unaligned_be16(60, &buf[2]);
+done:
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 5/7] scsi: target: Report atomic values in INQUIRY
2024-10-08 2:03 ` [PATCH 5/7] scsi: target: Report atomic values in INQUIRY Mike Christie
@ 2025-05-14 10:56 ` John Garry
2025-07-20 22:42 ` Mike Christie
0 siblings, 1 reply; 23+ messages in thread
From: John Garry @ 2025-05-14 10:56 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi, target-devel
> - */
> + put_unaligned_be16(12, &buf[2]);
> +
> if (!have_tp)
> - goto max_write_same;
> + goto try_atomic;
>
> /*
> * Set MAXIMUM UNMAP LBA COUNT
> @@ -595,9 +593,29 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
> /*
> * MAXIMUM WRITE SAME LENGTH
> */
> -max_write_same:
> put_unaligned_be64(dev->dev_attrib.max_write_same_len, &buf[36]);
>
> + put_unaligned_be16(40, &buf[2]);
> +
> +try_atomic:
> + /*
> + * ATOMIC
> + */
> + if (!dev->dev_attrib.atomic_supported)
> + goto done;
> +> + if (dev->dev_attrib.atomic_max_len < io_max_blocks)
> + put_unaligned_be32(dev->dev_attrib.atomic_max_len, &buf[44]);
> + else
> + put_unaligned_be32(io_max_blocks, &buf[44]);
> +
> + put_unaligned_be32(dev->dev_attrib.atomic_alignment, &buf[48]);
> + put_unaligned_be32(dev->dev_attrib.atomic_granularity, &buf[52]);
> + put_unaligned_be32(dev->dev_attrib.atomic_max_with_boundary, &buf[56]);
> + put_unaligned_be32(dev->dev_attrib.atomic_max_boundary, &buf[60]);
> +
> + put_unaligned_be16(60, &buf[2]);
this is the page length, right? if so, can we always report 60 and
ensure that the atomics and other fields are 0 when they should be? Or
does that violate the spec?
> +done:
> return 0;
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] scsi: target: Report atomic values in INQUIRY
2025-05-14 10:56 ` John Garry
@ 2025-07-20 22:42 ` Mike Christie
0 siblings, 0 replies; 23+ messages in thread
From: Mike Christie @ 2025-07-20 22:42 UTC (permalink / raw)
To: John Garry, martin.petersen, linux-scsi, target-devel
On 5/14/25 5:56 AM, John Garry wrote:
>>
>> + put_unaligned_be16(60, &buf[2]);
>
> this is the page length, right? if so, can we always report 60 and ensure that the atomics and other fields are 0 when they should be? Or does that violate the spec?
I think it was something on the initiator side does not like that
where it uses 0 for the value instead of a OS's default.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 6/7] scsi: target: Add WRITE_ATOMIC_16 support to RSOC
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
` (4 preceding siblings ...)
2024-10-08 2:03 ` [PATCH 5/7] scsi: target: Report atomic values in INQUIRY Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2024-10-08 2:03 ` [PATCH 7/7] scsi: target: Add atomic support to target_core_iblock Mike Christie
6 siblings, 0 replies; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This has us report if the device supports WRITE_ATOMIC_16 in the
REPORT_SUPPORTED_OPERATION_CODES command.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_spc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index ce5eec5c8b8a..3c28b0ec5daf 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -1470,6 +1470,24 @@ static struct target_opcode_descriptor tcm_opcode_write_same32 = {
.update_usage_bits = set_dpofua_usage_bits32,
};
+static bool tcm_is_atomic_enabled(struct target_opcode_descriptor *descr,
+ struct se_cmd *cmd)
+{
+ return cmd->se_dev->dev_attrib.atomic_supported;
+}
+
+static struct target_opcode_descriptor tcm_opcode_write_atomic16 = {
+ .support = SCSI_SUPPORT_FULL,
+ .opcode = WRITE_ATOMIC_16,
+ .cdb_size = 16,
+ .usage_bits = {WRITE_ATOMIC_16, 0xf8, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, SCSI_GROUP_NUMBER_MASK, SCSI_CONTROL_MASK},
+ .enabled = tcm_is_atomic_enabled,
+ .update_usage_bits = set_dpofua_usage_bits,
+};
+
static bool tcm_is_caw_enabled(struct target_opcode_descriptor *descr,
struct se_cmd *cmd)
{
@@ -2026,6 +2044,7 @@ static struct target_opcode_descriptor *tcm_supported_opcodes[] = {
&tcm_opcode_write16,
&tcm_opcode_write_verify16,
&tcm_opcode_write_same32,
+ &tcm_opcode_write_atomic16,
&tcm_opcode_compare_write,
&tcm_opcode_read_capacity,
&tcm_opcode_read_capacity16,
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 7/7] scsi: target: Add atomic support to target_core_iblock
2024-10-08 2:03 [PATCH 0/7] scsi: target: Add WRITE_ATOMIC_16 support Mike Christie
` (5 preceding siblings ...)
2024-10-08 2:03 ` [PATCH 6/7] scsi: target: Add WRITE_ATOMIC_16 support to RSOC Mike Christie
@ 2024-10-08 2:03 ` Mike Christie
2025-05-14 11:01 ` John Garry
6 siblings, 1 reply; 23+ messages in thread
From: Mike Christie @ 2024-10-08 2:03 UTC (permalink / raw)
To: martin.petersen, linux-scsi, target-devel; +Cc: Mike Christie
This has target_core_iblock use the LIO helper function to translate its
block_device atomic settings to LIO settings. If we then get a write
that LIO has indicated is atomic via the SCF_ATOMIC flag, we use the
REQ_ATOMIC flag to tell the block layer to perform an atomic write.
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/target/target_core_iblock.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 3a191bc35e04..afcd62f81d20 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -147,6 +147,8 @@ static int iblock_configure_device(struct se_device *dev)
if (bdev_nonrot(bd))
dev->dev_attrib.is_nonrot = 1;
+ target_configure_write_atomic_from_bdev(&dev->dev_attrib, bd);
+
bi = bdev_get_integrity(bd);
if (!bi)
return 0;
@@ -762,6 +764,9 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
else if (!bdev_write_cache(ib_dev->ibd_bd))
opf |= REQ_FUA;
}
+
+ if (cmd->se_cmd_flags & SCF_ATOMIC)
+ opf |= REQ_ATOMIC;
} else {
opf = REQ_OP_READ;
miter_dir = SG_MITER_FROM_SG;
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread