> On Sep 22, 2022, at 12:31 PM, ChenXiaoSong <chenxiaosong2@xxxxxxxxxx> wrote: > > Use DEFINE_SHOW_ATTRIBUTE helper macro to simplify the code. > > inode is converted from seq_file->file instead of seq_file->private in > client_info_show(). Generally speaking, the patch description should explain why a change is being made. We can already see that @inode is converted by looking at the diff below, so the above comment doesn't add value. Same remark applies to 4/5. What might work better: "The underlying inode needed by client_info_show() is already available via struct seq_file. Thus it is safe to replace client_info_open() with the open function implicitly defined by DEFINE_SHOW_ATTRIBUTE()." I'll apply these as-is to nfsd for-next to get them into the testing pipeline. Thanks! > Signed-off-by: ChenXiaoSong <chenxiaosong2@xxxxxxxxxx> > --- > fs/nfsd/nfs4state.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index c5d199d7e6b4..4017721a04c7 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -2478,7 +2478,7 @@ static const char *cb_state2str(int state) > > static int client_info_show(struct seq_file *m, void *v) > { > - struct inode *inode = m->private; > + struct inode *inode = file_inode(m->file); > struct nfs4_client *clp; > u64 clid; > > @@ -2518,17 +2518,7 @@ static int client_info_show(struct seq_file *m, void *v) > return 0; > } > > -static int client_info_open(struct inode *inode, struct file *file) > -{ > - return single_open(file, client_info_show, inode); > -} > - > -static const struct file_operations client_info_fops = { > - .open = client_info_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = single_release, > -}; > +DEFINE_SHOW_ATTRIBUTE(client_info); > > static void *states_start(struct seq_file *s, loff_t *pos) > __acquires(&clp->cl_lock) > -- > 2.31.1 > -- Chuck Lever