lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b85c1818-e5bf-470c-ae0c-6dd2e1525a2f@oracle.com>
Date: Mon, 17 Mar 2025 09:57:45 +0000
From: John Garry <john.g.garry@...cle.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: brauner@...nel.org, djwong@...nel.org, cem@...nel.org, dchinner@...hat.com,
        linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, ojaswin@...ux.ibm.com,
        ritesh.list@...il.com, martin.petersen@...cle.com, tytso@....edu,
        linux-ext4@...r.kernel.org, Carlos Maiolino <cmaiolino@...hat.com>
Subject: Re: [PATCH v6 13/13] xfs: update atomic write max size

On 17/03/2025 07:25, Christoph Hellwig wrote:
> On Thu, Mar 13, 2025 at 05:13:10PM +0000, John Garry wrote:
>> For simplicity, limit at the max of what the mounted bdev can support in
>> terms of atomic write limits. Maybe in future we will have a better way
>> to advertise this optimised limit.
> 
> You'll still need to cover limit this by the amount that can
> be commited in a single transactions. 

yeah ... I'll revisit that

> And handle the case where there
> is no hardware support at all.

So xfs_get_atomic_write_max_attr() -> xfs_inode_can_atomicwrite() covers 
no HW support.

The point of this function is just to calc atomic write limits according 
to mount point geometry and features.

Do you think that it is necessary to call xfs_inode_can_atomicwrite() 
here also? [And remove the xfs_get_atomic_write_max_attr() -> 
xfs_inode_can_atomicwrite()?]

> 
>>   xfs_get_atomic_write_max_attr(
> 
> I missed it in the previous version, but can be drop the
> pointless _attr for these two helpers?

ok, fine

> 
>> +static inline void
>> +xfs_compute_awu_max(
> 
> And use a more descriptive name than AWU, wich really just is a
> nvme field name.

I am just trying to be concise to limit spilling lines.

Maybe atomicwrite_unit_max is preferred

> 
>> +	awu_max = 1;
>> +	while (1) {
>> +		if (agsize % (awu_max * 2))
>> +			break;
> 
> 	while ((agsize % (awu_max * 2) == 0)) {
> 
> ?
> 
>> +	xfs_extlen_t		m_awu_max;	/* data device max atomic write */
> 
> overly long line.

there are a few overly long lines here (so following that example), but 
since there is a request to change the name, I'll be definitely using a 
newline for the comment

Thanks,
John

> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