diff mbox series

[3/4] fsstress: add EXCHANGE renameat2 support

Message ID 71744e89979dfd25f1bffc44c70f6df214a5477b.1571926791.git.kaixuxia@tencent.com (mailing list archive)
State New, archived
Headers show
Series xfstests: add deadlock between the AGI and AGF with RENAME_WHITEOUT test | expand

Commit Message

Kaixu Xia Oct. 24, 2019, 2:20 p.m. UTC 
Support the EXCHANGE renameat2 syscall in fsstress.

Signed-off-by: kaixuxia <kaixuxia@tencent.com>
---
 ltp/fsstress.c | 86 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 64 insertions(+), 22 deletions(-)

Comments

Brian Foster Oct. 25, 2019, 4:01 p.m. UTC | #1 
On Thu, Oct 24, 2019 at 10:20:50PM +0800, kaixuxia wrote:
> Support the EXCHANGE renameat2 syscall in fsstress.
> 
> Signed-off-by: kaixuxia <kaixuxia@tencent.com>
> ---
>  ltp/fsstress.c | 86 +++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 64 insertions(+), 22 deletions(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 5059639..958adf9 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
...
> @@ -1118,7 +1124,7 @@ fent_to_name(pathname_t *name, flist_t *flp, fent_t *fep)
>  }
>  
>  void
> -fix_parent(int oldid, int newid)
> +fix_parent(int oldid, int newid, int swap)
>  {
>  	fent_t	*fep;
>  	flist_t	*flp;
> @@ -1129,6 +1135,8 @@ fix_parent(int oldid, int newid)
>  		for (j = 0, fep = flp->fents; j < flp->nfiles; j++, fep++) {
>  			if (fep->parent == oldid)
>  				fep->parent = newid;
> +			if (swap && fep->parent == newid)
> +				fep->parent = oldid;

We might as well use a bool for swap.

>  		}
>  	}
>  }
> @@ -4256,6 +4264,7 @@ out:
>  
>  struct print_flags renameat2_flags [] = {
>  	{ RENAME_NOREPLACE, "NOREPLACE"},
> +	{ RENAME_EXCHANGE, "EXCHANGE"},
>  	{ RENAME_WHITEOUT, "WHITEOUT"},
>  	{ -1, NULL}
>  };
> @@ -4291,41 +4300,68 @@ do_renameat2(int opno, long r, int mode)
>  		return;
>  	}
>  
> -	/* get an existing directory for the destination parent directory name */
> -	if (!get_fname(FT_DIRm, random(), NULL, NULL, &dfep, &v))
> -		parid = -1;
> -	else
> -		parid = dfep->id;
> -	v |= v1;
> +	/* Both pathnames must exist for the RENAME_EXCHANGE */

This comment should also say that the types must be the same.

