All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH 06/15] ext2fs: add new APIs needed for fast commits
Date: Wed, 2 Dec 2020 13:44:58 -0500	[thread overview]
Message-ID: <20201202184458.GJ390058@mit.edu> (raw)
In-Reply-To: <20201120191606.2224881-7-harshadshirwadkar@gmail.com>

On Fri, Nov 20, 2020 at 11:15:57AM -0800, Harshad Shirwadkar wrote:
> This patch adds the following new APIs:
> 
> Count the total number of blocks occupied by inode including
> intermediate extent tree nodes.
> extern blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
>                                        struct ext2_inode *inode);
> 
> Convert ext3_extent to ext2fs_extent.
> extern void ext2fs_convert_extent(struct ext2fs_extent *to,
>                                        struct ext3_extent *from);

So one of the reasons why I've intentionally never exposed "struct
ext3_extent" in the libext2fs interface is because that's an on-disk
structure which I keep hoping we might change someday --- for example,
to allow for 64-bit logical block numbers so we can create ext4 files
greater than 2**32 blocks.  It might be that some other future
enhancement, such as say, reflinks (depending on how we implement
them), or reverse pointers, might also require making changes to the
on-disk format.

The kernel code has the on-disk format and the various logical
manipulations of the extent tree hopelessly entangled with each other,
which means changing the kernel code to support more than one on-disk
extent structure is going to be **hard**.  But in the userspace code,
all of the knowledge about the on-disk structure is abstracted away
inside lib/ext2fs/extent.c.

It may very well be that for fast commit, we're going to need to crack
open that abstraction barrier a bit.  But let's make sure the function
name makes it clear that what we are doing here is converting between
a particular on-disk encoding and the ext2fs abtract extent type.
"ext2fs_convert_extent" doesn't exactly make this clear.

It might also be that what should do is include a pointer to the fs
and inode structures, and call this something like
"ext2fs_{decode,encode}_extent()", and pass in the on-disk format via
a void *.  We might also want to have some kind of
ext2fs_validate_extent() function which takes a void * and validates
the on-disk structure to make sure it's sane.

What do you think?

					- Ted

  reply	other threads:[~2020-12-02 18:45 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 19:15 [PATCH 00/15] Fast commits support for e2fsprogs Harshad Shirwadkar
2020-11-20 19:15 ` [PATCH 01/15] ext2fs: move calculate_summary_stats to ext2fs lib Harshad Shirwadkar
2020-12-02 16:47   ` Theodore Y. Ts'o
2020-11-20 19:15 ` [PATCH 02/15] ext2fs, e2fsck: add kernel endian-ness conversion macros Harshad Shirwadkar
2020-12-02 16:50   ` Theodore Y. Ts'o
2020-12-03 18:10     ` harshad shirwadkar
2020-11-20 19:15 ` [PATCH 03/15] e2fsck: port fc changes from kernel's recovery.c to e2fsck Harshad Shirwadkar
2020-12-02 16:54   ` Theodore Y. Ts'o
2020-11-20 19:15 ` [PATCH 04/15] mke2fs, dumpe2fs: make fast commit blocks configurable Harshad Shirwadkar
2020-12-02 18:29   ` Theodore Y. Ts'o
2020-11-20 19:15 ` [PATCH 05/15] mke2fs, tune2fs: update man page with fast commit info Harshad Shirwadkar
2020-12-02 18:33   ` Theodore Y. Ts'o
2020-12-10  5:24     ` harshad shirwadkar
2020-11-20 19:15 ` [PATCH 06/15] ext2fs: add new APIs needed for fast commits Harshad Shirwadkar
2020-12-02 18:44   ` Theodore Y. Ts'o [this message]
2020-12-10  1:45     ` harshad shirwadkar
2020-12-10 15:48       ` Theodore Y. Ts'o
2020-11-20 19:15 ` [PATCH 07/15] e2fsck: add function to rewrite extent tree Harshad Shirwadkar
2020-12-02 18:46   ` Theodore Y. Ts'o
2020-11-20 19:15 ` [PATCH 08/15] e2fsck: add fast commit setup code Harshad Shirwadkar
2020-12-02 18:48   ` Theodore Y. Ts'o
2020-11-20 19:16 ` [PATCH 09/15] e2fsck: add fast commit scan pass Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 10/15] e2fsck: add fast commit replay skeleton Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 11/15] e2fsck: add fc replay for link, unlink, creat tags Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 12/15] e2fsck: add replay for add_range, del_range, and inode tags Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 13/15] debugfs: add fast commit support to logdump Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 14/15] tests: add fast commit recovery tests Harshad Shirwadkar
2020-11-20 19:16 ` [PATCH 15/15] ext4: fix tests to account for new dumpe2fs output Harshad Shirwadkar

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=20201202184458.GJ390058@mit.edu \
    --to=tytso@mit.edu \
    --cc=harshadshirwadkar@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.