Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <b6ed0e26-e3d4-40c1-b95d-11c5b3b71077@oracle.com>
Date:   Thu, 5 Oct 2023 11:24:57 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 21/21] nvme: Support atomic writes
To:     Pankaj Raghav <p.raghav@samsung.com>,
        Alan Adamson <alan.adamson@oracle.com>
Cc:     axboe@kernel.dk, kbusch@kernel.org, hch@lst.de, sagi@grimberg.me,
        jejb@linux.ibm.com, martin.petersen@oracle.com, djwong@kernel.org,
        viro@zeniv.linux.org.uk, brauner@kernel.org,
        chandan.babu@oracle.com, dchinner@redhat.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-nvme@lists.infradead.org, linux-xfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, tytso@mit.edu, jbongio@google.com,
        linux-api@vger.kernel.org
References: <20230929102726.2985188-1-john.g.garry@oracle.com>
 <20230929102726.2985188-22-john.g.garry@oracle.com>
 <CGME20231004113943eucas1p23a51ce5ef06c36459f826101bb7b85fc@eucas1p2.samsung.com>
 <20231004113941.zx3jlgnt23vs453r@localhost>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20231004113941.zx3jlgnt23vs453r@localhost>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TjFabWJlcGZvTU0wSjc0b3pFZjd4NkxkMXp6TVo3V2tYcmxCVVZ5Y05ldSsz?=
 =?utf-8?B?NHduZFdWSUFySGcxZnBEa3F1d1p3czJFaktNTldoaFNCZDZpYTd5YkZsQ05h?=
 =?utf-8?B?ZkJZWUpkQmJqcVMyd25QWHBvbmxQS0pBMzhMclpjSWJOVm4rdDU0bEZmelFL?=
 =?utf-8?B?Uy9wdjFMR1U3eEZwZmFjelEwVmx0bkdJNmhCdWZEK1FFSWVTbWN0MEVub0hE?=
 =?utf-8?B?dDBTamIwNFVBUzVvRVdKamthWTFuRDZrSGh4VW90UG9COFo1bWZVb3h3VDdE?=
 =?utf-8?B?YzhaVU40UW1ROWlpUmZnL2FPUWJ2YmlveXZoK3R2U2FxZ1MvdU1QWDRHTFhI?=
 =?utf-8?B?T0VHQlduejI4S1FuRGhsUjlPRzRSeW9iUGRGWml2K2ZuMHFhNW9vLzhNTEFn?=
 =?utf-8?B?WFFuVVlZajBEdk5ITmZkWkpwK1J6cS8wR3h3MmV1ODBQNUJobVRFZmdFY1Ev?=
 =?utf-8?B?ckoxUlhCdFl0SUlvM1hjN3dRdWVnMUpIVjJmc2NxKzdTbEhhT3A5cEpmT2tW?=
 =?utf-8?B?TVhpcWM2TkNMcHlUWHpFZE1hcE1mVXpsRnJWYU5UQS9SeTMzWFdoNzZQZVN1?=
 =?utf-8?B?QTFyNEdPckRPSFVKams2dVRMbFJEUHNOUk9ndHl2VFcrSCtyUnNuU1lHbWl5?=
 =?utf-8?B?d0Vma3RUVFNubXRDWHgvQVBDaWo2NlI5WU5RSWNwa2N6ZFRxcUYxVGExTzNx?=
 =?utf-8?B?M2xZczNEaWFEaGxTdGRWZzZxTG1pblpFN0YzL3d5bGNlczhyU0pGVmlMcmFq?=
 =?utf-8?B?TTNDSlM3MVJENkJlSklrSWNVRUdtZG83VG1EcHZpcFBidG9pbkFnblRyMmhn?=
 =?utf-8?B?NXNGeUxrKzZSSmQwQWZJay9JQmpBTzlVR1lIV1FmRTNyOUJqSjU5cm4zWVc2?=
 =?utf-8?B?Z0dleCtLQy9HQlRtYVRxY0RYc2xlU2crVWdPcCtSYjN6ZUdsUzhqWlpVcmE0?=
 =?utf-8?B?UnQ5cm1oeEQ3d3QzV3JzK0t4ZUZPWEM1WlV1dGFlTld2TkZxV0F1MUFvVHJK?=
 =?utf-8?B?QVVpeVFqWXhoVTAwZkcvamRjandxaXd0ZlBoWlBzRHRvTThVS0J3U1NpRVUz?=
 =?utf-8?B?SFBDQ2tleGFzSmlUZ1BwMmozM1RCT2dVMXRIQWlGYy9MeFZxdXJiZG9Dazcr?=
 =?utf-8?B?QU1CR2VTaDZmeXJ4STdvcmlUZ0k4MzhUWkc2K3NCVkpTZmJ5QitpUmt0U2VS?=
 =?utf-8?B?K2U3eThmR0I3bVNYNFE0STRKdjFhL2xoWFdvOGcxNGpWSFBPTGN4ak1ETHZq?=
 =?utf-8?B?anUvSExZREV1U002RFdDSU1aYW9KUXRoc0VFZ3ZqU0l3bmdJdEZFY0ZWU3dS?=
 =?utf-8?B?VklWa3JxVk56dkdHZ2kxdkUzbDJmdzk0aWs3alU3clJQSUI0eVY1bTF3cDNq?=
 =?utf-8?B?NlhrNGVMNldJOGZWSmdPTHlyQ1lETW80K3VBY3FLNElmSEY4MVpYTXhhVm5n?=
 =?utf-8?B?MmdVVVN4MW1WQW5jeGZMMDhIbU10TWxzbzl4cG44cmtRaTBCVUt5LzVGdzB3?=
 =?utf-8?B?SVFuS3Jzd0d6dEhGQmFtNkFhKzEwOGJQbnZBdXpuZWgvNkRzMGJDM2dpMkwx?=
 =?utf-8?B?N09Ec3lyZWJUOGYzcmgwOHJwZlIwTTNBM3VORlNxNEFwck1rRERHVGUvN3pT?=
 =?utf-8?B?SWFTbGpQZDBLVWVycWJFTnJFZ0N4ZlJBM2hZNXNXVmFQd21BQkR1ZldtQ0xZ?=
 =?utf-8?B?S3dMeTlSYUd5T2EyU0JVTDJGVmo4NEdqd2lPamQyNzI3RGkzQ0g3bXJSSGt1?=
 =?utf-8?B?Rk9VcVQ2dmwyZk9vQTdjTi81cE1CQ2RlaUp0R2o3ckZKN2thTTd5TEdQbDFq?=
 =?utf-8?B?c3M1ZzVNUUF0MVMxbUI2WDNXRnJKbUNJZTcwKzVzU2FJeG9GVEViYUlEOE9a?=
 =?utf-8?B?U0Z6TGVmazNHMW1xVnVnb2l5MDY1N3Z6dEUwU285K1VUaXh1SGFjWFFvREhQ?=
 =?utf-8?B?QWtUSnZrbjkxTlBlbXN1ZHBwL1U0VjBhSTAvd1BVLy9lQnZqSjZnOTVMNGhT?=
 =?utf-8?B?T1BRT0NQRmNLNnNpZEZ4bFFxNUpEeTRjYVd5YTBQd0VyV0dVV0k0M2dWL2Js?=
 =?utf-8?B?NHducWw5YWV3UDVpYjAyWGxLUzNweWdDTzNrZlZ1ZHNqb00zbjk2YXJES3ZZ?=
 =?utf-8?Q?A4mxwny/puz/eamJYmUwo7Cdh?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?c0JHc01TS3FGNVZWRU1vY1IxVHgvcTY4QmwzYUNYbW0zQXYxTlpYTjlwcm1r?=
 =?utf-8?B?cjVzcHJtMWJVSkVoZG96UmpsK21ZM253OVFUR1dNcG80ZGEyeEtIOXBvVWFU?=
 =?utf-8?B?OFFxSzllanh5ckhNUk5BOEVlNWJyQmRGTmZUUDhGM29ELzBOKy9pTDFKUnRv?=
 =?utf-8?B?UEI2b2dRNHBaUlhOSSs2R2ZKUmtvZ1h5akV6QTduendPdUNJUzdpWTdCUGhC?=
 =?utf-8?B?TktMYk5DdVE1czNwaUJ0Wml1ajNkWkdoWDFxNHpCUkJ2L29wMnBicUxkb1Y0?=
 =?utf-8?B?OGJTTzZDeU5XTHRqZklpWHpNcXA0Yno3cWRzMGpzd1NHM2VJa29ab1kxSENk?=
 =?utf-8?B?QlQzYnJwczUrbFBocGRnSnk1TWNsN2tyVkg2MzVQRXk2SGRlcEtCNElVaytj?=
 =?utf-8?B?bUlYT2MyWHVoRVNhVHhVL2ZWSm95ajg0L2VkcmZGYlFjVmsvQWRieXlJQ2xQ?=
 =?utf-8?B?VkRvNklVOUx3WDdGL2E4ZkNmOEtGdmk4OVNMSERtSjBJQ0JpRXQzRmxpZ000?=
 =?utf-8?B?OHYwYmNRYlNQc0tLaXMzK3ZITlNxWmRid2dNU0ZJaVg5SS9KcG9VUGcvZllw?=
 =?utf-8?B?QU56anJjK2tNR2hZYzU4eTBnV3RKcC9wdU05Nk9DSUIwbEw2cnhPTURZVk1T?=
 =?utf-8?B?aDhsNWNoR0F6WGlvMFpwbTUvWEtidWplSTA4bVRCdVZMdVY4cUtyWURUWThw?=
 =?utf-8?B?K2VhL1JXVWpZeTdIaWFubHpmUGJFeDNTMkdTazMwKzV2ajdHK0VIeUd0eWRS?=
 =?utf-8?B?bW1MZlJTTXgycERhaVB0UnlFazB2SldGbnRCNlFaalMwc1dmTVdqUE5wUTgx?=
 =?utf-8?B?TTRyWFVIVXp0dE1lNlpCRldBalJ6YTVvU21EYVZjcDF2djhpSjhrc1NHUzc3?=
 =?utf-8?B?OVB4TkZEdWpaZE9STlQ3TWcyL3Rya0xyNndJN2NVbnZpWklGYjA5M2NjNXli?=
 =?utf-8?B?bmJJdkZuTFliM3JpK0xtaU9uUlNoK2ZKd1ZCVUcxN0wySm9iMDEyaDc2S3NZ?=
 =?utf-8?B?ejN2LzVuQ2Z3OWVzSmltOS9RMmFwRURLbUxjMDNkUnNqNndDQ0lFSURSa0lC?=
 =?utf-8?B?eGc1b0UzV1lxSEJzM0lKeGNoWHJObm1xK2ZOMCtXeFkwckl6YWhySDZHY3lD?=
 =?utf-8?B?STRvbDZCRloza09yTVRsNFlyZ2EwRnhySi9VQUR6elp2NE81dlg0R1pBMzhr?=
 =?utf-8?B?UG00UmhtTzR5d2ZjUWJ4SmZqSS9oN3hWRjJlWGFWSmowV3k5TEtkbmU2UWpa?=
 =?utf-8?B?T05LQjh6bUQ5eWhQMldWY1h4Q1FHWGIrejlvY3hFUFhRSWlGQ0FKcllWWXhX?=
 =?utf-8?B?Z04xUXlETmpiVklKK3o3V1V0R3kvMzFRZkFyZk53bkdubDZJTXI4Q0h1RW1G?=
 =?utf-8?B?WHUwNHoxN1RvOHBqVzZrZStSeUVqa2JJL25WcGFUZlM4dncwZ2J5Ly9Pb0Vr?=
 =?utf-8?B?SnJkb2kycVZNOG5WVzZkSTdMTXhwL1lnSG5XVGU0dEpRdjdCcG82K2QwY2hG?=
 =?utf-8?Q?DF8SrzUnBDvEMOrhoyuiNSe2ZHa?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 33f86953-93af-46ae-fbc9-08dbc58d5662
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Oct 2023 10:25:03.5979
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HJEpIscTXbwGgdUQm2BpTdYWu9NlkY6yZR/clXf4cEg0VQcvU8/q/x0voPUf494y8xa8CJiii5SS1AwKwm/mwg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR10MB7031
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-05_07,2023-10-05_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 suspectscore=0 malwarescore=0 spamscore=0 bulkscore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2309180000 definitions=main-2310050081
X-Proofpoint-GUID: 0Tcm2gbfdCi6ZeYavL8lTGHJcnntTZ2e
X-Proofpoint-ORIG-GUID: 0Tcm2gbfdCi6ZeYavL8lTGHJcnntTZ2e
X-Spam-Status: No, score=-5.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 05 Oct 2023 08:47:17 -0700 (PDT)

