On Fri, May 18, 2018 at 02:43:02PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@xxxxxx>
>
> Jun Wu at Facebook reported that an internal service was seeing a return
> value of 1 from ftruncate() on Btrfs when compression is enabled. This
> is coming from the NEED_TRUNCATE_BLOCK return value from
> btrfs_truncate_inode_items().
>
> btrfs_truncate() uses two variables for error handling, ret and err (if
> this sounds familiar, it's because btrfs_truncate_inode_items() does
> something similar).
This err/ret pattern is widely used in btrfs code and IIRC mostly
consistently, where err is the global return and ret is local.
> When btrfs_truncate_inode_items() returns non-zero,
> we set err to the return value, but we don't reset it to zero in the
> successful NEED_TRUNCATE_BLOCK case. We only have err because we don't
> want to mask an error if we call btrfs_update_inode() and
> btrfs_end_transaction(), so let's make that its own scoped return
> variable and use ret everywhere else.
That's an alternative pattern when 'ret' is the global return and ret2
are locals. Either way, it would be good to unify that.
> Fixes: ddfae63cc8e0 ("btrfs: move btrfs_truncate_block out of trans handle")
> Reported-by: Jun Wu <quark@xxxxxx>
> Signed-off-by: Omar Sandoval <osandov@xxxxxx>
> ---
> This is based on Linus' master rather than my orphan ENOSPC fixes
> because I think we want to get this in for v4.17 and stable, and rebase
> my fixes on top of this.
For a 4.17-rc the patch would need to be smaller. You describe the
actual bug as a missing reset of the err, so if it's just that, we can
consider that for 4.17, but the patch as it is changes the error value
propagatin in not so trivial way.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
