linux-kernel - Re: [PATCH -next v5 7/8] xfs: speed up truncating down a big realtime inode

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]

Date: Sat, 15 Jun 2024 19:44:21 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, djwong@...nel.org, brauner@...nel.org,
 david@...morbit.com, chandanbabu@...nel.org, jack@...e.cz,
 yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com,
 John Garry <john.g.garry@...cle.com>
Subject: Re: [PATCH -next v5 7/8] xfs: speed up truncating down a big realtime
 inode

On 2024/6/14 17:13, Christoph Hellwig wrote:
> On Fri, Jun 14, 2024 at 03:18:07PM +0800, Zhang Yi wrote:
>> Thanks for your suggestion.
>>
>> Yeah, we could fix the realtime inode problem by just drop this part, but
>> for the upcoming forcealign feature and atomic feature by John, IIUC, we
>> couldn't split and convert the tail extent like RT inode does, we should
>> zero out the entire tail force aligned extent, if not, atomic write could
>> be broken by submitting unaligned bios.
> 
> Let's worry about that if/when those actually land.

OK, if we don't consider the upcoming forcealign feature and atomic feature,
I think only path 6 is needed to fix the issue.

> I also see no
> rason why those couldn't just use partially convert to unwritten path
> offhand (but without having looked into the details).
> 

The reason why atomic feature can't split and convert the tail extent on truncate
down now is the dio write iter loop will split an atomic dio which covers the
whole allocation unit(extsize) since there are two extents in on allocation unit.

Please see this code:
__iomap_dio_rw()
{
	...
	while ((ret = iomap_iter(&iomi, ops)) > 0) {
		iomi.processed = iomap_dio_iter(&iomi, dio);
	...
}

The first loop find and submit the frist extent and the second loop submit the
second extent, this breaks the atomic property.

For example, we have a file with only one extszie length，if we truncate down
and split the extent, the file becomes below,

  |   forced extsize (one atomic IO unit)  |
  wwwwwwwwwwwwww+uuuuuuuuuuuuuuuuuuuuuuuuuuu
                ^new size A                ^old size B

Then if we submit a DIO from 0 to B, xfs should submit it in one bio, but it
will submit to two bios, since there are two extents. So, unless we find
another way to guarantee submit one bio even we have two extents in one atomic
write unit (I guess it may complicated), we have to zero out the whole unit
when truncate down(I'd prefer this solution), we need consider this in the near
future.

Thanks,
Yi.