Patchwork [RFC,1/1] ext4: Undelete Feature for Ext4

login
register
mail settings
Submitter Lubos Uhliarik
Date March 18, 2014, 3:09 p.m.
Message ID <1395155384.15587.10.camel@zerobox.home>
Download mbox | patch
Permalink /patch/331456/
State New
Headers show

Comments

Lubos Uhliarik - March 18, 2014, 3:09 p.m.
This patch should make undelete process for deleted files under ext4 fs easier.

Signed-off-by: Lubos Uhliarik <uhliarik@seznam.cz>
---
 fs/ext4/ext4_extents.h | 14 +++++++++++
 fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 75 insertions(+), 6 deletions(-)
Andreas Dilger - March 18, 2014, 6:23 p.m.
On Mar 18, 2014, at 9:09 AM, Lubos Uhliarik <uhliarik@seznam.cz> wrote:
> This patch should make undelete process for deleted files under ext4 fs easier.

Hi Lubos,
thanks for the interesting patch.  This does seem like an improvement.
It would be good to add more description to your commit comment, since
it should explain what is actually being done in your patch (e.g. store
the extent count and depth in the ext4_extent_header eh_generation field,
...).

It would also be good to include a matching patch for e2fsprogs debugfs
in the "undelete" command.  I think one obstacle is the fact that inodes
have their blocks zeroed out by the kernel during truncation.  It would
be necessary to change the kernel slightly to avoid writing out i_blocks
with zero if the inode is being deleted.  Otherwise, this change is not
as useful as it could be.

Cheers, Andreas

