| Message ID | 20220116144301.399581-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFS: simplify check for freeing cn_resp | expand |
Hi Tom, On Sun, 2022-01-16 at 06:43 -0800, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > nfs42_files_from_same_server() is called to check if freeing > cn_resp is required. Instead of calling a function, check > the pointer. > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > fs/nfs/nfs4file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c > index e79ae4cbc395e..677631ea4cfb3 100644 > --- a/fs/nfs/nfs4file.c > +++ b/fs/nfs/nfs4file.c > @@ -180,7 +180,7 @@ static ssize_t __nfs4_copy_file_range(struct file > *file_in, loff_t pos_in, > ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, > count, > nss, cnrs, sync); > out: > - if (!nfs42_files_from_same_server(file_in, file_out)) > + if (cn_resp) > kfree(cn_resp); The kernel convention in these circumstances is to just skip the NULL pointer check, since kfree() does that anyway. > if (ret == -EAGAIN) > goto retry;
On 1/16/22 7:47 AM, Trond Myklebust wrote: > Hi Tom, > > On Sun, 2022-01-16 at 06:43 -0800, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> nfs42_files_from_same_server() is called to check if freeing >> cn_resp is required. Instead of calling a function, check >> the pointer. >> >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> fs/nfs/nfs4file.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c >> index e79ae4cbc395e..677631ea4cfb3 100644 >> --- a/fs/nfs/nfs4file.c >> +++ b/fs/nfs/nfs4file.c >> @@ -180,7 +180,7 @@ static ssize_t __nfs4_copy_file_range(struct file >> *file_in, loff_t pos_in, >> ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, >> count, >> nss, cnrs, sync); >> out: >> - if (!nfs42_files_from_same_server(file_in, file_out)) >> + if (cn_resp) >> kfree(cn_resp); > The kernel convention in these circumstances is to just skip the NULL > pointer check, since kfree() does that anyway. Yes. I'll respin. Tom > >> if (ret == -EAGAIN) >> goto retry;
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index e79ae4cbc395e..677631ea4cfb3 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -180,7 +180,7 @@ static ssize_t __nfs4_copy_file_range(struct file *file_in, loff_t pos_in, ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count, nss, cnrs, sync); out: - if (!nfs42_files_from_same_server(file_in, file_out)) + if (cn_resp) kfree(cn_resp); if (ret == -EAGAIN) goto retry;