To replace open coded iter->count. This makes code cleaner.
Signed-off-by: Baoquan He <[email protected]>
---
fs/proc/vmcore.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 4cbb8db7c507..ed58a7edc821 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -319,21 +319,21 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
u64 start;
struct vmcore *m = NULL;
- if (iter->count == 0 || *fpos >= vmcore_size)
+ if (!iov_iter_count(iter) || *fpos >= vmcore_size)
return 0;
iov_iter_truncate(iter, vmcore_size - *fpos);
/* Read ELF core header */
if (*fpos < elfcorebuf_sz) {
- tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count);
+ tsz = min(elfcorebuf_sz - (size_t)*fpos, iov_iter_count(iter));
if (copy_to_iter(elfcorebuf + *fpos, tsz, iter) < tsz)
return -EFAULT;
*fpos += tsz;
acc += tsz;
/* leave now if filled buffer already */
- if (iter->count == 0)
+ if (!iov_iter_count(iter))
return acc;
}
@@ -354,7 +354,7 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
/* Read device dumps */
if (*fpos < elfcorebuf_sz + vmcoredd_orig_sz) {
tsz = min(elfcorebuf_sz + vmcoredd_orig_sz -
- (size_t)*fpos, iter->count);
+ (size_t)*fpos, iov_iter_count(iter));
start = *fpos - elfcorebuf_sz;
if (vmcoredd_copy_dumps(iter, start, tsz))
return -EFAULT;
@@ -363,13 +363,14 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
acc += tsz;
/* leave now if filled buffer already */
- if (!iter->count)
+ if (!iov_iter_count(iter))
return acc;
}
#endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
/* Read remaining elf notes */
- tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count);
+ tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos,
+ iov_iter_count(iter));
kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz;
if (copy_to_iter(kaddr, tsz, iter) < tsz)
return -EFAULT;
@@ -378,7 +379,7 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
acc += tsz;
/* leave now if filled buffer already */
- if (iter->count == 0)
+ if (!iov_iter_count(iter))
return acc;
}
@@ -386,7 +387,7 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
if (*fpos < m->offset + m->size) {
tsz = (size_t)min_t(unsigned long long,
m->offset + m->size - *fpos,
- iter->count);
+ iov_iter_count(iter));
start = m->paddr + *fpos - m->offset;
tmp = read_from_oldmem(iter, tsz, &start,
cc_platform_has(CC_ATTR_MEM_ENCRYPT));
@@ -396,7 +397,7 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
acc += tsz;
/* leave now if filled buffer already */
- if (iter->count == 0)
+ if (!iov_iter_count(iter))
return acc;
}
}
--
2.34.1
On Fri, Mar 18, 2022 at 01:48:07PM +0000, David Laight wrote:
> For some definition of 'cleaner' :-)
>
> iter->count is clearly a simple, cheap structure member lookup.
> OTOH iov_iter_count(iter) might be an expensive traversal of
> the vector (or worse).
>
> So a quick read of the code by someone who isn't an expert
> in the iov functions leaves them wondering what is going on
> or having to spend time locating the definition ...
Thank you for having an opinion. Al's opinion, as the iov_iter
maintainer, trumps yours.
From: Baoquan He
> Sent: 18 March 2022 09:37
>
> To replace open coded iter->count. This makes code cleaner.
...
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 4cbb8db7c507..ed58a7edc821 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -319,21 +319,21 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
> u64 start;
> struct vmcore *m = NULL;
>
> - if (iter->count == 0 || *fpos >= vmcore_size)
> + if (!iov_iter_count(iter) || *fpos >= vmcore_size)
For some definition of 'cleaner' :-)
iter->count is clearly a simple, cheap structure member lookup.
OTOH iov_iter_count(iter) might be an expensive traversal of
the vector (or worse).
So a quick read of the code by someone who isn't an expert
in the iov functions leaves them wondering what is going on
or having to spend time locating the definition ...
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Hi David,
On 03/18/22 at 01:48pm, David Laight wrote:
> From: Baoquan He
> > Sent: 18 March 2022 09:37
> >
> > To replace open coded iter->count. This makes code cleaner.
> ...
> > diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> > index 4cbb8db7c507..ed58a7edc821 100644
> > --- a/fs/proc/vmcore.c
> > +++ b/fs/proc/vmcore.c
> > @@ -319,21 +319,21 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
> > u64 start;
> > struct vmcore *m = NULL;
> >
> > - if (iter->count == 0 || *fpos >= vmcore_size)
> > + if (!iov_iter_count(iter) || *fpos >= vmcore_size)
>
> For some definition of 'cleaner' :-)
>
> iter->count is clearly a simple, cheap structure member lookup.
> OTOH iov_iter_count(iter) might be an expensive traversal of
> the vector (or worse).
>
> So a quick read of the code by someone who isn't an expert
> in the iov functions leaves them wondering what is going on
> or having to spend time locating the definition ...
Thanks for reviewing and looking into this.
People may have the same feeling as you when looking at codes at the
first glance. While usually we all use editor to explore codes, so.
Basically, I noticed putting open code into wrapper is a tendency, see a
lot of patches to clean up open code in sub component. About the extra
cost of wrapper, I believe it does have. It should be one of reasons
in some places open code is necessary. However, in fs/proc/vmcore, I
don't have the worry since it's very tiny and can be ignorable.
Thanks
Baoquan