> Signed-off-by: Lubos Uhliarik <uhliarik@seznam.cz>
> ---
> fs/ext4/ext4_extents.h | 14 +++++++++++
> fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 75 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> index 5074fe2..22cf2cd 100644
> --- a/fs/ext4/ext4_extents.h
> +++ b/fs/ext4/ext4_extents.h
> @@ -251,6 +251,20 @@ static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
> 				      0xffff);
> }
> 
> +static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
> +					  __u16 eh_entries){
> +	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
> +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
> +				& 0xffff0000);
> +}
> +
> +static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
> +					 __u16 eh_depth){
> +	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
> +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
> +				& 0x0000ffff);
> +}
> +
> /*
>  * ext4_idx_store_pblock:
>  * stores a large physical block number into an index struct,
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 74bc2d5..f0f3615 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2565,6 +2565,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> 	ext4_lblk_t ex_ee_block;
> 	unsigned short ex_ee_len;
> 	unsigned uninitialized = 0;
> +	unsigned short ex_ee_entries;
> 	struct ext4_extent *ex;
> 	ext4_fsblk_t pblk;
> 
> @@ -2584,6 +2585,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> 
> 	ex_ee_block = le32_to_cpu(ex->ee_block);
> 	ex_ee_len = ext4_ext_get_actual_len(ex);
> +	ex_ee_entries = le16_to_cpu(eh->eh_entries);
> 
> 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
> 
> @@ -2662,11 +2664,9 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> 		if (err)
> 			goto out;
> 
> -		if (num == 0)
> -			/* this extent is removed; mark slot entirely unused */
> -			ext4_ext_store_pblock(ex, 0);
> +		if (num != 0)
> +			ex->ee_len = cpu_to_le16(num);
> 
> -		ex->ee_len = cpu_to_le16(num);
> 		/*
> 		 * Do not mark uninitialized if all the blocks in the
> 		 * extent have been removed.
> @@ -2726,8 +2726,19 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> 
> 	/* if this leaf is free, then we should
> 	 * remove it from index block above */
> -	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
> +	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
> +		err = ext4_ext_get_access(handle, inode, path + depth);
> +		if (err)
> +			goto out;
> +
> +		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
> +
> +		err = ext4_ext_dirty(handle, inode, path + depth);
> +		if (err)
> +			goto out;
> +
> 		err = ext4_ext_rm_idx(handle, inode, path, depth);
> +	}
> 
> out:
> 	return err;
> @@ -2760,6 +2771,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> 	struct super_block *sb = inode->i_sb;
> 	int depth = ext_depth(inode);
> 	struct ext4_ext_path *path = NULL;
> +	unsigned short *entries_path = NULL;
> 	long long partial_cluster = 0;
> 	handle_t *handle;
> 	int i = 0, err = 0;
> @@ -2864,6 +2876,18 @@ again:
> 	}
> 	err = 0;
> 
> +	if (!inode->i_nlink) {
> +		entries_path = kzalloc(sizeof(short) * (depth + 1),
> +				       GFP_NOFS);
> +		if (entries_path == NULL) {
> +			kfree(path);
> +			ext4_journal_stop(handle);
> +			return -ENOMEM;
> +		}
> +
> +		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
> +	}
> +
> 	while (i >= 0 && err == 0) {
> 		if (i == depth) {
> 			/* this is leaf block */
> @@ -2890,6 +2914,10 @@ again:
> 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
> 				  path[i].p_hdr,
> 				  le16_to_cpu(path[i].p_hdr->eh_entries));
> +			if (entries_path != NULL) {
> +				entries_path[i] = le16_to_cpu(
> +					path[i].p_hdr->eh_entries);
> +			}
> 		} else {
> 			/* we were already here, see at next index */
> 			path[i].p_idx--;
> @@ -2928,10 +2956,24 @@ again:
> 		} else {
> 			/* we finished processing this index, go up */
> 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> +				if (entries_path != NULL) {
> +					err = ext4_ext_get_access(handle,
> +						inode, path);
> +					if (err == 0) {
> +						ext4_ext_store_entries(
> +							path[i].p_hdr,
> +							(entries_path[i]));
> +						err = ext4_ext_dirty(handle,
> +							inode, path);
> +					}
> +				}
> +
> 				/* index is empty, remove it;
> 				 * handle must be already prepared by the
> 				 * truncatei_leaf() */
> -				err = ext4_ext_rm_idx(handle, inode, path, i);
> +				if (err == 0)
> +					err = ext4_ext_rm_idx(handle, inode,
> +						path, i);
> 			}
> 			/* root level has p_bh == NULL, brelse() eats this */
> 			brelse(path[i].p_bh);
> @@ -2964,6 +3006,17 @@ again:
> 		 */
> 		err = ext4_ext_get_access(handle, inode, path);
> 		if (err == 0) {
> +			/* Store depth and entries to eh_generation to make
> +			 * recovering deleted files easier. */
> +			if (entries_path != NULL) {
> +				ext4_ext_store_entries(ext_inode_hdr(inode),
> +					(entries_path[0]));
> +
> +				ext4_ext_store_depth(ext_inode_hdr(inode),
> +					le16_to_cpu(
> +					ext_inode_hdr(inode)->eh_depth));
> +			}
> +
> 			ext_inode_hdr(inode)->eh_depth = 0;
> 			ext_inode_hdr(inode)->eh_max =
> 				cpu_to_le16(ext4_ext_space_root(inode, 0));
> @@ -2973,6 +3026,8 @@ again:
> out:
> 	ext4_ext_drop_refs(path);
> 	kfree(path);
> +	kfree(entries_path);
> +
> 	if (err == -EAGAIN) {
> 		path = NULL;
> 		goto again;
> -- 
> 1.8.3.2
> 
> 
> 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas
Lubos Uhliarik - March 19, 2014, 1:59 p.m.
Hi Andreas,

maybe you didn't notice, but I sent comments in email with subject
"[RFC][PATCH 0/1] ext4: Undelete Feature for Ext4". Maybe, I should
mention this in [PATCH 1/1]. In this mail is also link to git repository
with undelete app (uses ext2fslibs), which demonstrates how to undelete
file with changes in kernel.

I hope, it will help you.

Regards,

Lubos

Andreas Dilger píše v Út 18. 03. 2014 v 12:23 -0600:
> On Mar 18, 2014, at 9:09 AM, Lubos Uhliarik <uhliarik@seznam.cz> wrote:
> > This patch should make undelete process for deleted files under ext4 fs easier.
> 
> Hi Lubos,
> thanks for the interesting patch.  This does seem like an improvement.
> It would be good to add more description to your commit comment, since
> it should explain what is actually being done in your patch (e.g. store
> the extent count and depth in the ext4_extent_header eh_generation field,
> ...).
> 
> It would also be good to include a matching patch for e2fsprogs debugfs
> in the "undelete" command.  I think one obstacle is the fact that inodes
> have their blocks zeroed out by the kernel during truncation.  It would
> be necessary to change the kernel slightly to avoid writing out i_blocks
> with zero if the inode is being deleted.  Otherwise, this change is not
> as useful as it could be.
> 
> Cheers, Andreas
> 
> > Signed-off-by: Lubos Uhliarik <uhliarik@seznam.cz>
> > ---
> > fs/ext4/ext4_extents.h | 14 +++++++++++
> > fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 75 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> > index 5074fe2..22cf2cd 100644
> > --- a/fs/ext4/ext4_extents.h
> > +++ b/fs/ext4/ext4_extents.h
> > @@ -251,6 +251,20 @@ static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
> > 				      0xffff);
> > }
> > 
> > +static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
> > +					  __u16 eh_entries){
> > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
> > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
> > +				& 0xffff0000);
> > +}
> > +
> > +static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
> > +					 __u16 eh_depth){
> > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
> > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
> > +				& 0x0000ffff);
> > +}
> > +
> > /*
> >  * ext4_idx_store_pblock:
> >  * stores a large physical block number into an index struct,
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index 74bc2d5..f0f3615 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -2565,6 +2565,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 	ext4_lblk_t ex_ee_block;
> > 	unsigned short ex_ee_len;
> > 	unsigned uninitialized = 0;
> > +	unsigned short ex_ee_entries;
> > 	struct ext4_extent *ex;
> > 	ext4_fsblk_t pblk;
> > 
> > @@ -2584,6 +2585,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 
> > 	ex_ee_block = le32_to_cpu(ex->ee_block);
> > 	ex_ee_len = ext4_ext_get_actual_len(ex);
> > +	ex_ee_entries = le16_to_cpu(eh->eh_entries);
> > 
> > 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
> > 
> > @@ -2662,11 +2664,9 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 		if (err)
> > 			goto out;
> > 
> > -		if (num == 0)
> > -			/* this extent is removed; mark slot entirely unused */
> > -			ext4_ext_store_pblock(ex, 0);
> > +		if (num != 0)
> > +			ex->ee_len = cpu_to_le16(num);
> > 
> > -		ex->ee_len = cpu_to_le16(num);
> > 		/*
> > 		 * Do not mark uninitialized if all the blocks in the
> > 		 * extent have been removed.
> > @@ -2726,8 +2726,19 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 
> > 	/* if this leaf is free, then we should
> > 	 * remove it from index block above */
> > -	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
> > +	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
> > +		err = ext4_ext_get_access(handle, inode, path + depth);
> > +		if (err)
> > +			goto out;
> > +
> > +		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
> > +
> > +		err = ext4_ext_dirty(handle, inode, path + depth);
> > +		if (err)
> > +			goto out;
> > +
> > 		err = ext4_ext_rm_idx(handle, inode, path, depth);
> > +	}
> > 
> > out:
> > 	return err;
> > @@ -2760,6 +2771,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> > 	struct super_block *sb = inode->i_sb;
> > 	int depth = ext_depth(inode);
> > 	struct ext4_ext_path *path = NULL;
> > +	unsigned short *entries_path = NULL;
> > 	long long partial_cluster = 0;
> > 	handle_t *handle;
> > 	int i = 0, err = 0;
> > @@ -2864,6 +2876,18 @@ again:
> > 	}
> > 	err = 0;
> > 
> > +	if (!inode->i_nlink) {
> > +		entries_path = kzalloc(sizeof(short) * (depth + 1),
> > +				       GFP_NOFS);
> > +		if (entries_path == NULL) {
> > +			kfree(path);
> > +			ext4_journal_stop(handle);
> > +			return -ENOMEM;
> > +		}
> > +
> > +		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
> > +	}
> > +
> > 	while (i >= 0 && err == 0) {
> > 		if (i == depth) {
> > 			/* this is leaf block */
> > @@ -2890,6 +2914,10 @@ again:
> > 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
> > 				  path[i].p_hdr,
> > 				  le16_to_cpu(path[i].p_hdr->eh_entries));
> > +			if (entries_path != NULL) {
> > +				entries_path[i] = le16_to_cpu(
> > +					path[i].p_hdr->eh_entries);
> > +			}
> > 		} else {
> > 			/* we were already here, see at next index */
> > 			path[i].p_idx--;
> > @@ -2928,10 +2956,24 @@ again:
> > 		} else {
> > 			/* we finished processing this index, go up */
> > 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> > +				if (entries_path != NULL) {
> > +					err = ext4_ext_get_access(handle,
> > +						inode, path);
> > +					if (err == 0) {
> > +						ext4_ext_store_entries(
> > +							path[i].p_hdr,
> > +							(entries_path[i]));
> > +						err = ext4_ext_dirty(handle,
> > +							inode, path);
> > +					}
> > +				}
> > +
> > 				/* index is empty, remove it;
> > 				 * handle must be already prepared by the
> > 				 * truncatei_leaf() */
> > -				err = ext4_ext_rm_idx(handle, inode, path, i);
> > +				if (err == 0)
> > +					err = ext4_ext_rm_idx(handle, inode,
> > +						path, i);
> > 			}
> > 			/* root level has p_bh == NULL, brelse() eats this */
> > 			brelse(path[i].p_bh);
> > @@ -2964,6 +3006,17 @@ again:
> > 		 */
> > 		err = ext4_ext_get_access(handle, inode, path);
> > 		if (err == 0) {
> > +			/* Store depth and entries to eh_generation to make
> > +			 * recovering deleted files easier. */
> > +			if (entries_path != NULL) {
> > +				ext4_ext_store_entries(ext_inode_hdr(inode),
> > +					(entries_path[0]));
> > +
> > +				ext4_ext_store_depth(ext_inode_hdr(inode),
> > +					le16_to_cpu(
> > +					ext_inode_hdr(inode)->eh_depth));
> > +			}
> > +
> > 			ext_inode_hdr(inode)->eh_depth = 0;
> > 			ext_inode_hdr(inode)->eh_max =
> > 				cpu_to_le16(ext4_ext_space_root(inode, 0));
> > @@ -2973,6 +3026,8 @@ again:
> > out:
> > 	ext4_ext_drop_refs(path);
> > 	kfree(path);
> > +	kfree(entries_path);
> > +
> > 	if (err == -EAGAIN) {
> > 		path = NULL;
> > 		goto again;
> > -- 
> > 1.8.3.2
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Czerner - March 19, 2014, 2:59 p.m.
On Wed, 19 Mar 2014, Lubos Uhliarik wrote:

> Date: Wed, 19 Mar 2014 14:59:04 +0100
> From: Lubos Uhliarik <uhliarik@seznam.cz>
> To: Andreas Dilger <adilger@dilger.ca>
> Cc: linux-ext4@vger.kernel.org, vojnar@fit.vutbr.cz, lczerner@redhat.com
> Subject: Re: [RFC][PATCH 1/1] ext4: Undelete Feature for Ext4
> 
> Hi Andreas,
> 
> maybe you didn't notice, but I sent comments in email with subject
> "[RFC][PATCH 0/1] ext4: Undelete Feature for Ext4". Maybe, I should
> mention this in [PATCH 1/1]. In this mail is also link to git repository
> with undelete app (uses ext2fslibs), which demonstrates how to undelete
> file with changes in kernel.

Hi Lubos,

you should definitely put some description into PATCH 1/1 as well -
that's what will eventually end up in the git repository and it
should contain description so that people know what it is all about.

Having e2fsprogs debugfs undelete command is actually a very good
idea. But I think this should be very easy to do with your current
program, or in addition to what you currently have since you're
already using ext2fs library.

Thanks!
-Lukas

> 
> I hope, it will help you.
> 
> Regards,
> 
> Lubos
> 
> Andreas Dilger píše v Út 18. 03. 2014 v 12:23 -0600:
> > On Mar 18, 2014, at 9:09 AM, Lubos Uhliarik <uhliarik@seznam.cz> wrote:
> > > This patch should make undelete process for deleted files under ext4 fs easier.
> > 
> > Hi Lubos,
> > thanks for the interesting patch.  This does seem like an improvement.
> > It would be good to add more description to your commit comment, since
> > it should explain what is actually being done in your patch (e.g. store
> > the extent count and depth in the ext4_extent_header eh_generation field,
> > ...).
> > 
> > It would also be good to include a matching patch for e2fsprogs debugfs
> > in the "undelete" command.  I think one obstacle is the fact that inodes
> > have their blocks zeroed out by the kernel during truncation.  It would
> > be necessary to change the kernel slightly to avoid writing out i_blocks
> > with zero if the inode is being deleted.  Otherwise, this change is not
> > as useful as it could be.
> > 
> > Cheers, Andreas
> > 
> > > Signed-off-by: Lubos Uhliarik <uhliarik@seznam.cz>
> > > ---
> > > fs/ext4/ext4_extents.h | 14 +++++++++++
> > > fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
> > > 2 files changed, 75 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> > > index 5074fe2..22cf2cd 100644
> > > --- a/fs/ext4/ext4_extents.h
> > > +++ b/fs/ext4/ext4_extents.h
> > > @@ -251,6 +251,20 @@ static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
> > > 				      0xffff);
> > > }
> > > 
> > > +static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
> > > +					  __u16 eh_entries){
> > > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
> > > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
> > > +				& 0xffff0000);
> > > +}
> > > +
> > > +static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
> > > +					 __u16 eh_depth){
> > > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
> > > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
> > > +				& 0x0000ffff);
> > > +}
> > > +
> > > /*
> > >  * ext4_idx_store_pblock:
> > >  * stores a large physical block number into an index struct,
> > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > > index 74bc2d5..f0f3615 100644
> > > --- a/fs/ext4/extents.c
> > > +++ b/fs/ext4/extents.c
> > > @@ -2565,6 +2565,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 	ext4_lblk_t ex_ee_block;
> > > 	unsigned short ex_ee_len;
> > > 	unsigned uninitialized = 0;
> > > +	unsigned short ex_ee_entries;
> > > 	struct ext4_extent *ex;
> > > 	ext4_fsblk_t pblk;
> > > 
> > > @@ -2584,6 +2585,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 
> > > 	ex_ee_block = le32_to_cpu(ex->ee_block);
> > > 	ex_ee_len = ext4_ext_get_actual_len(ex);
> > > +	ex_ee_entries = le16_to_cpu(eh->eh_entries);
> > > 
> > > 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
> > > 
> > > @@ -2662,11 +2664,9 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 		if (err)
> > > 			goto out;
> > > 
> > > -		if (num == 0)
> > > -			/* this extent is removed; mark slot entirely unused */
> > > -			ext4_ext_store_pblock(ex, 0);
> > > +		if (num != 0)
> > > +			ex->ee_len = cpu_to_le16(num);
> > > 
> > > -		ex->ee_len = cpu_to_le16(num);
> > > 		/*
> > > 		 * Do not mark uninitialized if all the blocks in the
> > > 		 * extent have been removed.
> > > @@ -2726,8 +2726,19 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > > 
> > > 	/* if this leaf is free, then we should
> > > 	 * remove it from index block above */
> > > -	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
> > > +	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
> > > +		err = ext4_ext_get_access(handle, inode, path + depth);
> > > +		if (err)
> > > +			goto out;
> > > +
> > > +		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
> > > +
> > > +		err = ext4_ext_dirty(handle, inode, path + depth);
> > > +		if (err)
> > > +			goto out;
> > > +
> > > 		err = ext4_ext_rm_idx(handle, inode, path, depth);
> > > +	}
> > > 
> > > out:
> > > 	return err;
> > > @@ -2760,6 +2771,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> > > 	struct super_block *sb = inode->i_sb;
> > > 	int depth = ext_depth(inode);
> > > 	struct ext4_ext_path *path = NULL;
> > > +	unsigned short *entries_path = NULL;
> > > 	long long partial_cluster = 0;
> > > 	handle_t *handle;
> > > 	int i = 0, err = 0;
> > > @@ -2864,6 +2876,18 @@ again:
> > > 	}
> > > 	err = 0;
> > > 
> > > +	if (!inode->i_nlink) {
> > > +		entries_path = kzalloc(sizeof(short) * (depth + 1),
> > > +				       GFP_NOFS);
> > > +		if (entries_path == NULL) {
> > > +			kfree(path);
> > > +			ext4_journal_stop(handle);
> > > +			return -ENOMEM;
> > > +		}
> > > +
> > > +		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
> > > +	}
> > > +
> > > 	while (i >= 0 && err == 0) {
> > > 		if (i == depth) {
> > > 			/* this is leaf block */
> > > @@ -2890,6 +2914,10 @@ again:
> > > 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
> > > 				  path[i].p_hdr,
> > > 				  le16_to_cpu(path[i].p_hdr->eh_entries));
> > > +			if (entries_path != NULL) {
> > > +				entries_path[i] = le16_to_cpu(
> > > +					path[i].p_hdr->eh_entries);
> > > +			}
> > > 		} else {
> > > 			/* we were already here, see at next index */
> > > 			path[i].p_idx--;
> > > @@ -2928,10 +2956,24 @@ again:
> > > 		} else {
> > > 			/* we finished processing this index, go up */
> > > 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> > > +				if (entries_path != NULL) {
> > > +					err = ext4_ext_get_access(handle,
> > > +						inode, path);
> > > +					if (err == 0) {
> > > +						ext4_ext_store_entries(
> > > +							path[i].p_hdr,
> > > +							(entries_path[i]));
> > > +						err = ext4_ext_dirty(handle,
> > > +							inode, path);
> > > +					}
> > > +				}
> > > +
> > > 				/* index is empty, remove it;
> > > 				 * handle must be already prepared by the
> > > 				 * truncatei_leaf() */
> > > -				err = ext4_ext_rm_idx(handle, inode, path, i);
> > > +				if (err == 0)
> > > +					err = ext4_ext_rm_idx(handle, inode,
> > > +						path, i);
> > > 			}
> > > 			/* root level has p_bh == NULL, brelse() eats this */
> > > 			brelse(path[i].p_bh);
> > > @@ -2964,6 +3006,17 @@ again:
> > > 		 */
> > > 		err = ext4_ext_get_access(handle, inode, path);
> > > 		if (err == 0) {
> > > +			/* Store depth and entries to eh_generation to make
> > > +			 * recovering deleted files easier. */
> > > +			if (entries_path != NULL) {
> > > +				ext4_ext_store_entries(ext_inode_hdr(inode),
> > > +					(entries_path[0]));
> > > +
> > > +				ext4_ext_store_depth(ext_inode_hdr(inode),
> > > +					le16_to_cpu(
> > > +					ext_inode_hdr(inode)->eh_depth));
> > > +			}
> > > +
> > > 			ext_inode_hdr(inode)->eh_depth = 0;
> > > 			ext_inode_hdr(inode)->eh_max =
> > > 				cpu_to_le16(ext4_ext_space_root(inode, 0));
> > > @@ -2973,6 +3026,8 @@ again:
> > > out:
> > > 	ext4_ext_drop_refs(path);
> > > 	kfree(path);
> > > +	kfree(entries_path);
> > > +
> > > 	if (err == -EAGAIN) {
> > > 		path = NULL;
> > > 		goto again;
> > > -- 
> > > 1.8.3.2
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > Cheers, Andreas
> > 
> > 
> > 
> > 
> > 
> 
> 
>
Lubos Uhliarik - April 18, 2014, 3:48 p.m.
Hi Andreas,

I will try to create matching patch for e2fsprogs debugfs later, after I
will finish my patch for ext4. 

You wrote, that you think inodes have their blocks zeroed out by kernel
during truncation, but in fact, it's not happening (I tested it and
there are only changes in extent header and ext4_extents structs...
Extent_indexes are untached by delete process.

Lubos

Andreas Dilger píše v Út 18. 03. 2014 v 12:23 -0600:
> On Mar 18, 2014, at 9:09 AM, Lubos Uhliarik <uhliarik@seznam.cz> wrote:
> > This patch should make undelete process for deleted files under ext4 fs easier.
> 
> Hi Lubos,
> thanks for the interesting patch.  This does seem like an improvement.
> It would be good to add more description to your commit comment, since
> it should explain what is actually being done in your patch (e.g. store
> the extent count and depth in the ext4_extent_header eh_generation field,
> ...).
> 
> It would also be good to include a matching patch for e2fsprogs debugfs
> in the "undelete" command.  I think one obstacle is the fact that inodes
> have their blocks zeroed out by the kernel during truncation.  It would
> be necessary to change the kernel slightly to avoid writing out i_blocks
> with zero if the inode is being deleted.  Otherwise, this change is not
> as useful as it could be.
> 
> Cheers, Andreas
> 
> > Signed-off-by: Lubos Uhliarik <uhliarik@seznam.cz>
> > ---
> > fs/ext4/ext4_extents.h | 14 +++++++++++
> > fs/ext4/extents.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 75 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> > index 5074fe2..22cf2cd 100644
> > --- a/fs/ext4/ext4_extents.h
> > +++ b/fs/ext4/ext4_extents.h
> > @@ -251,6 +251,20 @@ static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
> > 				      0xffff);
> > }
> > 
> > +static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
> > +					  __u16 eh_entries){
> > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
> > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
> > +				& 0xffff0000);
> > +}
> > +
> > +static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
> > +					 __u16 eh_depth){
> > +	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
> > +	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
> > +				& 0x0000ffff);
> > +}
> > +
> > /*
> >  * ext4_idx_store_pblock:
> >  * stores a large physical block number into an index struct,
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index 74bc2d5..f0f3615 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -2565,6 +2565,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 	ext4_lblk_t ex_ee_block;
> > 	unsigned short ex_ee_len;
> > 	unsigned uninitialized = 0;
> > +	unsigned short ex_ee_entries;
> > 	struct ext4_extent *ex;
> > 	ext4_fsblk_t pblk;
> > 
> > @@ -2584,6 +2585,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 
> > 	ex_ee_block = le32_to_cpu(ex->ee_block);
> > 	ex_ee_len = ext4_ext_get_actual_len(ex);
> > +	ex_ee_entries = le16_to_cpu(eh->eh_entries);
> > 
> > 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
> > 
> > @@ -2662,11 +2664,9 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 		if (err)
> > 			goto out;
> > 
> > -		if (num == 0)
> > -			/* this extent is removed; mark slot entirely unused */
> > -			ext4_ext_store_pblock(ex, 0);
> > +		if (num != 0)
> > +			ex->ee_len = cpu_to_le16(num);
> > 
> > -		ex->ee_len = cpu_to_le16(num);
> > 		/*
> > 		 * Do not mark uninitialized if all the blocks in the
> > 		 * extent have been removed.
> > @@ -2726,8 +2726,19 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
> > 
> > 	/* if this leaf is free, then we should
> > 	 * remove it from index block above */
> > -	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
> > +	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
> > +		err = ext4_ext_get_access(handle, inode, path + depth);
> > +		if (err)
> > +			goto out;
> > +
> > +		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
> > +
> > +		err = ext4_ext_dirty(handle, inode, path + depth);
> > +		if (err)
> > +			goto out;
> > +
> > 		err = ext4_ext_rm_idx(handle, inode, path, depth);
> > +	}
> > 
> > out:
> > 	return err;
> > @@ -2760,6 +2771,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> > 	struct super_block *sb = inode->i_sb;
> > 	int depth = ext_depth(inode);
> > 	struct ext4_ext_path *path = NULL;
> > +	unsigned short *entries_path = NULL;
> > 	long long partial_cluster = 0;
> > 	handle_t *handle;
> > 	int i = 0, err = 0;
> > @@ -2864,6 +2876,18 @@ again:
> > 	}
> > 	err = 0;
> > 
> > +	if (!inode->i_nlink) {
> > +		entries_path = kzalloc(sizeof(short) * (depth + 1),
> > +				       GFP_NOFS);
> > +		if (entries_path == NULL) {
> > +			kfree(path);
> > +			ext4_journal_stop(handle);
> > +			return -ENOMEM;
> > +		}
> > +
> > +		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
> > +	}
> > +
> > 	while (i >= 0 && err == 0) {
> > 		if (i == depth) {
> > 			/* this is leaf block */
> > @@ -2890,6 +2914,10 @@ again:
> > 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
> > 				  path[i].p_hdr,
> > 				  le16_to_cpu(path[i].p_hdr->eh_entries));
> > +			if (entries_path != NULL) {
> > +				entries_path[i] = le16_to_cpu(
> > +					path[i].p_hdr->eh_entries);
> > +			}
> > 		} else {
> > 			/* we were already here, see at next index */
> > 			path[i].p_idx--;
> > @@ -2928,10 +2956,24 @@ again:
> > 		} else {
> > 			/* we finished processing this index, go up */
> > 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> > +				if (entries_path != NULL) {
> > +					err = ext4_ext_get_access(handle,
> > +						inode, path);
> > +					if (err == 0) {
> > +						ext4_ext_store_entries(
> > +							path[i].p_hdr,
> > +							(entries_path[i]));
> > +						err = ext4_ext_dirty(handle,
> > +							inode, path);
> > +					}
> > +				}
> > +
> > 				/* index is empty, remove it;
> > 				 * handle must be already prepared by the
> > 				 * truncatei_leaf() */
> > -				err = ext4_ext_rm_idx(handle, inode, path, i);
> > +				if (err == 0)
> > +					err = ext4_ext_rm_idx(handle, inode,
> > +						path, i);
> > 			}
> > 			/* root level has p_bh == NULL, brelse() eats this */
> > 			brelse(path[i].p_bh);
> > @@ -2964,6 +3006,17 @@ again:
> > 		 */
> > 		err = ext4_ext_get_access(handle, inode, path);
> > 		if (err == 0) {
> > +			/* Store depth and entries to eh_generation to make
> > +			 * recovering deleted files easier. */
> > +			if (entries_path != NULL) {
> > +				ext4_ext_store_entries(ext_inode_hdr(inode),
> > +					(entries_path[0]));
> > +
> > +				ext4_ext_store_depth(ext_inode_hdr(inode),
> > +					le16_to_cpu(
> > +					ext_inode_hdr(inode)->eh_depth));
> > +			}
> > +
> > 			ext_inode_hdr(inode)->eh_depth = 0;
> > 			ext_inode_hdr(inode)->eh_max =
> > 				cpu_to_le16(ext4_ext_space_root(inode, 0));
> > @@ -2973,6 +3026,8 @@ again:
> > out:
> > 	ext4_ext_drop_refs(path);
> > 	kfree(path);
> > +	kfree(entries_path);
> > +
> > 	if (err == -EAGAIN) {
> > 		path = NULL;
> > 		goto again;
> > -- 
> > 1.8.3.2
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index 5074fe2..22cf2cd 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -251,6 +251,20 @@  static inline void ext4_ext_store_pblock(struct ext4_extent *ex,
 				      0xffff);
 }
 
+static inline void ext4_ext_store_entries(struct ext4_extent_header *eh,
+					  __u16 eh_entries){
+	eh->eh_generation &= cpu_to_le32((unsigned long) 0x0000ffff);
+	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_entries) << 16)
+				& 0xffff0000);
+}
+
+static inline void ext4_ext_store_depth(struct ext4_extent_header *eh,
+					 __u16 eh_depth){
+	eh->eh_generation &= cpu_to_le32((unsigned long) 0xffff0000);
+	eh->eh_generation |= cpu_to_le32(((unsigned long) (eh_depth))
+				& 0x0000ffff);
+}
+
 /*
  * ext4_idx_store_pblock:
  * stores a large physical block number into an index struct,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 74bc2d5..f0f3615 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2565,6 +2565,7 @@  ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
 	ext4_lblk_t ex_ee_block;
 	unsigned short ex_ee_len;
 	unsigned uninitialized = 0;
+	unsigned short ex_ee_entries;
 	struct ext4_extent *ex;
 	ext4_fsblk_t pblk;
 
@@ -2584,6 +2585,7 @@  ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
 
 	ex_ee_block = le32_to_cpu(ex->ee_block);
 	ex_ee_len = ext4_ext_get_actual_len(ex);
+	ex_ee_entries = le16_to_cpu(eh->eh_entries);
 
 	trace_ext4_ext_rm_leaf(inode, start, ex, *partial_cluster);
 
@@ -2662,11 +2664,9 @@  ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
 		if (err)
 			goto out;
 
-		if (num == 0)
-			/* this extent is removed; mark slot entirely unused */
-			ext4_ext_store_pblock(ex, 0);
+		if (num != 0)
+			ex->ee_len = cpu_to_le16(num);
 
