Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <15255c5a-82ec-4ce1-8caa-da250fcb7187@oracle.com>
Date:   Wed, 13 Dec 2023 19:01:22 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 01/16] block: Add atomic write operations to
 request_queue limits
To:     Ming Lei <ming.lei@redhat.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, dchinner@redhat.com,
        jack@suse.cz, 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-scsi@vger.kernel.org,
        jaswin@linux.ibm.com, bvanassche@acm.org,
        Himanshu Madhani <himanshu.madhani@oracle.com>
References: <20231212110844.19698-1-john.g.garry@oracle.com>
 <20231212110844.19698-2-john.g.garry@oracle.com> <ZXkIEnQld577uHqu@fedora>
 <36ee54b4-b8d5-4b3c-81a0-cc824b6ef68e@oracle.com> <ZXmjdnIqGHILTfQN@fedora>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <ZXmjdnIqGHILTfQN@fedora>
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?cHBiV29jdnVSWkxFQW1QYWwwQWxqcjhoZ1N5bXRkMW5YZ3BtQjF2YldEbll0?=
 =?utf-8?B?elA5eitjMXhvWDFDblZ4WGhkQWZkSW85dHg4SjExeVNHemdpMGRYUjNDbzNs?=
 =?utf-8?B?RFhreTRaWWh4UFJsWGw2c2RndU9sWmZRTEFxdEVDRitFSTNnZG1UTUh4eTNL?=
 =?utf-8?B?TjduQmEzZmVNemExWlhIU05MUmd5SXBjZVNDWThSVzBDRWlBc0oyWHNyT2xM?=
 =?utf-8?B?VC8rMGFpOVQ2T2h3VG9Pcml0azB0Ui96bTJjS1pEcEFTT3ZWdXI3b0U1eEhX?=
 =?utf-8?B?QTlIcjZvQ2FwQjhIcU5Ga0JYNHNmYnZkd0w2RkxRaVJkbjBxUFNaaWUxSENa?=
 =?utf-8?B?TVpSNWRiV1M2T3d0NWNxSUErWXdhcnl3RTJqcEF3UE5CSG5YRGRYUUJ3RDFl?=
 =?utf-8?B?emo3WXBDN3N3QkZpNkp3OVVsSUdVRmN2V0lnc3p6RWFLWTlyOFlJNEdMVkFO?=
 =?utf-8?B?K01OaFVXdTEwdStick9zYjdGMmVWeVFEanFBaGErQjAwSXNHSmhpT3NMcGo5?=
 =?utf-8?B?bWFWNkYyMHFmRHZUNTB4Tkp1OGxyQ2FORU5hMk4wZUVuK2ZSOUVxWGlsc3VK?=
 =?utf-8?B?amNaWW5mRFhNamtnbXAvVDVubndnYVFiLytjU0FCVkp1YjU5UnpoU2V4OEhj?=
 =?utf-8?B?dnFwSno5azk1ZjVRZ3pEMm1BREg1Vm4xMkhwa2dvQlFuY3ZkcHcxa1g1ZVVS?=
 =?utf-8?B?N0w4OUcxWFFOVzg2Q0VFd1lZWnJMYnhTMHdzRFgrTlVwRHJGQ1J3cjZXQk5C?=
 =?utf-8?B?dkNDVFZUM25xWVlFb3dxRDQvSU0rRkdXK3pncWZKRVRIclpRQ0pKS1JMUGFV?=
 =?utf-8?B?K2VUUXJCYUNRYzFXYldPNDJLQ0RXYTNLY0ZZTDBYdVg1cjBkMHVoUlNrK0pX?=
 =?utf-8?B?TlArZG8wQWIwb2gxaVVHR2U2SGNmTjMySU44TlJLZDdqRjl5Y3QzUXJFZVpr?=
 =?utf-8?B?YUtEUUQ4QUNvRFpMOXJMRWxYekgxSytkNzBsSWFUREMrZ2E5Zk9MMXgzZUNu?=
 =?utf-8?B?RHQva1VVOXBldytCYXpwNXVHMzVqWUhuYjdDb1FwbFBNdkVlUnVHTjVrYS85?=
 =?utf-8?B?SU1LM3RxTlVHMGgxaFprNXJLeGlzTUx3YVRwSFlqWGRFeU5vamhrQWdMNm1k?=
 =?utf-8?B?d0NVbVZtODRob3ZHOEwxaE9tczFBMXh0UmFuUGk4eEppUG9JZXdnS1VHN3Z3?=
 =?utf-8?B?RGc5S3ZJQ2NMRjV0bXF3SnhxUHdjazh5LytReUgzRmk3ZjBha1YyQ2c0clVX?=
 =?utf-8?B?S0RJTWk1T2NDeVlFeDFWTjNMK1RGdGNsUlJqbjcrNGVTNzByejNlOHpyWThs?=
 =?utf-8?B?OXBVVm9wZ2NxV2RDUkJYUGF5M3JEUnJaNlBBSHdIZDFkRFo0Zk9yVjlzZnJ6?=
 =?utf-8?B?TlBzcVV3cDBYM2R1a1VVQjNjVENhZnlXbWZvL2ZlRGYxcEpvNHo1bzJkb2NH?=
 =?utf-8?B?WU5CcTdnK2pwbG1sYzVVL1BnZDQ1TnNNdE5tN1F1RXV2dHBYVTVXSW9PMFV2?=
 =?utf-8?B?aTFqY3R1bHdzeHNJQUZWVkhNbmJiUHpYQzdIWWdZNzlrZm5HUmkyVHU3TlhF?=
 =?utf-8?B?cTBKUGZET1lTTVZQUTZPQ1A5MWExdUZIMGdvSzBQa1dCYm1iYi85dWdCTGZm?=
 =?utf-8?B?OUg3VWVhdlgwTTJqVVllcHo0bjdQTGx2b0RqaDhmQ1NQcnM4UVVrVnBLNXZq?=
 =?utf-8?B?UTlUb2dXd1lwMGhIc1JDL093T1dLS0NzWEN0T2hNcjRXM3o3Uk5QZEFQUWVQ?=
 =?utf-8?B?MXVuN3pNOFZkeE9UWllqNSt4RkxMS1lkMWx0dFczOS8xODVJcy9iRURYdGFI?=
 =?utf-8?B?Q0poNDYzT0liWVZ1bHdKczVGcklsbmxCeEs0dEVRZlY3WGZGUGFUc0JEVmx0?=
 =?utf-8?B?aUxCK3p4ZFQvb2Ezdlh0WEJZR2JUSzcvWUZSZzA5SWZNZW9FT1ZxdjU1MUJT?=
 =?utf-8?B?UUhTSmgzd0tseEdGTXhqeXBOYnJ4QWZOOEwzamdpSS85UGFKbnYxNjd2NWF5?=
 =?utf-8?B?eHMwTmlucDRxM09vRFF5MVllTUZOR1VEaCtuejM2SWluZzF2SFg3bGtsRkJt?=
 =?utf-8?B?RFlTeGJ1S3c5L2lpdHliWjBxMk1oSHJsdUhRZmN0aVYrWHByM0EvSDcwZmxr?=
 =?utf-8?Q?Kx3ofANZnBL71cgG3CkTIow+Z?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?Rm5YMFBYb1o2SGxEMGlJV1hYUC8ydGt0QThBejZmaHhPUEp0SlJZYU4zZ0o4?=
 =?utf-8?B?YjFURXIwZGhRTG43WndqNFJnd2ovQ3hVcjZJOHhWajhManFZUXFuYUhjY3Ey?=
 =?utf-8?B?aHc4SXR6TzZ6anZFaEtPdkx1N3hueHhzWThVYWhkMmxpT2JEVnp2SzhscWp1?=
 =?utf-8?B?eWtNVVZULzREUXJKVEFOdTBuWHoyNTFZblMrYWRlbEMxQXQ3eGN2bTZVT0ZM?=
 =?utf-8?B?WjhtTkVhSFNuaDJNOHF5M29EU09ZNnlBUThtcmYzU1RKSklnQnMxNGllQkNI?=
 =?utf-8?B?Wm5KMU5LdG9RNFNiWWg3WnRSWS9KRnVzYnJNRTRnN3JTZkNkZVYwZ0FVQ1dV?=
 =?utf-8?B?OWduYjVtOVkybE82Qks4eVBDT1dOMngzQlVmUUh0WWdwc2FBRWh4QU41MXVO?=
 =?utf-8?B?M1lxRmYza1luR2dhVlhtLy9lZjA1MTJLUTZJazIzOW1qUjF5SlA1U2Z3WGhx?=
 =?utf-8?B?WjJLVWtoQVNUZ25NWWl6SlBGdjBMaFRMUXFXM1ozREtZVTF1ME1UOGg2U0FY?=
 =?utf-8?B?eVhsWkdDcU9xUzRTdFNGdStydzR0V2l6QjFOZVJpREx4enJVUjBzZVFrcmRZ?=
 =?utf-8?B?UjB5MUtXSGNxU1ZHZ2F2M2hrY05FbXJ0OEt5QWtIdnlVbUlIUU1jSDBxSEo2?=
 =?utf-8?B?bzcrM2dxYW80a2lRajRaRjJoa1hSZzhFMk5DQ3JoOWd3WVBwMnAwSDR6Z1NY?=
 =?utf-8?B?RkIwK0YrdjV0T1ZXam11dTRkQ2IvVXZtUm43VHNXdVgyc0p3eGpmQzErNDk3?=
 =?utf-8?B?a21iVkhVVUNCRmdxT1EvZFplOVh4R1pVQ0RlaUNkNGFqWHRYL3hORW50anVi?=
 =?utf-8?B?QVZqckNES2NLNlptSnB4RTgyTUpIenVlS1JHdithTk1lWTJDbFVqUTgzelBI?=
 =?utf-8?B?NmtQeDBJT3FiMEpVZ0swN2ZoMzlNMkRYMHlqMzQzZWdSbm1ZWlZXZHVrV2s5?=
 =?utf-8?B?M2cxVEVYcGN6TkcyMnlHOXBvQTNwdjBTa2NnZVN6NlhTczFwcXE1bGxobkdP?=
 =?utf-8?B?U08zUGpiaTBIeDkweStzdUZCa3dEYWYxNVBTN1EwdkwvRC8rU0I4Tm5xWWZ1?=
 =?utf-8?B?bDdEVFZSUTdNaVhZelRhdDJmRDgwYzJlcnlsTWQzanU5UFBaVU1zWVI1WnJu?=
 =?utf-8?B?b01EVkZ1Y00xZVJrVHQ1MzhEVVQ0VFBNTHY5ZlRkak9ZSi9lUWg2UTRldGpu?=
 =?utf-8?B?NGVnNGFyTTZWOTZGMVFpc0tudk1zdWhyLzRWb0l1QTRnVHMxc3htTjFZYjRj?=
 =?utf-8?B?MldqVkU5aXlkZzF3aExvZ2VuUUw2aGhMYTVkamt2MHlySHk4VE1Obm1yNHZV?=
 =?utf-8?B?UWRjWnUrbkk3N3M0R3JKQjBPdnNoclJqWjQ2bnFQYkszR0FXc0JOVEVzWDVh?=
 =?utf-8?B?QmZ2MlltbjZUdTRzWHoxQTNoa0pPejY5OGhQdGd0RFpZbzFmRktYY01vNFg3?=
 =?utf-8?B?UDN6UTBZVC9KY25CK1paeWlZVDBmQkRBQzJYTjMvZFZITXQ2b1dDZmI4ODlF?=
 =?utf-8?Q?yXoOfL9l88eg4eVdGWMUpZfE1WP?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7f8eaf05-35e4-46e4-1616-08dbfc0de81d
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 19:01:26.4463
 (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: +R6nkktdHEi+ZXUYHGcmykUMBCzYowgBFZzTxRRp+OHcQBUSV9uC6QSFgUQgX/WvMdtIG8GPJniztbh3BpK2dA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6250
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-13_12,2023-12-13_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0
 phishscore=0 malwarescore=0 spamscore=0 mlxlogscore=999 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2312130136
X-Proofpoint-GUID: ZUuNZwbOs2fDkzcsXqOzPntt-P3QAoqD
X-Proofpoint-ORIG-GUID: ZUuNZwbOs2fDkzcsXqOzPntt-P3QAoqD
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.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 (pete.vger.email [0.0.0.0]); Wed, 13 Dec 2023 11:02:34 -0800 (PST)

On 13/12/2023 12:28, Ming Lei wrote:
>> For NVMe, we use the logical block size. For physical block size, that can
>> be greater than the logical block size for npwg set, and I don't think it's
>> suitable use that as minimum atomic write unit.
> I highly suspect it is wrong to use logical block size as minimum
> support atomic write unit, given physical block size is supposed to
> be the minimum atomic write unit.

I would tend to agree, but I am still a bit curious on how npwg is used 
to calculate atomic_bs/phys_bs as it seems to be a recommended 
performance-related value. It would hint to me that it is the phys_bs, 
but is that same as atomic min granularity?

> 
>> Anyway, I am not too keen on sanitizing this value in this way.
>>
>>>> +
>>>> +/*
>>>> + * blk_queue_atomic_write_unit_max_sectors - largest unit that can be written
>>>> + * atomically to the device.
>>>> + * @q: the request queue for the device
>>>> + * @sectors: must be a power-of-two.
>>>> + */
>>>> +void blk_queue_atomic_write_unit_max_sectors(struct request_queue *q,
>>>> +					     unsigned int sectors)
>>>> +{
>>>> +	struct queue_limits *limits = &q->limits;
>>>> +
>>>> +	limits->atomic_write_unit_max_sectors = sectors;
>>>> +}
>>>> +EXPORT_SYMBOL(blk_queue_atomic_write_unit_max_sectors);
>>> atomic_write_unit_max_sectors should be >= atomic_write_unit_min_sectors.
>>>
>> Again, we rely on the driver to provide sound values. However, as mentioned,
>> we can sanitize.
> Relying on driver to provide sound value is absolutely bad design from API
> viewpoint.

OK, fine, I'll look to revise the API to make it more robust.

Thanks,
John