Re: [PATCH v2] ext4: have ext4_xattr_set_handle() allocate journal credits

[Tahsin Erdogan]

> One problem with this approach is that restarting the transaction handle will
> make the xattr update non-atomic, which could be a real problem for some
> workloads.  For example, ACLs or SELinux or fscrypt xattrs being added in
> a separate transaction from file creation, or being modified (delete in a
> separate transaction from add) and then lost completely if the system crashes
> before the second transaction is committed.

Agreed.

> It isn't clear to me why using the current helper function to precompute the
> required transaction credits doesn't get this right in the first place?  It
> would just need to add the xattr credits to the original journal handle?

An example code path is this:

ext4_mkdir()
  ext4_new_inode_start_handle()
    __ext4_new_inode()   <<== transaction handle is started here
      ext4_init_acl()
        __ext4_set_acl()
          ext4_xattr_set_handle()

In this case, __ext4_new_inode() needs to figure out all journal
credits needed including the ones for ext4_xattr_set_handle(). This is
a few levels deep so reaching out to ext4_xattr_set_credits() with the
right parameters is where the complexity lies.