* [PATCH] fsstress: add renameat2 support @ 2019-10-11 7:56 kaixuxia 2019-10-15 2:34 ` kaixuxia 2019-10-15 14:55 ` Brian Foster 0 siblings, 2 replies; 9+ messages in thread From: kaixuxia @ 2019-10-11 7:56 UTC (permalink / raw) To: fstests; +Cc: linux-xfs, Eryu Guan, Brian Foster, newtongao, jasperwang Support the renameat2 syscall in fsstress. Signed-off-by: kaixuxia <kaixuxia@tencent.com> --- ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 11 deletions(-) diff --git a/ltp/fsstress.c b/ltp/fsstress.c index 51976f5..21529a2 100644 --- a/ltp/fsstress.c +++ b/ltp/fsstress.c @@ -44,6 +44,16 @@ io_context_t io_ctx; #define IOV_MAX 1024 #endif +#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 + #define FILELEN_MAX (32*4096) typedef enum { @@ -85,6 +95,9 @@ typedef enum { OP_READV, OP_REMOVEFATTR, OP_RENAME, + OP_RNOREPLACE, + OP_REXCHANGE, + OP_RWHITEOUT, OP_RESVSP, OP_RMDIR, OP_SETATTR, @@ -203,6 +216,9 @@ void readlink_f(int, long); 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); void setattr_f(int, long); @@ -262,6 +278,9 @@ opdesc_t ops[] = { /* remove (delete) extended attribute */ { 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 }, /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); DIR *opendir_path(pathname_t *); void process_freq(char *); int readlink_path(pathname_t *, char *, size_t); -int rename_path(pathname_t *, pathname_t *); +int rename_path(pathname_t *, pathname_t *, int); int rmdir_path(pathname_t *); void separate_pathname(pathname_t *, char *, pathname_t *); void show_ops(int, char *); @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) } int -rename_path(pathname_t *name1, pathname_t *name2) +rename_path(pathname_t *name1, pathname_t *name2, int mode) { char buf1[NAME_MAX + 1]; char buf2[NAME_MAX + 1]; @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) pathname_t newname2; int rval; - rval = rename(name1->path, name2->path); + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); if (rval >= 0 || errno != ENAMETOOLONG) return rval; separate_pathname(name1, buf1, &newname1); separate_pathname(name2, buf2, &newname2); if (strcmp(buf1, buf2) == 0) { if (chdir(buf1) == 0) { - rval = rename_path(&newname1, &newname2); + rval = rename_path(&newname1, &newname2, mode); assert(chdir("..") == 0); } } else { @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) append_pathname(&newname2, "../"); append_pathname(&newname2, name2->path); if (chdir(buf1) == 0) { - rval = rename_path(&newname1, &newname2); + rval = rename_path(&newname1, &newname2, mode); assert(chdir("..") == 0); } } else { @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) append_pathname(&newname1, "../"); append_pathname(&newname1, name1->path); if (chdir(buf2) == 0) { - rval = rename_path(&newname1, &newname2); + rval = rename_path(&newname1, &newname2, mode); assert(chdir("..") == 0); } } @@ -4215,8 +4234,18 @@ out: free_pathname(&f); } +struct print_flags renameat2_flags [] = { + { RENAME_NOREPLACE, "NOREPLACE"}, + { RENAME_EXCHANGE, "EXCHANGE"}, + { RENAME_WHITEOUT, "WHITEOUT"}, + { -1, NULL} +}; + +#define translate_renameat2_flags(mode) \ + ({translate_flags(mode, "|", renameat2_flags);}) + void -rename_f(int opno, long r) +do_renameat2(int opno, long r, int mode) { fent_t *dfep; int e; @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) int parid; int v; int v1; + int fd; /* get an existing path for the source of the rename */ init_pathname(&f); @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) free_pathname(&f); return; } - e = rename_path(&f, &newf) < 0 ? errno : 0; + /* Both pathnames must exist for the RENAME_EXCHANGE */ + if (mode == RENAME_EXCHANGE) { + fd = creat_path(&newf, 0666); + e = fd < 0 ? errno : 0; + check_cwd(); + if (fd < 0) { + if (v) + printf("%d/%d: renameat2 - creat %s failed %d\n", + procid, opno, newf.path, e); + 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; @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) add_to_flist(flp - flist, id, parid, xattr_counter); } if (v) { - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, + printf("%d/%d: rename(%s) %s to %s %d\n", procid, + opno, translate_renameat2_flags(mode), f.path, newf.path, e); if (e == 0) { - printf("%d/%d: rename del entry: id=%d,parent=%d\n", + printf("%d/%d: rename source entry: id=%d,parent=%d\n", procid, opno, fep->id, fep->parent); - printf("%d/%d: rename add entry: id=%d,parent=%d\n", + printf("%d/%d: rename target entry: id=%d,parent=%d\n", procid, opno, id, parid); } } @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) } void +rename_f(int opno, long r) +{ + do_renameat2(opno, r, 0); +} +void +rnoreplace_f(int opno, long r) +{ + do_renameat2(opno, r, RENAME_NOREPLACE); +} + +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); +} + +void resvsp_f(int opno, long r) { int e; -- 1.8.3.1 -- kaixuxia ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-11 7:56 [PATCH] fsstress: add renameat2 support kaixuxia @ 2019-10-15 2:34 ` kaixuxia 2019-10-15 10:51 ` Brian Foster 2019-10-15 15:05 ` Darrick J. Wong 2019-10-15 14:55 ` Brian Foster 1 sibling, 2 replies; 9+ messages in thread From: kaixuxia @ 2019-10-15 2:34 UTC (permalink / raw) To: fstests; +Cc: linux-xfs, Eryu Guan, Brian Foster, newtongao, jasperwang According to the comments, after adding this fsstress renameat2 support, the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced by using customized parameters(limited to rename_whiteout and creates). If this patch is okay, I will send the fsstress test patch. So, Eryu, Brian, comments? On 2019/10/11 15:56, kaixuxia wrote: > Support the renameat2 syscall in fsstress. > > Signed-off-by: kaixuxia <kaixuxia@tencent.com> > --- > ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 79 insertions(+), 11 deletions(-) > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > index 51976f5..21529a2 100644 > --- a/ltp/fsstress.c > +++ b/ltp/fsstress.c > @@ -44,6 +44,16 @@ io_context_t io_ctx; > #define IOV_MAX 1024 > #endif > > +#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 > + > #define FILELEN_MAX (32*4096) > > typedef enum { > @@ -85,6 +95,9 @@ typedef enum { > OP_READV, > OP_REMOVEFATTR, > OP_RENAME, > + OP_RNOREPLACE, > + OP_REXCHANGE, > + OP_RWHITEOUT, > OP_RESVSP, > OP_RMDIR, > OP_SETATTR, > @@ -203,6 +216,9 @@ void readlink_f(int, long); > 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); > void setattr_f(int, long); > @@ -262,6 +278,9 @@ opdesc_t ops[] = { > /* remove (delete) extended attribute */ > { 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 }, > /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ > @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); > DIR *opendir_path(pathname_t *); > void process_freq(char *); > int readlink_path(pathname_t *, char *, size_t); > -int rename_path(pathname_t *, pathname_t *); > +int rename_path(pathname_t *, pathname_t *, int); > int rmdir_path(pathname_t *); > void separate_pathname(pathname_t *, char *, pathname_t *); > void show_ops(int, char *); > @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) > } > > int > -rename_path(pathname_t *name1, pathname_t *name2) > +rename_path(pathname_t *name1, pathname_t *name2, int mode) > { > char buf1[NAME_MAX + 1]; > char buf2[NAME_MAX + 1]; > @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) > pathname_t newname2; > int rval; > > - rval = rename(name1->path, name2->path); > + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); > if (rval >= 0 || errno != ENAMETOOLONG) > return rval; > separate_pathname(name1, buf1, &newname1); > separate_pathname(name2, buf2, &newname2); > if (strcmp(buf1, buf2) == 0) { > if (chdir(buf1) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } else { > @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > append_pathname(&newname2, "../"); > append_pathname(&newname2, name2->path); > if (chdir(buf1) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } else { > @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > append_pathname(&newname1, "../"); > append_pathname(&newname1, name1->path); > if (chdir(buf2) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } > @@ -4215,8 +4234,18 @@ out: > free_pathname(&f); > } > > +struct print_flags renameat2_flags [] = { > + { RENAME_NOREPLACE, "NOREPLACE"}, > + { RENAME_EXCHANGE, "EXCHANGE"}, > + { RENAME_WHITEOUT, "WHITEOUT"}, > + { -1, NULL} > +}; > + > +#define translate_renameat2_flags(mode) \ > + ({translate_flags(mode, "|", renameat2_flags);}) > + > void > -rename_f(int opno, long r) > +do_renameat2(int opno, long r, int mode) > { > fent_t *dfep; > int e; > @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) > int parid; > int v; > int v1; > + int fd; > > /* get an existing path for the source of the rename */ > init_pathname(&f); > @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) > free_pathname(&f); > return; > } > - e = rename_path(&f, &newf) < 0 ? errno : 0; > + /* Both pathnames must exist for the RENAME_EXCHANGE */ > + if (mode == RENAME_EXCHANGE) { > + fd = creat_path(&newf, 0666); > + e = fd < 0 ? errno : 0; > + check_cwd(); > + if (fd < 0) { > + if (v) > + printf("%d/%d: renameat2 - creat %s failed %d\n", > + procid, opno, newf.path, e); > + 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; > @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) > add_to_flist(flp - flist, id, parid, xattr_counter); > } > if (v) { > - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, > + printf("%d/%d: rename(%s) %s to %s %d\n", procid, > + opno, translate_renameat2_flags(mode), f.path, > newf.path, e); > if (e == 0) { > - printf("%d/%d: rename del entry: id=%d,parent=%d\n", > + printf("%d/%d: rename source entry: id=%d,parent=%d\n", > procid, opno, fep->id, fep->parent); > - printf("%d/%d: rename add entry: id=%d,parent=%d\n", > + printf("%d/%d: rename target entry: id=%d,parent=%d\n", > procid, opno, id, parid); > } > } > @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) > } > > void > +rename_f(int opno, long r) > +{ > + do_renameat2(opno, r, 0); > +} > +void > +rnoreplace_f(int opno, long r) > +{ > + do_renameat2(opno, r, RENAME_NOREPLACE); > +} > + > +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); > +} > + > +void > resvsp_f(int opno, long r) > { > int e; > -- kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 2:34 ` kaixuxia @ 2019-10-15 10:51 ` Brian Foster 2019-10-15 11:41 ` kaixuxia 2019-10-15 15:05 ` Darrick J. Wong 1 sibling, 1 reply; 9+ messages in thread From: Brian Foster @ 2019-10-15 10:51 UTC (permalink / raw) To: kaixuxia; +Cc: fstests, linux-xfs, Eryu Guan, newtongao, jasperwang On Tue, Oct 15, 2019 at 10:34:31AM +0800, kaixuxia wrote: > According to the comments, after adding this fsstress renameat2 support, > the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced > by using customized parameters(limited to rename_whiteout and creates). If > this patch is okay, I will send the fsstress test patch. > So, Eryu, Brian, comments? > I still need to take a closer look at the patch, but are you saying you have reproduced the deadlock using fsstress with this patch? Does it reproduce reliably and within a reasonable amount of time? If so, I think it's probably a good idea to post a test along with this patch so we can test it.. Brian > On 2019/10/11 15:56, kaixuxia wrote: > > Support the renameat2 syscall in fsstress. > > > > Signed-off-by: kaixuxia <kaixuxia@tencent.com> > > --- > > ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 79 insertions(+), 11 deletions(-) > > > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > > index 51976f5..21529a2 100644 > > --- a/ltp/fsstress.c > > +++ b/ltp/fsstress.c > > @@ -44,6 +44,16 @@ io_context_t io_ctx; > > #define IOV_MAX 1024 > > #endif > > > > +#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 > > + > > #define FILELEN_MAX (32*4096) > > > > typedef enum { > > @@ -85,6 +95,9 @@ typedef enum { > > OP_READV, > > OP_REMOVEFATTR, > > OP_RENAME, > > + OP_RNOREPLACE, > > + OP_REXCHANGE, > > + OP_RWHITEOUT, > > OP_RESVSP, > > OP_RMDIR, > > OP_SETATTR, > > @@ -203,6 +216,9 @@ void readlink_f(int, long); > > 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); > > void setattr_f(int, long); > > @@ -262,6 +278,9 @@ opdesc_t ops[] = { > > /* remove (delete) extended attribute */ > > { 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 }, > > /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ > > @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); > > DIR *opendir_path(pathname_t *); > > void process_freq(char *); > > int readlink_path(pathname_t *, char *, size_t); > > -int rename_path(pathname_t *, pathname_t *); > > +int rename_path(pathname_t *, pathname_t *, int); > > int rmdir_path(pathname_t *); > > void separate_pathname(pathname_t *, char *, pathname_t *); > > void show_ops(int, char *); > > @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) > > } > > > > int > > -rename_path(pathname_t *name1, pathname_t *name2) > > +rename_path(pathname_t *name1, pathname_t *name2, int mode) > > { > > char buf1[NAME_MAX + 1]; > > char buf2[NAME_MAX + 1]; > > @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) > > pathname_t newname2; > > int rval; > > > > - rval = rename(name1->path, name2->path); > > + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); > > if (rval >= 0 || errno != ENAMETOOLONG) > > return rval; > > separate_pathname(name1, buf1, &newname1); > > separate_pathname(name2, buf2, &newname2); > > if (strcmp(buf1, buf2) == 0) { > > if (chdir(buf1) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } else { > > @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > > append_pathname(&newname2, "../"); > > append_pathname(&newname2, name2->path); > > if (chdir(buf1) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } else { > > @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > > append_pathname(&newname1, "../"); > > append_pathname(&newname1, name1->path); > > if (chdir(buf2) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } > > @@ -4215,8 +4234,18 @@ out: > > free_pathname(&f); > > } > > > > +struct print_flags renameat2_flags [] = { > > + { RENAME_NOREPLACE, "NOREPLACE"}, > > + { RENAME_EXCHANGE, "EXCHANGE"}, > > + { RENAME_WHITEOUT, "WHITEOUT"}, > > + { -1, NULL} > > +}; > > + > > +#define translate_renameat2_flags(mode) \ > > + ({translate_flags(mode, "|", renameat2_flags);}) > > + > > void > > -rename_f(int opno, long r) > > +do_renameat2(int opno, long r, int mode) > > { > > fent_t *dfep; > > int e; > > @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) > > int parid; > > int v; > > int v1; > > + int fd; > > > > /* get an existing path for the source of the rename */ > > init_pathname(&f); > > @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) > > free_pathname(&f); > > return; > > } > > - e = rename_path(&f, &newf) < 0 ? errno : 0; > > + /* Both pathnames must exist for the RENAME_EXCHANGE */ > > + if (mode == RENAME_EXCHANGE) { > > + fd = creat_path(&newf, 0666); > > + e = fd < 0 ? errno : 0; > > + check_cwd(); > > + if (fd < 0) { > > + if (v) > > + printf("%d/%d: renameat2 - creat %s failed %d\n", > > + procid, opno, newf.path, e); > > + 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; > > @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) > > add_to_flist(flp - flist, id, parid, xattr_counter); > > } > > if (v) { > > - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, > > + printf("%d/%d: rename(%s) %s to %s %d\n", procid, > > + opno, translate_renameat2_flags(mode), f.path, > > newf.path, e); > > if (e == 0) { > > - printf("%d/%d: rename del entry: id=%d,parent=%d\n", > > + printf("%d/%d: rename source entry: id=%d,parent=%d\n", > > procid, opno, fep->id, fep->parent); > > - printf("%d/%d: rename add entry: id=%d,parent=%d\n", > > + printf("%d/%d: rename target entry: id=%d,parent=%d\n", > > procid, opno, id, parid); > > } > > } > > @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) > > } > > > > void > > +rename_f(int opno, long r) > > +{ > > + do_renameat2(opno, r, 0); > > +} > > +void > > +rnoreplace_f(int opno, long r) > > +{ > > + do_renameat2(opno, r, RENAME_NOREPLACE); > > +} > > + > > +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); > > +} > > + > > +void > > resvsp_f(int opno, long r) > > { > > int e; > > > > -- > kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 10:51 ` Brian Foster @ 2019-10-15 11:41 ` kaixuxia 2019-10-15 14:57 ` Brian Foster 0 siblings, 1 reply; 9+ messages in thread From: kaixuxia @ 2019-10-15 11:41 UTC (permalink / raw) To: Brian Foster; +Cc: fstests, linux-xfs, Eryu Guan, newtongao, jasperwang On 2019/10/15 18:51, Brian Foster wrote: > On Tue, Oct 15, 2019 at 10:34:31AM +0800, kaixuxia wrote: >> According to the comments, after adding this fsstress renameat2 support, >> the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced >> by using customized parameters(limited to rename_whiteout and creates). If >> this patch is okay, I will send the fsstress test patch. >> So, Eryu, Brian, comments? >> > > I still need to take a closer look at the patch, but are you saying you > have reproduced the deadlock using fsstress with this patch? Does > it reproduce reliably and within a reasonable amount of time? If so, I > think it's probably a good idea to post a test along with this patch so > we can test it.. Actually, adding renameat2 support to fsstress is a good decision, since the reproduce possibility is very high(nears to 100%) by using fsstress with this patch, and the run time is in half a minute on my vm. The fsstress parameters as follow: $FSSTRESS_PROG -z -n 100 -p 50 -v \ -f creat=5 \ -f rename=5 \ -f rnoreplace=5 \ -f rexchange=5 \ -f rwhiteout=5 \ -d $SCRATCH_MNT/fsstress So maybe we can check this patch firstly to decide how to add renameat2 to fsstress, if the approach in this patch is okay, I will send the next test patch. If the approach is not reasonable, I need to prepare the new patch to add renameat2 support and test the reproduce possibility. > > Brian > >> On 2019/10/11 15:56, kaixuxia wrote: >>> Support the renameat2 syscall in fsstress. >>> >>> Signed-off-by: kaixuxia <kaixuxia@tencent.com> >>> --- >>> ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- >>> 1 file changed, 79 insertions(+), 11 deletions(-) >>> >>> diff --git a/ltp/fsstress.c b/ltp/fsstress.c >>> index 51976f5..21529a2 100644 >>> --- a/ltp/fsstress.c >>> +++ b/ltp/fsstress.c >>> @@ -44,6 +44,16 @@ io_context_t io_ctx; >>> #define IOV_MAX 1024 >>> #endif >>> >>> +#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 >>> + >>> #define FILELEN_MAX (32*4096) >>> >>> typedef enum { >>> @@ -85,6 +95,9 @@ typedef enum { >>> OP_READV, >>> OP_REMOVEFATTR, >>> OP_RENAME, >>> + OP_RNOREPLACE, >>> + OP_REXCHANGE, >>> + OP_RWHITEOUT, >>> OP_RESVSP, >>> OP_RMDIR, >>> OP_SETATTR, >>> @@ -203,6 +216,9 @@ void readlink_f(int, long); >>> 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); >>> void setattr_f(int, long); >>> @@ -262,6 +278,9 @@ opdesc_t ops[] = { >>> /* remove (delete) extended attribute */ >>> { 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 }, >>> /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ >>> @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); >>> DIR *opendir_path(pathname_t *); >>> void process_freq(char *); >>> int readlink_path(pathname_t *, char *, size_t); >>> -int rename_path(pathname_t *, pathname_t *); >>> +int rename_path(pathname_t *, pathname_t *, int); >>> int rmdir_path(pathname_t *); >>> void separate_pathname(pathname_t *, char *, pathname_t *); >>> void show_ops(int, char *); >>> @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) >>> } >>> >>> int >>> -rename_path(pathname_t *name1, pathname_t *name2) >>> +rename_path(pathname_t *name1, pathname_t *name2, int mode) >>> { >>> char buf1[NAME_MAX + 1]; >>> char buf2[NAME_MAX + 1]; >>> @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> pathname_t newname2; >>> int rval; >>> >>> - rval = rename(name1->path, name2->path); >>> + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); >>> if (rval >= 0 || errno != ENAMETOOLONG) >>> return rval; >>> separate_pathname(name1, buf1, &newname1); >>> separate_pathname(name2, buf2, &newname2); >>> if (strcmp(buf1, buf2) == 0) { >>> if (chdir(buf1) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } else { >>> @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> append_pathname(&newname2, "../"); >>> append_pathname(&newname2, name2->path); >>> if (chdir(buf1) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } else { >>> @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> append_pathname(&newname1, "../"); >>> append_pathname(&newname1, name1->path); >>> if (chdir(buf2) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } >>> @@ -4215,8 +4234,18 @@ out: >>> free_pathname(&f); >>> } >>> >>> +struct print_flags renameat2_flags [] = { >>> + { RENAME_NOREPLACE, "NOREPLACE"}, >>> + { RENAME_EXCHANGE, "EXCHANGE"}, >>> + { RENAME_WHITEOUT, "WHITEOUT"}, >>> + { -1, NULL} >>> +}; >>> + >>> +#define translate_renameat2_flags(mode) \ >>> + ({translate_flags(mode, "|", renameat2_flags);}) >>> + >>> void >>> -rename_f(int opno, long r) >>> +do_renameat2(int opno, long r, int mode) >>> { >>> fent_t *dfep; >>> int e; >>> @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) >>> int parid; >>> int v; >>> int v1; >>> + int fd; >>> >>> /* get an existing path for the source of the rename */ >>> init_pathname(&f); >>> @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) >>> free_pathname(&f); >>> return; >>> } >>> - e = rename_path(&f, &newf) < 0 ? errno : 0; >>> + /* Both pathnames must exist for the RENAME_EXCHANGE */ >>> + if (mode == RENAME_EXCHANGE) { >>> + fd = creat_path(&newf, 0666); >>> + e = fd < 0 ? errno : 0; >>> + check_cwd(); >>> + if (fd < 0) { >>> + if (v) >>> + printf("%d/%d: renameat2 - creat %s failed %d\n", >>> + procid, opno, newf.path, e); >>> + 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; >>> @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) >>> add_to_flist(flp - flist, id, parid, xattr_counter); >>> } >>> if (v) { >>> - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, >>> + printf("%d/%d: rename(%s) %s to %s %d\n", procid, >>> + opno, translate_renameat2_flags(mode), f.path, >>> newf.path, e); >>> if (e == 0) { >>> - printf("%d/%d: rename del entry: id=%d,parent=%d\n", >>> + printf("%d/%d: rename source entry: id=%d,parent=%d\n", >>> procid, opno, fep->id, fep->parent); >>> - printf("%d/%d: rename add entry: id=%d,parent=%d\n", >>> + printf("%d/%d: rename target entry: id=%d,parent=%d\n", >>> procid, opno, id, parid); >>> } >>> } >>> @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) >>> } >>> >>> void >>> +rename_f(int opno, long r) >>> +{ >>> + do_renameat2(opno, r, 0); >>> +} >>> +void >>> +rnoreplace_f(int opno, long r) >>> +{ >>> + do_renameat2(opno, r, RENAME_NOREPLACE); >>> +} >>> + >>> +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); >>> +} >>> + >>> +void >>> resvsp_f(int opno, long r) >>> { >>> int e; >>> >> >> -- >> kaixuxia -- kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 11:41 ` kaixuxia @ 2019-10-15 14:57 ` Brian Foster 0 siblings, 0 replies; 9+ messages in thread From: Brian Foster @ 2019-10-15 14:57 UTC (permalink / raw) To: kaixuxia; +Cc: fstests, linux-xfs, Eryu Guan, newtongao, jasperwang On Tue, Oct 15, 2019 at 07:41:11PM +0800, kaixuxia wrote: > On 2019/10/15 18:51, Brian Foster wrote: > > On Tue, Oct 15, 2019 at 10:34:31AM +0800, kaixuxia wrote: > >> According to the comments, after adding this fsstress renameat2 support, > >> the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced > >> by using customized parameters(limited to rename_whiteout and creates). If > >> this patch is okay, I will send the fsstress test patch. > >> So, Eryu, Brian, comments? > >> > > > > I still need to take a closer look at the patch, but are you saying you > > have reproduced the deadlock using fsstress with this patch? Does > > it reproduce reliably and within a reasonable amount of time? If so, I > > think it's probably a good idea to post a test along with this patch so > > we can test it.. > > Actually, adding renameat2 support to fsstress is a good decision, since the > reproduce possibility is very high(nears to 100%) by using fsstress with this > patch, and the run time is in half a minute on my vm. The fsstress parameters > as follow: > $FSSTRESS_PROG -z -n 100 -p 50 -v \ > -f creat=5 \ > -f rename=5 \ > -f rnoreplace=5 \ > -f rexchange=5 \ > -f rwhiteout=5 \ > -d $SCRATCH_MNT/fsstress > Great! > So maybe we can check this patch firstly to decide how to add renameat2 to > fsstress, if the approach in this patch is okay, I will send the next test > patch. If the approach is not reasonable, I need to prepare the new patch > to add renameat2 support and test the reproduce possibility. > I just sent a couple comments in a separate mail, but for the most part it looks reasonable to me. Brian > > > > Brian > > > >> On 2019/10/11 15:56, kaixuxia wrote: > >>> Support the renameat2 syscall in fsstress. > >>> > >>> Signed-off-by: kaixuxia <kaixuxia@tencent.com> > >>> --- > >>> ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > >>> 1 file changed, 79 insertions(+), 11 deletions(-) > >>> > >>> diff --git a/ltp/fsstress.c b/ltp/fsstress.c > >>> index 51976f5..21529a2 100644 > >>> --- a/ltp/fsstress.c > >>> +++ b/ltp/fsstress.c > >>> @@ -44,6 +44,16 @@ io_context_t io_ctx; > >>> #define IOV_MAX 1024 > >>> #endif > >>> > >>> +#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 > >>> + > >>> #define FILELEN_MAX (32*4096) > >>> > >>> typedef enum { > >>> @@ -85,6 +95,9 @@ typedef enum { > >>> OP_READV, > >>> OP_REMOVEFATTR, > >>> OP_RENAME, > >>> + OP_RNOREPLACE, > >>> + OP_REXCHANGE, > >>> + OP_RWHITEOUT, > >>> OP_RESVSP, > >>> OP_RMDIR, > >>> OP_SETATTR, > >>> @@ -203,6 +216,9 @@ void readlink_f(int, long); > >>> 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); > >>> void setattr_f(int, long); > >>> @@ -262,6 +278,9 @@ opdesc_t ops[] = { > >>> /* remove (delete) extended attribute */ > >>> { 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 }, > >>> /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ > >>> @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); > >>> DIR *opendir_path(pathname_t *); > >>> void process_freq(char *); > >>> int readlink_path(pathname_t *, char *, size_t); > >>> -int rename_path(pathname_t *, pathname_t *); > >>> +int rename_path(pathname_t *, pathname_t *, int); > >>> int rmdir_path(pathname_t *); > >>> void separate_pathname(pathname_t *, char *, pathname_t *); > >>> void show_ops(int, char *); > >>> @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) > >>> } > >>> > >>> int > >>> -rename_path(pathname_t *name1, pathname_t *name2) > >>> +rename_path(pathname_t *name1, pathname_t *name2, int mode) > >>> { > >>> char buf1[NAME_MAX + 1]; > >>> char buf2[NAME_MAX + 1]; > >>> @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) > >>> pathname_t newname2; > >>> int rval; > >>> > >>> - rval = rename(name1->path, name2->path); > >>> + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); > >>> if (rval >= 0 || errno != ENAMETOOLONG) > >>> return rval; > >>> separate_pathname(name1, buf1, &newname1); > >>> separate_pathname(name2, buf2, &newname2); > >>> if (strcmp(buf1, buf2) == 0) { > >>> if (chdir(buf1) == 0) { > >>> - rval = rename_path(&newname1, &newname2); > >>> + rval = rename_path(&newname1, &newname2, mode); > >>> assert(chdir("..") == 0); > >>> } > >>> } else { > >>> @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > >>> append_pathname(&newname2, "../"); > >>> append_pathname(&newname2, name2->path); > >>> if (chdir(buf1) == 0) { > >>> - rval = rename_path(&newname1, &newname2); > >>> + rval = rename_path(&newname1, &newname2, mode); > >>> assert(chdir("..") == 0); > >>> } > >>> } else { > >>> @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > >>> append_pathname(&newname1, "../"); > >>> append_pathname(&newname1, name1->path); > >>> if (chdir(buf2) == 0) { > >>> - rval = rename_path(&newname1, &newname2); > >>> + rval = rename_path(&newname1, &newname2, mode); > >>> assert(chdir("..") == 0); > >>> } > >>> } > >>> @@ -4215,8 +4234,18 @@ out: > >>> free_pathname(&f); > >>> } > >>> > >>> +struct print_flags renameat2_flags [] = { > >>> + { RENAME_NOREPLACE, "NOREPLACE"}, > >>> + { RENAME_EXCHANGE, "EXCHANGE"}, > >>> + { RENAME_WHITEOUT, "WHITEOUT"}, > >>> + { -1, NULL} > >>> +}; > >>> + > >>> +#define translate_renameat2_flags(mode) \ > >>> + ({translate_flags(mode, "|", renameat2_flags);}) > >>> + > >>> void > >>> -rename_f(int opno, long r) > >>> +do_renameat2(int opno, long r, int mode) > >>> { > >>> fent_t *dfep; > >>> int e; > >>> @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) > >>> int parid; > >>> int v; > >>> int v1; > >>> + int fd; > >>> > >>> /* get an existing path for the source of the rename */ > >>> init_pathname(&f); > >>> @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) > >>> free_pathname(&f); > >>> return; > >>> } > >>> - e = rename_path(&f, &newf) < 0 ? errno : 0; > >>> + /* Both pathnames must exist for the RENAME_EXCHANGE */ > >>> + if (mode == RENAME_EXCHANGE) { > >>> + fd = creat_path(&newf, 0666); > >>> + e = fd < 0 ? errno : 0; > >>> + check_cwd(); > >>> + if (fd < 0) { > >>> + if (v) > >>> + printf("%d/%d: renameat2 - creat %s failed %d\n", > >>> + procid, opno, newf.path, e); > >>> + 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; > >>> @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) > >>> add_to_flist(flp - flist, id, parid, xattr_counter); > >>> } > >>> if (v) { > >>> - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, > >>> + printf("%d/%d: rename(%s) %s to %s %d\n", procid, > >>> + opno, translate_renameat2_flags(mode), f.path, > >>> newf.path, e); > >>> if (e == 0) { > >>> - printf("%d/%d: rename del entry: id=%d,parent=%d\n", > >>> + printf("%d/%d: rename source entry: id=%d,parent=%d\n", > >>> procid, opno, fep->id, fep->parent); > >>> - printf("%d/%d: rename add entry: id=%d,parent=%d\n", > >>> + printf("%d/%d: rename target entry: id=%d,parent=%d\n", > >>> procid, opno, id, parid); > >>> } > >>> } > >>> @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) > >>> } > >>> > >>> void > >>> +rename_f(int opno, long r) > >>> +{ > >>> + do_renameat2(opno, r, 0); > >>> +} > >>> +void > >>> +rnoreplace_f(int opno, long r) > >>> +{ > >>> + do_renameat2(opno, r, RENAME_NOREPLACE); > >>> +} > >>> + > >>> +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); > >>> +} > >>> + > >>> +void > >>> resvsp_f(int opno, long r) > >>> { > >>> int e; > >>> > >> > >> -- > >> kaixuxia > > -- > kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 2:34 ` kaixuxia 2019-10-15 10:51 ` Brian Foster @ 2019-10-15 15:05 ` Darrick J. Wong 2019-10-16 3:11 ` kaixuxia 1 sibling, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2019-10-15 15:05 UTC (permalink / raw) To: kaixuxia; +Cc: fstests, linux-xfs, Eryu Guan, Brian Foster, newtongao, jasperwang On Tue, Oct 15, 2019 at 10:34:31AM +0800, kaixuxia wrote: > According to the comments, after adding this fsstress renameat2 support, > the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced > by using customized parameters(limited to rename_whiteout and creates). If > this patch is okay, I will send the fsstress test patch. /me looks forward to that, particularly because I asked weeks ago if the xfs_droplink calls in xfs_rename() could try to lock the AGI after we'd already locked the AGF for the directory expansion, but nobody sent an answer... > So, Eryu, Brian, comments? > > On 2019/10/11 15:56, kaixuxia wrote: > > Support the renameat2 syscall in fsstress. > > > > Signed-off-by: kaixuxia <kaixuxia@tencent.com> > > --- > > ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 79 insertions(+), 11 deletions(-) > > > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > > index 51976f5..21529a2 100644 > > --- a/ltp/fsstress.c > > +++ b/ltp/fsstress.c > > @@ -44,6 +44,16 @@ io_context_t io_ctx; > > #define IOV_MAX 1024 > > #endif > > > > +#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 > > + > > #define FILELEN_MAX (32*4096) > > > > typedef enum { > > @@ -85,6 +95,9 @@ typedef enum { > > OP_READV, > > OP_REMOVEFATTR, > > OP_RENAME, > > + OP_RNOREPLACE, > > + OP_REXCHANGE, > > + OP_RWHITEOUT, > > OP_RESVSP, > > OP_RMDIR, > > OP_SETATTR, > > @@ -203,6 +216,9 @@ void readlink_f(int, long); > > 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); > > void setattr_f(int, long); > > @@ -262,6 +278,9 @@ opdesc_t ops[] = { > > /* remove (delete) extended attribute */ > > { 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 }, > > /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ > > @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); > > DIR *opendir_path(pathname_t *); > > void process_freq(char *); > > int readlink_path(pathname_t *, char *, size_t); > > -int rename_path(pathname_t *, pathname_t *); > > +int rename_path(pathname_t *, pathname_t *, int); > > int rmdir_path(pathname_t *); > > void separate_pathname(pathname_t *, char *, pathname_t *); > > void show_ops(int, char *); > > @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) > > } > > > > int > > -rename_path(pathname_t *name1, pathname_t *name2) > > +rename_path(pathname_t *name1, pathname_t *name2, int mode) > > { > > char buf1[NAME_MAX + 1]; > > char buf2[NAME_MAX + 1]; > > @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) > > pathname_t newname2; > > int rval; > > > > - rval = rename(name1->path, name2->path); > > + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); For the rename(..., 0) case, would we be crippling fsstress if the kernel doesn't know about renameat2 and doesn't fall back to renameat() or regular rename()? I guess renameat2 showed up in 3.15 which was quite a long time ago except in RHEL land. :) --D > > if (rval >= 0 || errno != ENAMETOOLONG) > > return rval; > > separate_pathname(name1, buf1, &newname1); > > separate_pathname(name2, buf2, &newname2); > > if (strcmp(buf1, buf2) == 0) { > > if (chdir(buf1) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } else { > > @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > > append_pathname(&newname2, "../"); > > append_pathname(&newname2, name2->path); > > if (chdir(buf1) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } else { > > @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > > append_pathname(&newname1, "../"); > > append_pathname(&newname1, name1->path); > > if (chdir(buf2) == 0) { > > - rval = rename_path(&newname1, &newname2); > > + rval = rename_path(&newname1, &newname2, mode); > > assert(chdir("..") == 0); > > } > > } > > @@ -4215,8 +4234,18 @@ out: > > free_pathname(&f); > > } > > > > +struct print_flags renameat2_flags [] = { > > + { RENAME_NOREPLACE, "NOREPLACE"}, > > + { RENAME_EXCHANGE, "EXCHANGE"}, > > + { RENAME_WHITEOUT, "WHITEOUT"}, > > + { -1, NULL} > > +}; > > + > > +#define translate_renameat2_flags(mode) \ > > + ({translate_flags(mode, "|", renameat2_flags);}) > > + > > void > > -rename_f(int opno, long r) > > +do_renameat2(int opno, long r, int mode) > > { > > fent_t *dfep; > > int e; > > @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) > > int parid; > > int v; > > int v1; > > + int fd; > > > > /* get an existing path for the source of the rename */ > > init_pathname(&f); > > @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) > > free_pathname(&f); > > return; > > } > > - e = rename_path(&f, &newf) < 0 ? errno : 0; > > + /* Both pathnames must exist for the RENAME_EXCHANGE */ > > + if (mode == RENAME_EXCHANGE) { > > + fd = creat_path(&newf, 0666); > > + e = fd < 0 ? errno : 0; > > + check_cwd(); > > + if (fd < 0) { > > + if (v) > > + printf("%d/%d: renameat2 - creat %s failed %d\n", > > + procid, opno, newf.path, e); > > + 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; > > @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) > > add_to_flist(flp - flist, id, parid, xattr_counter); > > } > > if (v) { > > - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, > > + printf("%d/%d: rename(%s) %s to %s %d\n", procid, > > + opno, translate_renameat2_flags(mode), f.path, > > newf.path, e); > > if (e == 0) { > > - printf("%d/%d: rename del entry: id=%d,parent=%d\n", > > + printf("%d/%d: rename source entry: id=%d,parent=%d\n", > > procid, opno, fep->id, fep->parent); > > - printf("%d/%d: rename add entry: id=%d,parent=%d\n", > > + printf("%d/%d: rename target entry: id=%d,parent=%d\n", > > procid, opno, id, parid); > > } > > } > > @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) > > } > > > > void > > +rename_f(int opno, long r) > > +{ > > + do_renameat2(opno, r, 0); > > +} > > +void > > +rnoreplace_f(int opno, long r) > > +{ > > + do_renameat2(opno, r, RENAME_NOREPLACE); > > +} > > + > > +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); > > +} > > + > > +void > > resvsp_f(int opno, long r) > > { > > int e; > > > > -- > kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 15:05 ` Darrick J. Wong @ 2019-10-16 3:11 ` kaixuxia 0 siblings, 0 replies; 9+ messages in thread From: kaixuxia @ 2019-10-16 3:11 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, linux-xfs, Eryu Guan, Brian Foster, newtongao, jasperwang On 2019/10/15 23:05, Darrick J. Wong wrote: > On Tue, Oct 15, 2019 at 10:34:31AM +0800, kaixuxia wrote: >> According to the comments, after adding this fsstress renameat2 support, >> the deadlock between the AGI and AGF with RENAME_WHITEOUT can be reproduced >> by using customized parameters(limited to rename_whiteout and creates). If >> this patch is okay, I will send the fsstress test patch. > > /me looks forward to that, particularly because I asked weeks ago if the > xfs_droplink calls in xfs_rename() could try to lock the AGI after we'd > already locked the AGF for the directory expansion, but nobody sent an > answer... Yeah, It's been a long time since we talked this xfs_droplink() deadlock problem. I already have the simple fix patch, now I'm trying to get a xfstests test patch to reproduce this deadlock problem with high possibility. I will send the fix patch ASAP. > >> So, Eryu, Brian, comments? >> >> On 2019/10/11 15:56, kaixuxia wrote: >>> Support the renameat2 syscall in fsstress. >>> >>> Signed-off-by: kaixuxia <kaixuxia@tencent.com> >>> --- >>> ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- >>> 1 file changed, 79 insertions(+), 11 deletions(-) >>> >>> diff --git a/ltp/fsstress.c b/ltp/fsstress.c >>> index 51976f5..21529a2 100644 >>> --- a/ltp/fsstress.c >>> +++ b/ltp/fsstress.c >>> @@ -44,6 +44,16 @@ io_context_t io_ctx; >>> #define IOV_MAX 1024 >>> #endif >>> >>> +#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 >>> + >>> #define FILELEN_MAX (32*4096) >>> >>> typedef enum { >>> @@ -85,6 +95,9 @@ typedef enum { >>> OP_READV, >>> OP_REMOVEFATTR, >>> OP_RENAME, >>> + OP_RNOREPLACE, >>> + OP_REXCHANGE, >>> + OP_RWHITEOUT, >>> OP_RESVSP, >>> OP_RMDIR, >>> OP_SETATTR, >>> @@ -203,6 +216,9 @@ void readlink_f(int, long); >>> 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); >>> void setattr_f(int, long); >>> @@ -262,6 +278,9 @@ opdesc_t ops[] = { >>> /* remove (delete) extended attribute */ >>> { 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 }, >>> /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ >>> @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); >>> DIR *opendir_path(pathname_t *); >>> void process_freq(char *); >>> int readlink_path(pathname_t *, char *, size_t); >>> -int rename_path(pathname_t *, pathname_t *); >>> +int rename_path(pathname_t *, pathname_t *, int); >>> int rmdir_path(pathname_t *); >>> void separate_pathname(pathname_t *, char *, pathname_t *); >>> void show_ops(int, char *); >>> @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) >>> } >>> >>> int >>> -rename_path(pathname_t *name1, pathname_t *name2) >>> +rename_path(pathname_t *name1, pathname_t *name2, int mode) >>> { >>> char buf1[NAME_MAX + 1]; >>> char buf2[NAME_MAX + 1]; >>> @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> pathname_t newname2; >>> int rval; >>> >>> - rval = rename(name1->path, name2->path); >>> + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); > > For the rename(..., 0) case, would we be crippling fsstress if the > kernel doesn't know about renameat2 and doesn't fall back to renameat() > or regular rename()? I guess renameat2 showed up in 3.15 which was > quite a long time ago except in RHEL land. :) > Yeah, will preserve the rename() call when it is not available in V2. > --D > >>> if (rval >= 0 || errno != ENAMETOOLONG) >>> return rval; >>> separate_pathname(name1, buf1, &newname1); >>> separate_pathname(name2, buf2, &newname2); >>> if (strcmp(buf1, buf2) == 0) { >>> if (chdir(buf1) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } else { >>> @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> append_pathname(&newname2, "../"); >>> append_pathname(&newname2, name2->path); >>> if (chdir(buf1) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } else { >>> @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >>> append_pathname(&newname1, "../"); >>> append_pathname(&newname1, name1->path); >>> if (chdir(buf2) == 0) { >>> - rval = rename_path(&newname1, &newname2); >>> + rval = rename_path(&newname1, &newname2, mode); >>> assert(chdir("..") == 0); >>> } >>> } >>> @@ -4215,8 +4234,18 @@ out: >>> free_pathname(&f); >>> } >>> >>> +struct print_flags renameat2_flags [] = { >>> + { RENAME_NOREPLACE, "NOREPLACE"}, >>> + { RENAME_EXCHANGE, "EXCHANGE"}, >>> + { RENAME_WHITEOUT, "WHITEOUT"}, >>> + { -1, NULL} >>> +}; >>> + >>> +#define translate_renameat2_flags(mode) \ >>> + ({translate_flags(mode, "|", renameat2_flags);}) >>> + >>> void >>> -rename_f(int opno, long r) >>> +do_renameat2(int opno, long r, int mode) >>> { >>> fent_t *dfep; >>> int e; >>> @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) >>> int parid; >>> int v; >>> int v1; >>> + int fd; >>> >>> /* get an existing path for the source of the rename */ >>> init_pathname(&f); >>> @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) >>> free_pathname(&f); >>> return; >>> } >>> - e = rename_path(&f, &newf) < 0 ? errno : 0; >>> + /* Both pathnames must exist for the RENAME_EXCHANGE */ >>> + if (mode == RENAME_EXCHANGE) { >>> + fd = creat_path(&newf, 0666); >>> + e = fd < 0 ? errno : 0; >>> + check_cwd(); >>> + if (fd < 0) { >>> + if (v) >>> + printf("%d/%d: renameat2 - creat %s failed %d\n", >>> + procid, opno, newf.path, e); >>> + 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; >>> @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) >>> add_to_flist(flp - flist, id, parid, xattr_counter); >>> } >>> if (v) { >>> - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, >>> + printf("%d/%d: rename(%s) %s to %s %d\n", procid, >>> + opno, translate_renameat2_flags(mode), f.path, >>> newf.path, e); >>> if (e == 0) { >>> - printf("%d/%d: rename del entry: id=%d,parent=%d\n", >>> + printf("%d/%d: rename source entry: id=%d,parent=%d\n", >>> procid, opno, fep->id, fep->parent); >>> - printf("%d/%d: rename add entry: id=%d,parent=%d\n", >>> + printf("%d/%d: rename target entry: id=%d,parent=%d\n", >>> procid, opno, id, parid); >>> } >>> } >>> @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) >>> } >>> >>> void >>> +rename_f(int opno, long r) >>> +{ >>> + do_renameat2(opno, r, 0); >>> +} >>> +void >>> +rnoreplace_f(int opno, long r) >>> +{ >>> + do_renameat2(opno, r, RENAME_NOREPLACE); >>> +} >>> + >>> +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); >>> +} >>> + >>> +void >>> resvsp_f(int opno, long r) >>> { >>> int e; >>> >> >> -- >> kaixuxia -- kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-11 7:56 [PATCH] fsstress: add renameat2 support kaixuxia 2019-10-15 2:34 ` kaixuxia @ 2019-10-15 14:55 ` Brian Foster 2019-10-16 2:47 ` kaixuxia 1 sibling, 1 reply; 9+ messages in thread From: Brian Foster @ 2019-10-15 14:55 UTC (permalink / raw) To: kaixuxia; +Cc: fstests, linux-xfs, Eryu Guan, newtongao, jasperwang On Fri, Oct 11, 2019 at 03:56:01PM +0800, kaixuxia wrote: > Support the renameat2 syscall in fsstress. > > Signed-off-by: kaixuxia <kaixuxia@tencent.com> > --- > ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 79 insertions(+), 11 deletions(-) > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > index 51976f5..21529a2 100644 > --- a/ltp/fsstress.c > +++ b/ltp/fsstress.c > @@ -44,6 +44,16 @@ io_context_t io_ctx; > #define IOV_MAX 1024 > #endif > > +#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 > + > #define FILELEN_MAX (32*4096) > > typedef enum { > @@ -85,6 +95,9 @@ typedef enum { > OP_READV, > OP_REMOVEFATTR, > OP_RENAME, > + OP_RNOREPLACE, > + OP_REXCHANGE, > + OP_RWHITEOUT, > OP_RESVSP, > OP_RMDIR, > OP_SETATTR, > @@ -203,6 +216,9 @@ void readlink_f(int, long); > 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); > void setattr_f(int, long); > @@ -262,6 +278,9 @@ opdesc_t ops[] = { > /* remove (delete) extended attribute */ > { 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 }, > /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ > @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); > DIR *opendir_path(pathname_t *); > void process_freq(char *); > int readlink_path(pathname_t *, char *, size_t); > -int rename_path(pathname_t *, pathname_t *); > +int rename_path(pathname_t *, pathname_t *, int); > int rmdir_path(pathname_t *); > void separate_pathname(pathname_t *, char *, pathname_t *); > void show_ops(int, char *); > @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) > } > > int > -rename_path(pathname_t *name1, pathname_t *name2) > +rename_path(pathname_t *name1, pathname_t *name2, int mode) > { > char buf1[NAME_MAX + 1]; > char buf2[NAME_MAX + 1]; > @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) > pathname_t newname2; > int rval; > > - rval = rename(name1->path, name2->path); > + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); Perhaps we should preserve the rename() call with an ifdef or something (similar to src/renameat2.c) in the event that renameat2 is not available. In fact, I suppose we'd want to compile out the new renameat2 based ops as well so the user doesn't have to disable them where they aren't supported. > if (rval >= 0 || errno != ENAMETOOLONG) > return rval; > separate_pathname(name1, buf1, &newname1); > separate_pathname(name2, buf2, &newname2); > if (strcmp(buf1, buf2) == 0) { > if (chdir(buf1) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } else { > @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > append_pathname(&newname2, "../"); > append_pathname(&newname2, name2->path); > if (chdir(buf1) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } else { > @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) > append_pathname(&newname1, "../"); > append_pathname(&newname1, name1->path); > if (chdir(buf2) == 0) { > - rval = rename_path(&newname1, &newname2); > + rval = rename_path(&newname1, &newname2, mode); > assert(chdir("..") == 0); > } > } > @@ -4215,8 +4234,18 @@ out: > free_pathname(&f); > } > > +struct print_flags renameat2_flags [] = { > + { RENAME_NOREPLACE, "NOREPLACE"}, > + { RENAME_EXCHANGE, "EXCHANGE"}, > + { RENAME_WHITEOUT, "WHITEOUT"}, > + { -1, NULL} > +}; > + > +#define translate_renameat2_flags(mode) \ > + ({translate_flags(mode, "|", renameat2_flags);}) > + > void > -rename_f(int opno, long r) > +do_renameat2(int opno, long r, int mode) > { > fent_t *dfep; > int e; > @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) > int parid; > int v; > int v1; > + int fd; > > /* get an existing path for the source of the rename */ > init_pathname(&f); > @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) > free_pathname(&f); > return; > } > - e = rename_path(&f, &newf) < 0 ? errno : 0; > + /* Both pathnames must exist for the RENAME_EXCHANGE */ > + if (mode == RENAME_EXCHANGE) { > + fd = creat_path(&newf, 0666); > + e = fd < 0 ? errno : 0; > + check_cwd(); > + if (fd < 0) { > + if (v) > + printf("%d/%d: renameat2 - creat %s failed %d\n", > + procid, opno, newf.path, e); > + free_pathname(&newf); > + free_pathname(&f); > + return; > + } > + } > + e = rename_path(&f, &newf, mode) < 0 ? errno : 0; It looks like the existing code looks up a source entry and a target directory and generates a new name to serve as the rename target entry. Here we create the entry so we can perform an exchange. Can we check mode earlier and instead look up another existing entry if this is an exchange, else run the existing code to generate a name? Other than those couple things this looks pretty good to me. Brian > check_cwd(); > if (e == 0) { > int xattr_counter = fep->xattr_counter; > @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) > add_to_flist(flp - flist, id, parid, xattr_counter); > } > if (v) { > - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, > + printf("%d/%d: rename(%s) %s to %s %d\n", procid, > + opno, translate_renameat2_flags(mode), f.path, > newf.path, e); > if (e == 0) { > - printf("%d/%d: rename del entry: id=%d,parent=%d\n", > + printf("%d/%d: rename source entry: id=%d,parent=%d\n", > procid, opno, fep->id, fep->parent); > - printf("%d/%d: rename add entry: id=%d,parent=%d\n", > + printf("%d/%d: rename target entry: id=%d,parent=%d\n", > procid, opno, id, parid); > } > } > @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) > } > > void > +rename_f(int opno, long r) > +{ > + do_renameat2(opno, r, 0); > +} > +void > +rnoreplace_f(int opno, long r) > +{ > + do_renameat2(opno, r, RENAME_NOREPLACE); > +} > + > +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); > +} > + > +void > resvsp_f(int opno, long r) > { > int e; > -- > 1.8.3.1 > > -- > kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsstress: add renameat2 support 2019-10-15 14:55 ` Brian Foster @ 2019-10-16 2:47 ` kaixuxia 0 siblings, 0 replies; 9+ messages in thread From: kaixuxia @ 2019-10-16 2:47 UTC (permalink / raw) To: Brian Foster; +Cc: fstests, linux-xfs, Eryu Guan, newtongao, jasperwang On 2019/10/15 22:55, Brian Foster wrote: > On Fri, Oct 11, 2019 at 03:56:01PM +0800, kaixuxia wrote: >> Support the renameat2 syscall in fsstress. >> >> Signed-off-by: kaixuxia <kaixuxia@tencent.com> >> --- >> ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 79 insertions(+), 11 deletions(-) >> >> diff --git a/ltp/fsstress.c b/ltp/fsstress.c >> index 51976f5..21529a2 100644 >> --- a/ltp/fsstress.c >> +++ b/ltp/fsstress.c >> @@ -44,6 +44,16 @@ io_context_t io_ctx; >> #define IOV_MAX 1024 >> #endif >> >> +#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 >> + >> #define FILELEN_MAX (32*4096) >> >> typedef enum { >> @@ -85,6 +95,9 @@ typedef enum { >> OP_READV, >> OP_REMOVEFATTR, >> OP_RENAME, >> + OP_RNOREPLACE, >> + OP_REXCHANGE, >> + OP_RWHITEOUT, >> OP_RESVSP, >> OP_RMDIR, >> OP_SETATTR, >> @@ -203,6 +216,9 @@ void readlink_f(int, long); >> 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); >> void setattr_f(int, long); >> @@ -262,6 +278,9 @@ opdesc_t ops[] = { >> /* remove (delete) extended attribute */ >> { 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 }, >> /* set attribute flag (FS_IOC_SETFLAGS ioctl) */ >> @@ -354,7 +373,7 @@ int open_path(pathname_t *, int); >> DIR *opendir_path(pathname_t *); >> void process_freq(char *); >> int readlink_path(pathname_t *, char *, size_t); >> -int rename_path(pathname_t *, pathname_t *); >> +int rename_path(pathname_t *, pathname_t *, int); >> int rmdir_path(pathname_t *); >> void separate_pathname(pathname_t *, char *, pathname_t *); >> void show_ops(int, char *); >> @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz) >> } >> >> int >> -rename_path(pathname_t *name1, pathname_t *name2) >> +rename_path(pathname_t *name1, pathname_t *name2, int mode) >> { >> char buf1[NAME_MAX + 1]; >> char buf2[NAME_MAX + 1]; >> @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2) >> pathname_t newname2; >> int rval; >> >> - rval = rename(name1->path, name2->path); >> + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode); > > Perhaps we should preserve the rename() call with an ifdef or something > (similar to src/renameat2.c) in the event that renameat2 is not > available. In fact, I suppose we'd want to compile out the new renameat2 > based ops as well so the user doesn't have to disable them where they > aren't supported. Right, will fix it in V2. > >> if (rval >= 0 || errno != ENAMETOOLONG) >> return rval; >> separate_pathname(name1, buf1, &newname1); >> separate_pathname(name2, buf2, &newname2); >> if (strcmp(buf1, buf2) == 0) { >> if (chdir(buf1) == 0) { >> - rval = rename_path(&newname1, &newname2); >> + rval = rename_path(&newname1, &newname2, mode); >> assert(chdir("..") == 0); >> } >> } else { >> @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >> append_pathname(&newname2, "../"); >> append_pathname(&newname2, name2->path); >> if (chdir(buf1) == 0) { >> - rval = rename_path(&newname1, &newname2); >> + rval = rename_path(&newname1, &newname2, mode); >> assert(chdir("..") == 0); >> } >> } else { >> @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2) >> append_pathname(&newname1, "../"); >> append_pathname(&newname1, name1->path); >> if (chdir(buf2) == 0) { >> - rval = rename_path(&newname1, &newname2); >> + rval = rename_path(&newname1, &newname2, mode); >> assert(chdir("..") == 0); >> } >> } >> @@ -4215,8 +4234,18 @@ out: >> free_pathname(&f); >> } >> >> +struct print_flags renameat2_flags [] = { >> + { RENAME_NOREPLACE, "NOREPLACE"}, >> + { RENAME_EXCHANGE, "EXCHANGE"}, >> + { RENAME_WHITEOUT, "WHITEOUT"}, >> + { -1, NULL} >> +}; >> + >> +#define translate_renameat2_flags(mode) \ >> + ({translate_flags(mode, "|", renameat2_flags);}) >> + >> void >> -rename_f(int opno, long r) >> +do_renameat2(int opno, long r, int mode) >> { >> fent_t *dfep; >> int e; >> @@ -4229,6 +4258,7 @@ rename_f(int opno, long r) >> int parid; >> int v; >> int v1; >> + int fd; >> >> /* get an existing path for the source of the rename */ >> init_pathname(&f); >> @@ -4260,7 +4290,21 @@ rename_f(int opno, long r) >> free_pathname(&f); >> return; >> } >> - e = rename_path(&f, &newf) < 0 ? errno : 0; >> + /* Both pathnames must exist for the RENAME_EXCHANGE */ >> + if (mode == RENAME_EXCHANGE) { >> + fd = creat_path(&newf, 0666); >> + e = fd < 0 ? errno : 0; >> + check_cwd(); >> + if (fd < 0) { >> + if (v) >> + printf("%d/%d: renameat2 - creat %s failed %d\n", >> + procid, opno, newf.path, e); >> + free_pathname(&newf); >> + free_pathname(&f); >> + return; >> + } >> + } >> + e = rename_path(&f, &newf, mode) < 0 ? errno : 0; > > It looks like the existing code looks up a source entry and a target > directory and generates a new name to serve as the rename target entry. > Here we create the entry so we can perform an exchange. Can we check > mode earlier and instead look up another existing entry if this is an > exchange, else run the existing code to generate a name? Yeah, it is more reasonable, will fix it in V2. > > Other than those couple things this looks pretty good to me. > > Brian > > >> check_cwd(); >> if (e == 0) { >> int xattr_counter = fep->xattr_counter; >> @@ -4273,12 +4317,13 @@ rename_f(int opno, long r) >> add_to_flist(flp - flist, id, parid, xattr_counter); >> } >> if (v) { >> - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path, >> + printf("%d/%d: rename(%s) %s to %s %d\n", procid, >> + opno, translate_renameat2_flags(mode), f.path, >> newf.path, e); >> if (e == 0) { >> - printf("%d/%d: rename del entry: id=%d,parent=%d\n", >> + printf("%d/%d: rename source entry: id=%d,parent=%d\n", >> procid, opno, fep->id, fep->parent); >> - printf("%d/%d: rename add entry: id=%d,parent=%d\n", >> + printf("%d/%d: rename target entry: id=%d,parent=%d\n", >> procid, opno, id, parid); >> } >> } >> @@ -4287,6 +4332,29 @@ rename_f(int opno, long r) >> } >> >> void >> +rename_f(int opno, long r) >> +{ >> + do_renameat2(opno, r, 0); >> +} >> +void >> +rnoreplace_f(int opno, long r) >> +{ >> + do_renameat2(opno, r, RENAME_NOREPLACE); >> +} >> + >> +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); >> +} >> + >> +void >> resvsp_f(int opno, long r) >> { >> int e; >> -- >> 1.8.3.1 >> >> -- >> kaixuxia -- kaixuxia ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-10-16 3:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-11 7:56 [PATCH] fsstress: add renameat2 support kaixuxia 2019-10-15 2:34 ` kaixuxia 2019-10-15 10:51 ` Brian Foster 2019-10-15 11:41 ` kaixuxia 2019-10-15 14:57 ` Brian Foster 2019-10-15 15:05 ` Darrick J. Wong 2019-10-16 3:11 ` kaixuxia 2019-10-15 14:55 ` Brian Foster 2019-10-16 2:47 ` kaixuxia
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).