| Message ID | 20231220053238.GT1674809@ZenIV (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PATCH 01/22] hostfs: use d_splice_alias() calling conventions to simplify failure exits | expand |
Howdy Al... I applied your orangefs patch to 6.7.0-rc6 and found one xfstests failure that was not there when I ran against xfstests-6.7.0-rc5. (generic/438) I'm about to hit the road for a several day motorcycle ride in an hour or so, I just wanted to give feedback before Ieft. I'll look into it further when I get back. -Mike On Wed, Dec 20, 2023 at 12:33 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > orangefs_dir_fill() doesn't use oi and dentry arguments at all > do_readdir() gets dentry, uses only dentry->d_inode; it also > gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode - > constant offset). > orangefs_dir_mode() gets dentry and oi, uses only to pass those > to do_readdir(). > orangefs_dir_iterate() uses dentry and oi only to pass those to > orangefs_dir_fill() and orangefs_dir_more(). > > The only thing it really needs is ->d_inode; moreover, that's > better expressed as file_inode(file) - no need to go through > ->f_path.dentry->d_inode. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/orangefs/dir.c | 32 ++++++++++++-------------------- > 1 file changed, 12 insertions(+), 20 deletions(-) > > diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c > index 9cacce5d55c1..6d1fbeca9d81 100644 > --- a/fs/orangefs/dir.c > +++ b/fs/orangefs/dir.c > @@ -58,10 +58,10 @@ struct orangefs_dir { > * first part of the part list. > */ > > -static int do_readdir(struct orangefs_inode_s *oi, > - struct orangefs_dir *od, struct dentry *dentry, > +static int do_readdir(struct orangefs_dir *od, struct inode *inode, > struct orangefs_kernel_op_s *op) > { > + struct orangefs_inode_s *oi = ORANGEFS_I(inode); > struct orangefs_readdir_response_s *resp; > int bufi, r; > > @@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi, > op->upcall.req.readdir.buf_index = bufi; > > r = service_operation(op, "orangefs_readdir", > - get_interruptible_flag(dentry->d_inode)); > + get_interruptible_flag(inode)); > > orangefs_readdir_index_put(bufi); > > @@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od, > return 0; > } > > -static int orangefs_dir_more(struct orangefs_inode_s *oi, > - struct orangefs_dir *od, struct dentry *dentry) > +static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode) > { > struct orangefs_kernel_op_s *op; > int r; > @@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi, > od->error = -ENOMEM; > return -ENOMEM; > } > - r = do_readdir(oi, od, dentry, op); > + r = do_readdir(od, inode, op); > if (r) { > od->error = r; > goto out; > @@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part, > return 1; > } > > -static int orangefs_dir_fill(struct orangefs_inode_s *oi, > - struct orangefs_dir *od, struct dentry *dentry, > - struct dir_context *ctx) > +static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx) > { > struct orangefs_dir_part *part; > size_t count; > @@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset, > static int orangefs_dir_iterate(struct file *file, > struct dir_context *ctx) > { > - struct orangefs_inode_s *oi; > - struct orangefs_dir *od; > - struct dentry *dentry; > + struct orangefs_dir *od = file->private_data; > + struct inode *inode = file_inode(file); > int r; > > - dentry = file->f_path.dentry; > - oi = ORANGEFS_I(dentry->d_inode); > - od = file->private_data; > - > if (od->error) > return od->error; > > @@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file, > */ > while (od->token != ORANGEFS_ITERATE_END && > ctx->pos > od->end) { > - r = orangefs_dir_more(oi, od, dentry); > + r = orangefs_dir_more(od, inode); > if (r) > return r; > } > @@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file, > > /* Then try to fill if there's any left in the buffer. */ > if (ctx->pos < od->end) { > - r = orangefs_dir_fill(oi, od, dentry, ctx); > + r = orangefs_dir_fill(od, ctx); > if (r) > return r; > } > > /* Finally get some more and try to fill. */ > if (od->token != ORANGEFS_ITERATE_END) { > - r = orangefs_dir_more(oi, od, dentry); > + r = orangefs_dir_more(od, inode); > if (r) > return r; > - r = orangefs_dir_fill(oi, od, dentry, ctx); > + r = orangefs_dir_fill(od, ctx); > } > > return r; > -- > 2.39.2 > >
Hi Al... without going into detail about me being clumsy, there is no new regression. There is a problem I found in orangefs and have sent it up to Walt Ligon. I had then excluded that test from my xfstests runs and lost that particular exclusion when I recently pulled/updated my xfstests. So... I have tested your patch and I think it is good. Some parts of the motorcycle ride were chilly... https://hubcapsc.com/misc/madison_georgia_motel_frost.jpg -Mike On Wed, Dec 27, 2023 at 7:05 AM Mike Marshall <hubcap@omnibond.com> wrote: > > Howdy Al... I applied your orangefs patch to 6.7.0-rc6 and found one > xfstests failure that was not there when I ran against > xfstests-6.7.0-rc5. (generic/438) > > I'm about to hit the road for a several day motorcycle ride in an hour > or so, I just wanted to give feedback before Ieft. I'll look into it > further when I get back. > > -Mike > > On Wed, Dec 20, 2023 at 12:33 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > orangefs_dir_fill() doesn't use oi and dentry arguments at all > > do_readdir() gets dentry, uses only dentry->d_inode; it also > > gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode - > > constant offset). > > orangefs_dir_mode() gets dentry and oi, uses only to pass those > > to do_readdir(). > > orangefs_dir_iterate() uses dentry and oi only to pass those to > > orangefs_dir_fill() and orangefs_dir_more(). > > > > The only thing it really needs is ->d_inode; moreover, that's > > better expressed as file_inode(file) - no need to go through > > ->f_path.dentry->d_inode. > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > --- > > fs/orangefs/dir.c | 32 ++++++++++++-------------------- > > 1 file changed, 12 insertions(+), 20 deletions(-) > > > > diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c > > index 9cacce5d55c1..6d1fbeca9d81 100644 > > --- a/fs/orangefs/dir.c > > +++ b/fs/orangefs/dir.c > > @@ -58,10 +58,10 @@ struct orangefs_dir { > > * first part of the part list. > > */ > > > > -static int do_readdir(struct orangefs_inode_s *oi, > > - struct orangefs_dir *od, struct dentry *dentry, > > +static int do_readdir(struct orangefs_dir *od, struct inode *inode, > > struct orangefs_kernel_op_s *op) > > { > > + struct orangefs_inode_s *oi = ORANGEFS_I(inode); > > struct orangefs_readdir_response_s *resp; > > int bufi, r; > > > > @@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi, > > op->upcall.req.readdir.buf_index = bufi; > > > > r = service_operation(op, "orangefs_readdir", > > - get_interruptible_flag(dentry->d_inode)); > > + get_interruptible_flag(inode)); > > > > orangefs_readdir_index_put(bufi); > > > > @@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od, > > return 0; > > } > > > > -static int orangefs_dir_more(struct orangefs_inode_s *oi, > > - struct orangefs_dir *od, struct dentry *dentry) > > +static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode) > > { > > struct orangefs_kernel_op_s *op; > > int r; > > @@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi, > > od->error = -ENOMEM; > > return -ENOMEM; > > } > > - r = do_readdir(oi, od, dentry, op); > > + r = do_readdir(od, inode, op); > > if (r) { > > od->error = r; > > goto out; > > @@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part, > > return 1; > > } > > > > -static int orangefs_dir_fill(struct orangefs_inode_s *oi, > > - struct orangefs_dir *od, struct dentry *dentry, > > - struct dir_context *ctx) > > +static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx) > > { > > struct orangefs_dir_part *part; > > size_t count; > > @@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset, > > static int orangefs_dir_iterate(struct file *file, > > struct dir_context *ctx) > > { > > - struct orangefs_inode_s *oi; > > - struct orangefs_dir *od; > > - struct dentry *dentry; > > + struct orangefs_dir *od = file->private_data; > > + struct inode *inode = file_inode(file); > > int r; > > > > - dentry = file->f_path.dentry; > > - oi = ORANGEFS_I(dentry->d_inode); > > - od = file->private_data; > > - > > if (od->error) > > return od->error; > > > > @@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file, > > */ > > while (od->token != ORANGEFS_ITERATE_END && > > ctx->pos > od->end) { > > - r = orangefs_dir_more(oi, od, dentry); > > + r = orangefs_dir_more(od, inode); > > if (r) > > return r; > > } > > @@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file, > > > > /* Then try to fill if there's any left in the buffer. */ > > if (ctx->pos < od->end) { > > - r = orangefs_dir_fill(oi, od, dentry, ctx); > > + r = orangefs_dir_fill(od, ctx); > > if (r) > > return r; > > } > > > > /* Finally get some more and try to fill. */ > > if (od->token != ORANGEFS_ITERATE_END) { > > - r = orangefs_dir_more(oi, od, dentry); > > + r = orangefs_dir_more(od, inode); > > if (r) > > return r; > > - r = orangefs_dir_fill(oi, od, dentry, ctx); > > + r = orangefs_dir_fill(od, ctx); > > } > > > > return r; > > -- > > 2.39.2 > > > >
diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c index 9cacce5d55c1..6d1fbeca9d81 100644 --- a/fs/orangefs/dir.c +++ b/fs/orangefs/dir.c @@ -58,10 +58,10 @@ struct orangefs_dir { * first part of the part list. */ -static int do_readdir(struct orangefs_inode_s *oi, - struct orangefs_dir *od, struct dentry *dentry, +static int do_readdir(struct orangefs_dir *od, struct inode *inode, struct orangefs_kernel_op_s *op) { + struct orangefs_inode_s *oi = ORANGEFS_I(inode); struct orangefs_readdir_response_s *resp; int bufi, r; @@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi, op->upcall.req.readdir.buf_index = bufi; r = service_operation(op, "orangefs_readdir", - get_interruptible_flag(dentry->d_inode)); + get_interruptible_flag(inode)); orangefs_readdir_index_put(bufi); @@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od, return 0; } -static int orangefs_dir_more(struct orangefs_inode_s *oi, - struct orangefs_dir *od, struct dentry *dentry) +static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode) { struct orangefs_kernel_op_s *op; int r; @@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi, od->error = -ENOMEM; return -ENOMEM; } - r = do_readdir(oi, od, dentry, op); + r = do_readdir(od, inode, op); if (r) { od->error = r; goto out; @@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part, return 1; } -static int orangefs_dir_fill(struct orangefs_inode_s *oi, - struct orangefs_dir *od, struct dentry *dentry, - struct dir_context *ctx) +static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx) { struct orangefs_dir_part *part; size_t count; @@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset, static int orangefs_dir_iterate(struct file *file, struct dir_context *ctx) { - struct orangefs_inode_s *oi; - struct orangefs_dir *od; - struct dentry *dentry; + struct orangefs_dir *od = file->private_data; + struct inode *inode = file_inode(file); int r; - dentry = file->f_path.dentry; - oi = ORANGEFS_I(dentry->d_inode); - od = file->private_data; - if (od->error) return od->error; @@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file, */ while (od->token != ORANGEFS_ITERATE_END && ctx->pos > od->end) { - r = orangefs_dir_more(oi, od, dentry); + r = orangefs_dir_more(od, inode); if (r) return r; } @@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file, /* Then try to fill if there's any left in the buffer. */ if (ctx->pos < od->end) { - r = orangefs_dir_fill(oi, od, dentry, ctx); + r = orangefs_dir_fill(od, ctx); if (r) return r; } /* Finally get some more and try to fill. */ if (od->token != ORANGEFS_ITERATE_END) { - r = orangefs_dir_more(oi, od, dentry); + r = orangefs_dir_more(od, inode); if (r) return r; - r = orangefs_dir_fill(oi, od, dentry, ctx); + r = orangefs_dir_fill(od, ctx); } return r;
orangefs_dir_fill() doesn't use oi and dentry arguments at all do_readdir() gets dentry, uses only dentry->d_inode; it also gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode - constant offset). orangefs_dir_mode() gets dentry and oi, uses only to pass those to do_readdir(). orangefs_dir_iterate() uses dentry and oi only to pass those to orangefs_dir_fill() and orangefs_dir_more(). The only thing it really needs is ->d_inode; moreover, that's better expressed as file_inode(file) - no need to go through ->f_path.dentry->d_inode. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/orangefs/dir.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-)