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: <20240709074623.GB21491@lst.de>
Date: Tue, 9 Jul 2024 09:46:23 +0200
From: Christoph Hellwig <hch@....de>
To: John Garry <john.g.garry@...cle.com>
Cc: Christoph Hellwig <hch@....de>, chandan.babu@...cle.com,
	djwong@...nel.org, dchinner@...hat.com, viro@...iv.linux.org.uk,
	brauner@...nel.org, jack@...e.cz, linux-xfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	catherine.hoang@...cle.com, martin.petersen@...cle.com
Subject: Re: [PATCH v2 10/13] xfs: Unmap blocks according to forcealign

On Mon, Jul 08, 2024 at 03:48:20PM +0100, John Garry wrote:
>>> +	int			isforcealign;	/* freeing for inode with forcealign */
>>
>> This is really a bool.  And while it matches the code around it the
>> code feels a bit too verbose..
>
> I can change both to a bool - would that be better?
>
> Using isfa (instead of isforcealign) might be interpreted as something else 

The check should be used in one single place where we decided if
we need to to the alignment based adjustments.  So IMHO just killing
it and open coding it there seems way easier.   Yes, it is in a loop,
but compared to all the work done is is really cheap.

>> We've been long wanting to split the whole align / convert unwritten /
>> etc code into a helper outside the main bumapi flow.  And when adding
>> new logic to it this might indeed be a good time.
>
> ok, I'll see if can come up with something

I can take a look too.  There is some real mess in there like trying
to account for cases where the transaction doesn't have a block
reservation, which I think could have happen in truncate until
Zhang Yi fixed it for the 6.11 merge window.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