diff mbox series

[1/2] ext4: return lblk from ext4_find_entry

Message ID 20200325093728.204211-1-harshadshirwadkar@gmail.com
State New
Headers show
Series [1/2] ext4: return lblk from ext4_find_entry | expand

Commit Message

harshad shirwadkar March 25, 2020, 9:37 a.m. UTC 
This patch makes ext4_find_entry and related routines to return
logical block address of the dirent block. This logical block address
is used in the directory shrinking code to perform reverse lookup and
verify that the lookup was successful.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 fs/ext4/namei.c | 54 +++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

Comments

Andreas Dilger March 28, 2020, 11:24 p.m. UTC | #1 
On Mar 25, 2020, at 3:37 AM, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
> 
> This patch makes ext4_find_entry and related routines to return
> logical block address of the dirent block. This logical block address
> is used in the directory shrinking code to perform reverse lookup and
> verify that the lookup was successful.
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> ---
> fs/ext4/namei.c | 54 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 32 insertions(+), 22 deletions(-)

Would it make sense to add the "lblk" field to struct ext4_renament,
rather than adding an extra argument for all of these functions?

Otherwise, the patch looks OK.

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
> -			       int force_reread)
> +			       int force_reread, ext4_lblk_t lblk)
> {
> 	int retval;
> 	/*
> @@ -3593,7 +3600,8 @@ static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
> 		retval = ext4_find_delete_entry(handle, ent->dir,
> 						&ent->dentry->d_name);
> 	} else {
> -		retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh);
> +		retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh,
> +					   lblk);
> 		if (retval == -ENOENT) {
> 			retval = ext4_find_delete_entry(handle, ent->dir,
> 							&ent->dentry->d_name);
> @@ -3679,6 +3687,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> 	struct inode *whiteout = NULL;
> 	int credits;
> 	u8 old_file_type;
> +	ext4_lblk_t lblk;
> 
> 	if (new.inode && new.inode->i_nlink == 0) {
> 		EXT4_ERROR_INODE(new.inode,
> @@ -3706,7 +3715,8 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> 			return retval;
> 	}
> 
> -	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL);
> +	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL,
> +				 &lblk);
> 	if (IS_ERR(old.bh))
> 		return PTR_ERR(old.bh);
> 	/*
> @@ -3720,7 +3730,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> 		goto end_rename;
> 
> 	new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
> -				 &new.de, &new.inlined);
> +				 &new.de, &new.inlined, NULL);
> 	if (IS_ERR(new.bh)) {
> 		retval = PTR_ERR(new.bh);
> 		new.bh = NULL;
> @@ -3817,7 +3827,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> 		/*
> 		 * ok, that's it
> 		 */
> -		ext4_rename_delete(handle, &old, force_reread);
> +		ext4_rename_delete(handle, &old, force_reread, lblk);
> 	}
> 
> 	if (new.inode) {
> @@ -3900,7 +3910,7 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> 		return retval;
> 
> 	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name,
> -				 &old.de, &old.inlined);
> +				 &old.de, &old.inlined, NULL);
> 	if (IS_ERR(old.bh))
> 		return PTR_ERR(old.bh);
> 	/*
> @@ -3914,7 +3924,7 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> 		goto end_rename;
> 
> 	new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
> -				 &new.de, &new.inlined);
> +				 &new.de, &new.inlined, NULL);
> 	if (IS_ERR(new.bh)) {
> 		retval = PTR_ERR(new.bh);
> 		new.bh = NULL;
> --
> 2.25.1.696.g5e7596f4ac-goog
> 


Cheers, Andreas
harshad shirwadkar April 7, 2020, 6:35 a.m. UTC | #2 
On Sat, Mar 28, 2020 at 4:24 PM Andreas Dilger <adilger@dilger.ca> wrote:
>
> On Mar 25, 2020, at 3:37 AM, Harshad Shirwadkar <harshadshirwadkar@gmail.com> wrote:
> >
> > This patch makes ext4_find_entry and related routines to return
> > logical block address of the dirent block. This logical block address
> > is used in the directory shrinking code to perform reverse lookup and
> > verify that the lookup was successful.
> >
> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> > ---
> > fs/ext4/namei.c | 54 +++++++++++++++++++++++++++++--------------------
> > 1 file changed, 32 insertions(+), 22 deletions(-)
>
> Would it make sense to add the "lblk" field to struct ext4_renament,
> rather than adding an extra argument for all of these functions?

ext4_renament is only available in ext4_rename_delete(), for other
callers we still need a way to use lblk. So, I am not sure if adding
lblk to ext4_renament is helpful. Am I missing something here?


>
> Otherwise, the patch looks OK.
>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
>
> > static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
> > -                            int force_reread)
> > +                            int force_reread, ext4_lblk_t lblk)
> > {
> >       int retval;
> >       /*
> > @@ -3593,7 +3600,8 @@ static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
> >               retval = ext4_find_delete_entry(handle, ent->dir,
> >                                               &ent->dentry->d_name);
> >       } else {
> > -             retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh);
> > +             retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh,
> > +                                        lblk);
> >               if (retval == -ENOENT) {
> >                       retval = ext4_find_delete_entry(handle, ent->dir,
> >                                                       &ent->dentry->d_name);
> > @@ -3679,6 +3687,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> >       struct inode *whiteout = NULL;
> >       int credits;
> >       u8 old_file_type;
> > +     ext4_lblk_t lblk;
> >
> >       if (new.inode && new.inode->i_nlink == 0) {
> >               EXT4_ERROR_INODE(new.inode,
> > @@ -3706,7 +3715,8 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> >                       return retval;
> >       }
> >
> > -     old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL);
> > +     old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL,
> > +                              &lblk);
> >       if (IS_ERR(old.bh))
> >               return PTR_ERR(old.bh);
> >       /*
> > @@ -3720,7 +3730,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> >               goto end_rename;
> >
> >       new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
> > -                              &new.de, &new.inlined);
> > +                              &new.de, &new.inlined, NULL);
> >       if (IS_ERR(new.bh)) {
> >               retval = PTR_ERR(new.bh);
> >               new.bh = NULL;
> > @@ -3817,7 +3827,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> >               /*
> >                * ok, that's it
> >                */
> > -             ext4_rename_delete(handle, &old, force_reread);
> > +             ext4_rename_delete(handle, &old, force_reread, lblk);
> >       }
> >
> >       if (new.inode) {
> > @@ -3900,7 +3910,7 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> >               return retval;
> >
> >       old.bh = ext4_find_entry(old.dir, &old.dentry->d_name,
> > -                              &old.de, &old.inlined);
> > +                              &old.de, &old.inlined, NULL);
> >       if (IS_ERR(old.bh))
> >               return PTR_ERR(old.bh);
> >       /*
> > @@ -3914,7 +3924,7 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> >               goto end_rename;
> >
> >       new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
> > -                              &new.de, &new.inlined);
> > +                              &new.de, &new.inlined, NULL);
> >       if (IS_ERR(new.bh)) {
> >               retval = PTR_ERR(new.bh);
> >               new.bh = NULL;
> > --
> > 2.25.1.696.g5e7596f4ac-goog
> >
>
>
> Cheers, Andreas
>
>
>
>
>
diff mbox series

Patch

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index b05ea72f38fd..d567b9589875 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -295,7 +295,7 @@  static int ext4_htree_next_block(struct inode *dir, __u32 hash,
 				 __u32 *start_hash);
 static struct buffer_head * ext4_dx_find_entry(struct inode *dir,
 		struct ext4_filename *fname,
-		struct ext4_dir_entry_2 **res_dir);
+		struct ext4_dir_entry_2 **res_dir, ext4_lblk_t *lblk);
 static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
 			     struct inode *dir, struct inode *inode);
 
@@ -1443,7 +1443,7 @@  static int is_dx_internal_node(struct inode *dir, ext4_lblk_t block,
 static struct buffer_head *__ext4_find_entry(struct inode *dir,
 					     struct ext4_filename *fname,
 					     struct ext4_dir_entry_2 **res_dir,
-					     int *inlined)
+					     int *inlined, ext4_lblk_t *lblk)
 {
 	struct super_block *sb;
 	struct buffer_head *bh_use[NAMEI_RA_SIZE];
@@ -1485,7 +1485,7 @@  static struct buffer_head *__ext4_find_entry(struct inode *dir,
 		goto restart;
 	}
 	if (is_dx(dir)) {
-		ret = ext4_dx_find_entry(dir, fname, res_dir);
+		ret = ext4_dx_find_entry(dir, fname, res_dir, lblk);
 		/*
 		 * On success, or if the error was file not found,
 		 * return.  Otherwise, fall back to doing a search the
@@ -1588,7 +1588,8 @@  static struct buffer_head *__ext4_find_entry(struct inode *dir,
 static struct buffer_head *ext4_find_entry(struct inode *dir,
 					   const struct qstr *d_name,
 					   struct ext4_dir_entry_2 **res_dir,
-					   int *inlined)
+					   int *inlined,
+					   ext4_lblk_t *lblk)
 {
 	int err;
 	struct ext4_filename fname;
@@ -1600,7 +1601,7 @@  static struct buffer_head *ext4_find_entry(struct inode *dir,
 	if (err)
 		return ERR_PTR(err);
 
-	bh = __ext4_find_entry(dir, &fname, res_dir, inlined);
+	bh = __ext4_find_entry(dir, &fname, res_dir, inlined, lblk);
 
 	ext4_fname_free_filename(&fname);
 	return bh;
@@ -1620,7 +1621,7 @@  static struct buffer_head *ext4_lookup_entry(struct inode *dir,
 	if (err)
 		return ERR_PTR(err);
 
-	bh = __ext4_find_entry(dir, &fname, res_dir, NULL);
+	bh = __ext4_find_entry(dir, &fname, res_dir, NULL, NULL);
 
 	ext4_fname_free_filename(&fname);
 	return bh;
@@ -1628,7 +1629,8 @@  static struct buffer_head *ext4_lookup_entry(struct inode *dir,
 
 static struct buffer_head * ext4_dx_find_entry(struct inode *dir,
 			struct ext4_filename *fname,
-			struct ext4_dir_entry_2 **res_dir)
+			struct ext4_dir_entry_2 **res_dir,
+			ext4_lblk_t *lblk)
 {
 	struct super_block * sb = dir->i_sb;
 	struct dx_frame frames[EXT4_HTREE_LEVEL], *frame;
@@ -1675,6 +1677,8 @@  static struct buffer_head * ext4_dx_find_entry(struct inode *dir,
 errout:
 	dxtrace(printk(KERN_DEBUG "%s not found\n", fname->usr_fname->name));
 success:
+	if (lblk)
+		*lblk = block;
 	dx_release(frames);
 	return bh;
 }
@@ -1743,7 +1747,7 @@  struct dentry *ext4_get_parent(struct dentry *child)
 	struct ext4_dir_entry_2 * de;
 	struct buffer_head *bh;
 
-	bh = ext4_find_entry(d_inode(child), &dotdot, &de, NULL);
+	bh = ext4_find_entry(d_inode(child), &dotdot, &de, NULL, NULL);
 	if (IS_ERR(bh))
 		return ERR_CAST(bh);
 	if (!bh)
@@ -2495,7 +2499,7 @@  int ext4_generic_delete_entry(handle_t *handle,
 static int ext4_delete_entry(handle_t *handle,
 			     struct inode *dir,
 			     struct ext4_dir_entry_2 *de_del,
-			     struct buffer_head *bh)
+			     struct buffer_head *bh, ext4_lblk_t lblk)
 {
 	int err, csum_size = 0;
 
@@ -3091,6 +3095,7 @@  static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
 	struct buffer_head *bh;
 	struct ext4_dir_entry_2 *de;
 	handle_t *handle = NULL;
+	ext4_lblk_t lblk;
 
 	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
 		return -EIO;
@@ -3105,7 +3110,7 @@  static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
 		return retval;
 
 	retval = -ENOENT;
-	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL, &lblk);
 	if (IS_ERR(bh))
 		return PTR_ERR(bh);
 	if (!bh)
@@ -3132,7 +3137,7 @@  static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
 	if (IS_DIRSYNC(dir))
 		ext4_handle_sync(handle);
 
-	retval = ext4_delete_entry(handle, dir, de, bh);
+	retval = ext4_delete_entry(handle, dir, de, bh, lblk);
 	if (retval)
 		goto end_rmdir;
 	if (!EXT4_DIR_LINK_EMPTY(inode))
@@ -3178,6 +3183,7 @@  static int ext4_unlink(struct inode *dir, struct dentry *dentry)
 	struct buffer_head *bh;
 	struct ext4_dir_entry_2 *de;
 	handle_t *handle = NULL;
+	ext4_lblk_t lblk;
 
 	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
 		return -EIO;
@@ -3193,7 +3199,7 @@  static int ext4_unlink(struct inode *dir, struct dentry *dentry)
 		return retval;
 
 	retval = -ENOENT;
-	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL, &lblk);
 	if (IS_ERR(bh))
 		return PTR_ERR(bh);
 	if (!bh)
@@ -3216,7 +3222,7 @@  static int ext4_unlink(struct inode *dir, struct dentry *dentry)
 	if (IS_DIRSYNC(dir))
 		ext4_handle_sync(handle);
 
-	retval = ext4_delete_entry(handle, dir, de, bh);
+	retval = ext4_delete_entry(handle, dir, de, bh, lblk);
 	if (retval)
 		goto end_unlink;
 	dir->i_ctime = dir->i_mtime = current_time(dir);
@@ -3564,19 +3570,20 @@  static int ext4_find_delete_entry(handle_t *handle, struct inode *dir,
 	int retval = -ENOENT;
 	struct buffer_head *bh;
 	struct ext4_dir_entry_2 *de;
+	ext4_lblk_t lblk;
 
-	bh = ext4_find_entry(dir, d_name, &de, NULL);
+	bh = ext4_find_entry(dir, d_name, &de, NULL, &lblk);
 	if (IS_ERR(bh))
 		return PTR_ERR(bh);
 	if (bh) {
-		retval = ext4_delete_entry(handle, dir, de, bh);
+		retval = ext4_delete_entry(handle, dir, de, bh, lblk);
 		brelse(bh);
 	}
 	return retval;
 }
 
 static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
-			       int force_reread)
+			       int force_reread, ext4_lblk_t lblk)
 {
 	int retval;
 	/*
@@ -3593,7 +3600,8 @@  static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent,
 		retval = ext4_find_delete_entry(handle, ent->dir,
 						&ent->dentry->d_name);
 	} else {
-		retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh);
+		retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh,
+					   lblk);
 		if (retval == -ENOENT) {
 			retval = ext4_find_delete_entry(handle, ent->dir,
 							&ent->dentry->d_name);
@@ -3679,6 +3687,7 @@  static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct inode *whiteout = NULL;
 	int credits;
 	u8 old_file_type;
+	ext4_lblk_t lblk;
 
 	if (new.inode && new.inode->i_nlink == 0) {
 		EXT4_ERROR_INODE(new.inode,
@@ -3706,7 +3715,8 @@  static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 			return retval;
 	}
 
-	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL);
+	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL,
+				 &lblk);
 	if (IS_ERR(old.bh))
 		return PTR_ERR(old.bh);
 	/*
@@ -3720,7 +3730,7 @@  static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 		goto end_rename;
 
 	new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
-				 &new.de, &new.inlined);
+				 &new.de, &new.inlined, NULL);
 	if (IS_ERR(new.bh)) {
 		retval = PTR_ERR(new.bh);
 		new.bh = NULL;
@@ -3817,7 +3827,7 @@  static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 		/*
 		 * ok, that's it
 		 */
-		ext4_rename_delete(handle, &old, force_reread);
+		ext4_rename_delete(handle, &old, force_reread, lblk);
 	}
 
 	if (new.inode) {
@@ -3900,7 +3910,7 @@  static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 		return retval;
 
 	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name,
-				 &old.de, &old.inlined);
+				 &old.de, &old.inlined, NULL);
 	if (IS_ERR(old.bh))
 		return PTR_ERR(old.bh);
 	/*
@@ -3914,7 +3924,7 @@  static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 		goto end_rename;
 
 	new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
-				 &new.de, &new.inlined);
+				 &new.de, &new.inlined, NULL);
 	if (IS_ERR(new.bh)) {
 		retval = PTR_ERR(new.bh);
 		new.bh = NULL;