On 04/10/2023 12:39, Pankaj Raghav wrote:
>> +++ b/drivers/nvme/host/core.c
>> @@ -1926,6 +1926,35 @@ static void nvme_update_disk_info(struct gendisk *disk,
>>   	blk_queue_io_min(disk->queue, phys_bs);
>>   	blk_queue_io_opt(disk->queue, io_opt);
>>   
>> +	atomic_bs = rounddown_pow_of_two(atomic_bs);
>> +	if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf) {
>> +		if (id->nabo) {
>> +			dev_err(ns->ctrl->device, "Support atomic NABO=%x\n",
>> +				id->nabo);
>> +		} else {
>> +			u32 boundary = 0;
>> +
>> +			if (le16_to_cpu(id->nabspf))
>> +				boundary = (le16_to_cpu(id->nabspf) + 1) * bs;
>> +
>> +			if (is_power_of_2(boundary) || !boundary) {

note to self/Alan: boundary just needs to be multiple of atomic write 
unit max, and not necessarily a power-of-2

>> +				blk_queue_atomic_write_max_bytes(disk->queue, atomic_bs);
>> +				blk_queue_atomic_write_unit_min_sectors(disk->queue, 1);
>> +				blk_queue_atomic_write_unit_max_sectors(disk->queue,
>> +									atomic_bs / bs);
> blk_queue_atomic_write_unit_[min| max]_sectors expects sectors (512 bytes unit)
> as input but no conversion is done here from device logical block size
> to SECTORs.

Yeah, you are right. I think that we can just use:

blk_queue_atomic_write_unit_max_sectors(disk->queue,
atomic_bs >> SECTOR_SHIFT);

Thanks,
John

>> +				blk_queue_atomic_write_boundary_bytes(disk->queue, boundary);
>> +			} else {
>> +				dev_err(ns->ctrl->device, "Unsupported atomic boundary=0x%x\n",
>> +					boundary);
>> +			}
>>