-		ex->ee_len = cpu_to_le16(num);
 		/*
 		 * Do not mark uninitialized if all the blocks in the
 		 * extent have been removed.
@@ -2726,8 +2726,19 @@  ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
 
 	/* if this leaf is free, then we should
 	 * remove it from index block above */
-	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
+	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
+		err = ext4_ext_get_access(handle, inode, path + depth);
+		if (err)
+			goto out;
+
+		ext4_ext_store_entries(path[depth].p_hdr, ex_ee_entries);
+
+		err = ext4_ext_dirty(handle, inode, path + depth);
+		if (err)
+			goto out;
+
 		err = ext4_ext_rm_idx(handle, inode, path, depth);
+	}
 
 out:
 	return err;
@@ -2760,6 +2771,7 @@  int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
 	struct super_block *sb = inode->i_sb;
 	int depth = ext_depth(inode);
 	struct ext4_ext_path *path = NULL;
+	unsigned short *entries_path = NULL;
 	long long partial_cluster = 0;
 	handle_t *handle;
 	int i = 0, err = 0;
@@ -2864,6 +2876,18 @@  again:
 	}
 	err = 0;
 
+	if (!inode->i_nlink) {
+		entries_path = kzalloc(sizeof(short) * (depth + 1),
+				       GFP_NOFS);
+		if (entries_path == NULL) {
+			kfree(path);
+			ext4_journal_stop(handle);
+			return -ENOMEM;
+		}
+
+		entries_path[0] = le16_to_cpu(ext_inode_hdr(inode)->eh_entries);
+	}
+
 	while (i >= 0 && err == 0) {
 		if (i == depth) {
 			/* this is leaf block */
@@ -2890,6 +2914,10 @@  again:
 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
 				  path[i].p_hdr,
 				  le16_to_cpu(path[i].p_hdr->eh_entries));
+			if (entries_path != NULL) {
+				entries_path[i] = le16_to_cpu(
+					path[i].p_hdr->eh_entries);
+			}
 		} else {
 			/* we were already here, see at next index */
 			path[i].p_idx--;
@@ -2928,10 +2956,24 @@  again:
 		} else {
 			/* we finished processing this index, go up */
 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
+				if (entries_path != NULL) {
+					err = ext4_ext_get_access(handle,
+						inode, path);
+					if (err == 0) {
+						ext4_ext_store_entries(
+							path[i].p_hdr,
+							(entries_path[i]));
+						err = ext4_ext_dirty(handle,
+							inode, path);
+					}
+				}
+
 				/* index is empty, remove it;
 				 * handle must be already prepared by the
 				 * truncatei_leaf() */
-				err = ext4_ext_rm_idx(handle, inode, path, i);
+				if (err == 0)
+					err = ext4_ext_rm_idx(handle, inode,
+						path, i);
 			}
 			/* root level has p_bh == NULL, brelse() eats this */
 			brelse(path[i].p_bh);
@@ -2964,6 +3006,17 @@  again:
 		 */
 		err = ext4_ext_get_access(handle, inode, path);
 		if (err == 0) {
+			/* Store depth and entries to eh_generation to make
+			 * recovering deleted files easier. */
+			if (entries_path != NULL) {
+				ext4_ext_store_entries(ext_inode_hdr(inode),
+					(entries_path[0]));
+
+				ext4_ext_store_depth(ext_inode_hdr(inode),
+					le16_to_cpu(
+					ext_inode_hdr(inode)->eh_depth));
+			}
+
 			ext_inode_hdr(inode)->eh_depth = 0;
 			ext_inode_hdr(inode)->eh_max =
 				cpu_to_le16(ext4_ext_space_root(inode, 0));
@@ -2973,6 +3026,8 @@  again:
 out:
 	ext4_ext_drop_refs(path);
 	kfree(path);
+	kfree(entries_path);
+
 	if (err == -EAGAIN) {
 		path = NULL;
 		goto again;