Received-SPF: pass (google.com: domain of linux-kernel+bounces-65244-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <8332ea29-ac17-4b1a-8ed9-e566d03fd220@oracle.com>
Date: Wed, 14 Feb 2024 13:02:21 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 14/15] nvme: Support atomic writes
To: Nilay Shroff <nilay@linux.ibm.com>
Cc: alan.adamson@oracle.com, axboe@kernel.dk, brauner@kernel.org,
        bvanassche@acm.org, dchinner@redhat.com, djwong@kernel.org, hch@lst.de,
        jack@suse.cz, jbongio@google.com, jejb@linux.ibm.com,
        kbusch@kernel.org, linux-block@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
        linux-xfs@vger.kernel.org, martin.petersen@oracle.com,
        ming.lei@redhat.com, ojaswin@linux.ibm.com, sagi@grimberg.me,
        tytso@mit.edu, viro@zeniv.linux.org.uk
References: <20240124113841.31824-15-john.g.garry@oracle.com>
 <20240214122719.184946-1-nilay@linux.ibm.com>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240214122719.184946-1-nilay@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?Y2RYMUhJZWdSWlZlZzBhR2FSRFNpTm13YTM5Mk1qcDI5SUZKdEo1SkkzaCsz?=
 =?utf-8?B?QkJKa1FRbmU0UFV6WUxyWkJRaTIrKzJTZEFmNkdOdXRFa2IxcG5sQk9MNFAx?=
 =?utf-8?B?Yk1nbDZZSERRWjNLaGhiQm5ZMG5zQ2xLN29NZjI5aUJqc0g2WDZMaGUvL1Fj?=
 =?utf-8?B?N3NnVjNmdlZ5Ym5pVXVCRmEyUHhVUHduNnhmbEV3Y01mZVRUM2YycVVGVnE3?=
 =?utf-8?B?TUJleEs5eUowTVNGa3hrNEQ2TXN1alMyd0N1UDcvVDMzQUF2YjRqNW9VL01C?=
 =?utf-8?B?SG8rUEpnUXFJcUVYandBRjYxZTc3TjJMR2tvNEQvTklYM3BkWlVQNzlycVQ0?=
 =?utf-8?B?ZDlBM2wyQUc2QitONnFobW9LVmdKK2JGWEtmODhUYUZwQ0xnSUxpUUYrNkQw?=
 =?utf-8?B?d1IvS01MQjcwdHFSNk14QnB4V2hiRGNucUYvY1lYdUJ5ajB5RHpGbEpjRGZY?=
 =?utf-8?B?ajFzUk1RYmY3R053Unh4eDdTbGliR2Z5QjNqTmJtN2VCMUVMSFhBL0ZPVFFk?=
 =?utf-8?B?YzlPRVU4UklXWHdjcC80cGlzNm9hM3Y4cktuMHNaR0l4UHdPcmVUcXJ6Vjdl?=
 =?utf-8?B?ZnRkOFB3bUZoMzh4dlZpSDA0cGswU3RQNFBQY09FSms1cnpYc2FlSnhPMjVt?=
 =?utf-8?B?UjdzUWxpdlZlK2owMFU0d0ZWeGdOT2pPNXVIc0VTRTNYRWVvY3BTSnlOYVBH?=
 =?utf-8?B?QUJXVXRBTGhNQVJrbW1weFkwa1JkSVEzQnZDUmFMeDdZYWlNcGpLTUtIQklu?=
 =?utf-8?B?WEtWeGQ0N0F5NExKNHdaY1EvcWQxbG9kT1daL1FwZm14TW42K3ZhcXMxdkpY?=
 =?utf-8?B?eUFuTzBqVDhSZ0pLZ1NUdEtZeUwrM1o2d0dIbTcvekxVOGtmbUdOY1ZTWmNo?=
 =?utf-8?B?UFUzZFNzS25ibStVWEdSYjBUSnhZVExacEM5aUYyUDZETmtQMEdZTU44L3lW?=
 =?utf-8?B?K0FSMUlRZWkzREZUZGluVkM3YmdZN1h1YW9MTGpHdVM1OGVzSE1UTHIrZmFZ?=
 =?utf-8?B?TysvR0t0MCtIWis1bkkvaHFxQXpuZmxxUU9Kck5KNUFxTlY3bkw3c2hFeEJt?=
 =?utf-8?B?QVdKZlBvQkRIUjNuSzlUTzdaeDRWU0U3SGY1cUFNbFZQeTg2RU44RWc5SzVm?=
 =?utf-8?B?d2IvTEUvSHFERE0raU9aT3dpY0dCbEFqN0h4bENWMUt4UFNPMkg5bUxzMGJp?=
 =?utf-8?B?ZS9jQTFVbSsvQUNuUWRoREphMmNWeWVlZlJFaHVxM2szQ2owNTdUTU1wRzEx?=
 =?utf-8?B?NFp0aFY4OW55T1BLV1hjWmJNM1gwWHZRQjFJUjltTytLelVwV292RlBxWjIz?=
 =?utf-8?B?RExZOUZKRkxzUFIxci96N0RKalBEQzd3MmtOZVRHL2xwZDFZY2RGeU4rQ0c3?=
 =?utf-8?B?bkR5cGl1TGN5Mk8vMTRVd0Q0YWpwVks2dXJVQVkyUGE2MGFKWVVVM1UyM0F1?=
 =?utf-8?B?QllpTGxtTHBkSU5CWmZ4ZW5rV2ZJT0NPSlhZd001bjlob3QzMFFZc01HSkhz?=
 =?utf-8?B?Zm9ESXVGWXFyQVh3T3EzYWFxYU9rclR1ZFc0VmNmc0NhVTdQM3ZmRjZqblZs?=
 =?utf-8?B?QTJObVJ3L1RRM3RYWXBEeGRXaTh0U04yKzd0L0FDdE5mM3duQlVMTVk4Z0g4?=
 =?utf-8?B?MkRqb3l3RWxQYlFQVnBNdTU5TzVmR0d1b25UU2NvQ28rUUsxTFUvUm9rUm5D?=
 =?utf-8?B?MnVoY1RWalVDa0NzZUVaMndSSGExenROU3o1NjVLcWhxcSs4NnpUaUgyN0ZD?=
 =?utf-8?B?c29Qd0ljeTJkeWRpNnNGNUh6TG9nbXcycHhPZVR6b1FUSFZZK0hJdkgxZ1VQ?=
 =?utf-8?B?OWJJdHhVZy8vR1RpcFFSWTBqOXozN0lpM0FEcWdnV1BZZnh3K1RwSXlLek5F?=
 =?utf-8?B?YXdoSm5VUW9Rcy9SY3dzbS9ITUlFUnhoMFp1SGRyVzUza1c3eUdXRkZPU2V5?=
 =?utf-8?B?cEEwdEZjQnZ4b0tUamE3bW9Sd3hnSmp3SjdsT3NON3JUVEFEdlJYdWFxOGRF?=
 =?utf-8?B?eG9JdmRSUmNRcENZTUt0ZHZ6T1RRQ2xPcGZvUTI1Ty94dHFRY3V3UVoxVUdT?=
 =?utf-8?B?dTE1VzFZT1MvZk1yZVVFRzFER1dRajRtKy9nVmsvY2IydFJwZ1ZLczFrcS9P?=
 =?utf-8?B?RHBNSW1jdmttaDRPSE5JcnV1eVVwMlNYUHl3UWtZLzIvZXBCOXJFQ1NCU0RN?=
 =?utf-8?B?R3c9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	/81/r913BXzr+meQpUU2qnmlGxcHct4br28QsLb0QfWHb8A/8ZrsYSymZNMmQQfqNfJg5bWY4VkdNvIbbAQua/j87y1pzCfa0dmNqQcHPdBlMyFxoU7mFLaW4xhup7HmVholmIIunIqMDYzKqu1DeTw/5bgGqVqVOEbIkmSYl9zkuvwGIzVxlpXOoOSJrjaSerCHKlv0z6QHd9MbJAUnB7oFikVFB6hg4zJ42fsmBAs9BrIOh3rlkPxpYorPXbEE+b5zFfcn8aJR3fd1TnMswjXxqJbpMyMXQUAwZRNKPrt2GKZvZmByGsgPwCt+DigOlovwzcIre2jEFFYdDER/GhSS2Y7rfClJDnY+CQDWLnKkEh+1gIbyXrec7PBVPHLwXRP07AW7zf0qK5PBBih/vghz1+khjnq6GNpQ9LI/N2R2i3qNoArIC7pv3vA0NEkl0uwyXhCoJIVhQL4TaKZ258Jly38lhJwTb1wcolxVgZv0dTE1z99nHBFTSiKQjMqereFJjQLNVUFUxVVRbLmgP6/mH5LWzp+69PBIpZ8xN0dkmGiJd9KWU2pgnpNm6JmpUoSV4eto3TG5Bu11uSDOltDBM5HTxdkNPKM7lehfjFk=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2947dbee-84fa-4890-e3ca-08dc2d5d317c
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2024 13:02:26.7112
 (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: nfrejtdPtIt82C9k8F2faRcHdnDdzmgEx2zj3utSvqKcxkBzeMjiO82rrOyw8GqnbFFeMxQiim577BHT+9UDgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB7252
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-14_05,2024-02-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 adultscore=0
 suspectscore=0 mlxscore=0 phishscore=0 bulkscore=0 malwarescore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402140099
X-Proofpoint-ORIG-GUID: 9Xtj4XPBCiXEI9r6vhTxHLPBmnAvMV9T
X-Proofpoint-GUID: 9Xtj4XPBCiXEI9r6vhTxHLPBmnAvMV9T

On 14/02/2024 12:27, Nilay Shroff wrote:
>> Support reading atomic write registers to fill in request_queue
> 
>> properties.
> 
> 
> 
>> Use following method to calculate limits:
> 
>> atomic_write_max_bytes = flp2(NAWUPF ?: AWUPF)
> 

You still need to fix that mail client to not add extra blank lines.

>> atomic_write_unit_min = logical_block_size
> 
>> atomic_write_unit_max = flp2(NAWUPF ?: AWUPF)
> 
>> atomic_write_boundary = NABSPF
> 
> 
> 
> In case the device doesn't support namespace atomic boundary size (i.e. NABSPF
> 
> is zero) then while merging atomic block-IO we should allow merge.
> 
>   
> 
> For example, while front/back merging the atomic block IO, we check whether
> 
> boundary is defined or not. In case if boundary is not-defined (i.e. it's zero)
> 
> then we simply reject merging ateempt (as implemented in
> 
> rq_straddles_atomic_write_boundary()).

Are you sure about that? In rq_straddles_atomic_write_boundary(), if 
boundary == 0, then we return false, i.e. there is no boundary, so we 
can never be crossing it.

static bool rq_straddles_atomic_write_boundary(struct request *rq,
unsigned int front,
unsigned int back)
{
	unsigned int boundary = queue_atomic_write_boundary_bytes(rq->q);
	unsigned int mask, imask;
	loff_t start, end;

	if (!boundary)
		return false;

	...
}

And then will not reject a merge for that reason, like:

int ll_back_merge_fn(struct request *req, struct bio *bio, unsigned int 
nr_segs)
{
	...

	if (req->cmd_flags & REQ_ATOMIC) {
		if (rq_straddles_atomic_write_boundary(req,
			0, bio->bi_iter.bi_size)) {
			return 0;
		}
	}

	return ll_new_hw_segment(req, bio, nr_segs);
}


> 
> 
> 
> I am quoting this from NVMe spec (Command Set Specification, revision 1.0a,
> 
> Section 2.1.4.3) : "To ensure backwards compatibility, the values reported for
> 
> AWUN, AWUPF, and ACWU shall be set such that they  are  supported  even  if  a
> 
> write  crosses  an  atomic  boundary.  If  a  controller  does  not  guarantee
> 
> atomicity across atomic boundaries, the controller shall set AWUN, AWUPF, and
> 
> ACWU to 0h (1 LBA)."
> 
> 
> 

Thanks,
John