linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Wang Shilong <wangshilong1991@gmail.com>
Cc: linux-ext4@vger.kernel.org, lixi@ddn.com, adilger@dilger.ca,
	dongyangli@ddn.com, Wang Shilong <wshilong@ddn.com>
Subject: Re: [PATCH] e2fsprogs: fix to use inode i_blocks correctly
Date: Mon, 30 Dec 2019 10:19:21 -0500	[thread overview]
Message-ID: <20191230151921.GA125106@mit.edu> (raw)
In-Reply-To: <1577705766-20736-1-git-send-email-wangshilong1991@gmail.com>

On Mon, Dec 30, 2019 at 08:36:06PM +0900, Wang Shilong wrote:
> blocks_from_inode() did not return wrong inode blocks, and
> ext2fs_inode_i_blocks() is not taking EXT4_HUGE_FILE_FL into account
> at all, while the some callers deal it correctly, some not. This patch
> try to unify to handle it in ext2fs_inode_i_blocks() to return.
> blocks(based on 512 bytes)

I don't really want to change the functionality of
ext2fs_inode_i_blocks().  First of all, it's in a shared library, so
if there are any binaries which were expecting the old behavior, that
could get surprising.  Secondly, its name is confusing and so we're
better off creating a new function ext2fs_get_stat_i_blocks() which
makes it clear that we function is using units of 512 byte sectors,
instead of either the file system block size, or the raw i_blocks
value from the inode.

Two other things about your patch.  First of all, the filefrag command
in debugfs was intended to print the number of file system blocks, so
it was correct as written.  Secondly, please note the blkcnt_t is a
signed type (because the block iterator functions use negative values
to indicate various kinds of metadata blocks), while blk64_t is an
unsigned type.  So using blkcnt_t as a temporary value and returning
it in a function which has a return type of blk64_t will (righly)
trigger compiler warnings.

Here's the patch I've checked into the maint branch of e2fsprogs to
address the issue you've identified.

					- Ted

  reply	other threads:[~2019-12-30 15:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-30 11:36 [PATCH] e2fsprogs: fix to use inode i_blocks correctly Wang Shilong
2019-12-30 15:19 ` Theodore Y. Ts'o [this message]
2019-12-30 18:37   ` Andreas Dilger
2019-12-30 19:52     ` Theodore Y. Ts'o
2019-12-31  1:49       ` Wang Shilong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191230151921.GA125106@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger@dilger.ca \
    --cc=dongyangli@ddn.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=lixi@ddn.com \
    --cc=wangshilong1991@gmail.com \
    --cc=wshilong@ddn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).