> +	if (mode == RENAME_EXCHANGE) {
> +		which = 1 << (flp - flist);
> +		init_pathname(&newf);
> +		if (!get_fname(which, random(), &newf, NULL, &dfep, &v)) {
> +			if (v)
> +				printf("%d/%d: rename - no target filename\n",
> +					procid, opno);
> +			free_pathname(&newf);
> +			free_pathname(&f);
> +			return;
> +		}
> +		v |= v1;
> +		id = dfep->id;
> +		parid = dfep->parent;
> +	} else {
> +		/*
> +		 * get an existing directory for the destination parent
> +		 * directory name.
> +		 */
> +		if (!get_fname(FT_DIRm, random(), NULL, NULL, &dfep, &v))
> +			parid = -1;
> +		else
> +			parid = dfep->id;
> +		v |= v1;
>  
> -	/* generate a new path using an existing parent directory in name */
> -	init_pathname(&newf);
> -	e = generate_fname(dfep, flp - flist, &newf, &id, &v1);
> -	v |= v1;
> -	if (!e) {
> -		if (v) {
> -			(void)fent_to_name(&f, &flist[FT_DIR], dfep);
> -			printf("%d/%d: rename - no filename from %s\n",
> -				procid, opno, f.path);
> +		/*
> +		 * generate a new path using an existing parent directory
> +		 * in name.
> +		 */
> +		init_pathname(&newf);
> +		e = generate_fname(dfep, flp - flist, &newf, &id, &v1);
> +		v |= v1;
> +		if (!e) {
> +			if (v) {
> +				(void)fent_to_name(&f, &flist[FT_DIR], dfep);
> +				printf("%d/%d: rename - no filename from %s\n",
> +					procid, opno, f.path);
> +			}
> +			free_pathname(&newf);
> +			free_pathname(&f);
> +			return;
>  		}
> -		free_pathname(&newf);
> -		free_pathname(&f);
> -		return;
>  	}
>  	e = rename_path(&f, &newf, mode) < 0 ? errno : 0;
>  	check_cwd();
>  	if (e == 0) {
>  		int xattr_counter = fep->xattr_counter;
> +		int swap = (mode == RENAME_EXCHANGE) ? 1 : 0;
>  
>  		oldid = fep->id;
>  		oldparid = fep->parent;
>  
>  		if (flp - flist == FT_DIR)
> -			fix_parent(oldid, id);
> +			fix_parent(oldid, id, swap);

What about the other directory if we exchanged two dirs (also see my
comment on the previous version around the safety of doing two separate
swaps)? BTW however this turns out, it would also be useful to see some
comments in this area of code to explain it along with some content in
the commit log descriptions of both patches to document the limitations
of the various renameat2 modes.

Brian

>  
>  		if (mode == RENAME_WHITEOUT)
>  			add_to_flist(flp - flist, id, parid, xattr_counter);
> -		else {
> +		else if (mode == RENAME_EXCHANGE) {
> +			fep->xattr_counter = dfep->xattr_counter;
> +			dfep->xattr_counter = xattr_counter;
> +		} else {
>  			del_from_flist(flp - flist, fep - flp->fents);
>  			add_to_flist(flp - flist, id, parid, xattr_counter);
>  		}
> @@ -4358,6 +4394,12 @@ rnoreplace_f(int opno, long r)
>  }
>  
>  void
> +rexchange_f(int opno, long r)
> +{
> +	do_renameat2(opno, r, RENAME_EXCHANGE);
> +}
> +
> +void
>  rwhiteout_f(int opno, long r)
>  {
>  	do_renameat2(opno, r, RENAME_WHITEOUT);
> -- 
> 1.8.3.1
>
diff mbox series

Patch

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 5059639..958adf9 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -69,6 +69,9 @@  static int renameat2(int dfd1, const char *path1,
 #ifndef RENAME_NOREPLACE
 #define RENAME_NOREPLACE	(1 << 0)	/* Don't overwrite target */
 #endif
+#ifndef RENAME_EXCHANGE
+#define RENAME_EXCHANGE		(1 << 1)	/* Exchange source and dest */
+#endif
 #ifndef RENAME_WHITEOUT
 #define RENAME_WHITEOUT		(1 << 2)	/* Whiteout source */
 #endif
@@ -115,6 +118,7 @@  typedef enum {
 	OP_REMOVEFATTR,
 	OP_RENAME,
 	OP_RNOREPLACE,
+	OP_REXCHANGE,
 	OP_RWHITEOUT,
 	OP_RESVSP,
 	OP_RMDIR,
@@ -235,6 +239,7 @@  void	readv_f(int, long);
 void	removefattr_f(int, long);
 void	rename_f(int, long);
 void	rnoreplace_f(int, long);
+void	rexchange_f(int, long);
 void	rwhiteout_f(int, long);
 void	resvsp_f(int, long);
 void	rmdir_f(int, long);
@@ -296,6 +301,7 @@  opdesc_t	ops[] = {
 	{ OP_REMOVEFATTR, "removefattr", removefattr_f, 1, 1 },
 	{ OP_RENAME, "rename", rename_f, 2, 1 },
 	{ OP_RNOREPLACE, "rnoreplace", rnoreplace_f, 2, 1 },
+	{ OP_REXCHANGE, "rexchange", rexchange_f, 2, 1 },
 	{ OP_RWHITEOUT, "rwhiteout", rwhiteout_f, 2, 1 },
 	{ OP_RESVSP, "resvsp", resvsp_f, 1, 1 },
 	{ OP_RMDIR, "rmdir", rmdir_f, 1, 1 },
@@ -371,7 +377,7 @@  void	del_from_flist(int, int);
 int	dirid_to_name(char *, int);
 void	doproc(void);
 int	fent_to_name(pathname_t *, flist_t *, fent_t *);
-void	fix_parent(int, int);
+void	fix_parent(int, int, int);
 void	free_pathname(pathname_t *);
 int	generate_fname(fent_t *, int, pathname_t *, int *, int *);
 int	generate_xattr_name(int, char *, int);
@@ -1118,7 +1124,7 @@  fent_to_name(pathname_t *name, flist_t *flp, fent_t *fep)
 }
 
 void
-fix_parent(int oldid, int newid)
+fix_parent(int oldid, int newid, int swap)
 {
 	fent_t	*fep;
 	flist_t	*flp;
@@ -1129,6 +1135,8 @@  fix_parent(int oldid, int newid)
 		for (j = 0, fep = flp->fents; j < flp->nfiles; j++, fep++) {
 			if (fep->parent == oldid)
 				fep->parent = newid;
+			if (swap && fep->parent == newid)
+				fep->parent = oldid;
 		}
 	}
 }
@@ -4256,6 +4264,7 @@  out:
 
 struct print_flags renameat2_flags [] = {
 	{ RENAME_NOREPLACE, "NOREPLACE"},
+	{ RENAME_EXCHANGE, "EXCHANGE"},
 	{ RENAME_WHITEOUT, "WHITEOUT"},
 	{ -1, NULL}
 };
@@ -4291,41 +4300,68 @@  do_renameat2(int opno, long r, int mode)
 		return;
 	}
 
-	/* get an existing directory for the destination parent directory name */
-	if (!get_fname(FT_DIRm, random(), NULL, NULL, &dfep, &v))
-		parid = -1;
-	else
-		parid = dfep->id;
-	v |= v1;
+	/* Both pathnames must exist for the RENAME_EXCHANGE */
+	if (mode == RENAME_EXCHANGE) {
+		which = 1 << (flp - flist);
+		init_pathname(&newf);
+		if (!get_fname(which, random(), &newf, NULL, &dfep, &v)) {
+			if (v)
+				printf("%d/%d: rename - no target filename\n",
+					procid, opno);
+			free_pathname(&newf);
+			free_pathname(&f);
+			return;
+		}
+		v |= v1;
+		id = dfep->id;
+		parid = dfep->parent;
+	} else {
+		/*
+		 * get an existing directory for the destination parent
+		 * directory name.
+		 */
+		if (!get_fname(FT_DIRm, random(), NULL, NULL, &dfep, &v))
+			parid = -1;
+		else
+			parid = dfep->id;
+		v |= v1;
 
-	/* generate a new path using an existing parent directory in name */
-	init_pathname(&newf);
-	e = generate_fname(dfep, flp - flist, &newf, &id, &v1);
-	v |= v1;
-	if (!e) {
-		if (v) {
-			(void)fent_to_name(&f, &flist[FT_DIR], dfep);
-			printf("%d/%d: rename - no filename from %s\n",
-				procid, opno, f.path);
+		/*
+		 * generate a new path using an existing parent directory
+		 * in name.
+		 */
+		init_pathname(&newf);
+		e = generate_fname(dfep, flp - flist, &newf, &id, &v1);
+		v |= v1;
+		if (!e) {
+			if (v) {
+				(void)fent_to_name(&f, &flist[FT_DIR], dfep);
+				printf("%d/%d: rename - no filename from %s\n",
+					procid, opno, f.path);
+			}
+			free_pathname(&newf);
+			free_pathname(&f);
+			return;
 		}
-		free_pathname(&newf);
-		free_pathname(&f);
-		return;
 	}
 	e = rename_path(&f, &newf, mode) < 0 ? errno : 0;
 	check_cwd();
 	if (e == 0) {
 		int xattr_counter = fep->xattr_counter;
+		int swap = (mode == RENAME_EXCHANGE) ? 1 : 0;
 
 		oldid = fep->id;
 		oldparid = fep->parent;
 
 		if (flp - flist == FT_DIR)
-			fix_parent(oldid, id);
+			fix_parent(oldid, id, swap);
 
 		if (mode == RENAME_WHITEOUT)
 			add_to_flist(flp - flist, id, parid, xattr_counter);
-		else {
+		else if (mode == RENAME_EXCHANGE) {
+			fep->xattr_counter = dfep->xattr_counter;
+			dfep->xattr_counter = xattr_counter;
+		} else {
 			del_from_flist(flp - flist, fep - flp->fents);
 			add_to_flist(flp - flist, id, parid, xattr_counter);
 		}
@@ -4358,6 +4394,12 @@  rnoreplace_f(int opno, long r)
 }
 
 void
+rexchange_f(int opno, long r)
+{
+	do_renameat2(opno, r, RENAME_EXCHANGE);
+}
+
+void
 rwhiteout_f(int opno, long r)
 {
 	do_renameat2(opno, r, RENAME_WHITEOUT);