linux-ext4 - Re: More thoughts about xattrs, journal credits, and their location

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-next>] [day] [month] [year] [list]

Message-Id: <3B21155F-4426-413C-AF16-84E594812530@dilger.ca>
Date:   Fri, 14 Jul 2017 15:13:27 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Tahsin Erdogan <tahsin@...gle.com>
Cc:     "Theodore Y . Ts'o" <tytso@....edu>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Jan Kara <jack@...e.cz>
Subject: Re: More thoughts about xattrs, journal credits, and their location

On Jul 9, 2017, at 14:01, Tahsin Erdogan <tahsin@...gle.com> wrote:

>> What we could do is have ext4_new_inode check to see if there are
>> enough credits to do add the xattr's (if necessary) in a single
>> commit.  If not, what we could do is to add the inode to the orphan
>> list, and then set an inode state flag indicating we have done this.
>> At this point, we *can* break the ext4_new_inode() operation into
>> multiple commits, because if we crash in the middle the inode will be
>> cleaned up when we do the orphan list processing.
> 
> This makes sense. Also, we currently add the worst case credit
> estimates of individual set xattr ops and start a journal handle with
> the sum of it. A slight optimization is to do this lazily.
> We can start with enough credits that can get us to a point where it
> is safe to start a new transaction (safe because of orphan addition).
> Then opportunistically extend the credits to get us to the next safe
> point, if that doesn't work, do the orphan add operation and start a
> new transaction. This should handle the worst case scenario and also
> optimize for common case. Also this should in general reduce the
> amount of allocated-but-unused credits which helps parallelism.

What about accumulating the total xattr size in the credits calculation? In
most cases we know the xattr sizes in advance, and if the transaction handle
tracks the total xattr size it can make a good estimate whether the xattrs
will fit in the inode or not rather than using worst-case credits all the time.

>> The downsides of this approach is that it causes the orphan list to be
>> a bottleneck.  So we would definitely not want to do this all time.
> 
> Yes and I think lazy extend/restart should mitigate this.

Jan had a patch to improve the orphan list performance that never made it
into the kernel by having a per-CPU orphan list or similar.

It recall it got hung up on running out of reserved inodes or similar, which
is an issue we should fix in any case.

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)