Use a local var to collect flags in kiocb_set_rw_flags(). That spares
some memory writes and allows to replace most of the jumps with MOVEcc.
Signed-off-by: Pavel Begunkov <[email protected]>
---
v2: fast exit on flags == 0 (Matthew Wilcox)
include/linux/fs.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8a00ba99284e..b7f1f1b7d691 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3282,22 +3282,28 @@ static inline int iocb_flags(struct file *file)
static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags)
{
+ int kiocb_flags = 0;
+
+ if (!flags)
+ return 0;
if (unlikely(flags & ~RWF_SUPPORTED))
return -EOPNOTSUPP;
if (flags & RWF_NOWAIT) {
if (!(ki->ki_filp->f_mode & FMODE_NOWAIT))
return -EOPNOTSUPP;
- ki->ki_flags |= IOCB_NOWAIT;
+ kiocb_flags |= IOCB_NOWAIT;
}
if (flags & RWF_HIPRI)
- ki->ki_flags |= IOCB_HIPRI;
+ kiocb_flags |= IOCB_HIPRI;
if (flags & RWF_DSYNC)
- ki->ki_flags |= IOCB_DSYNC;
+ kiocb_flags |= IOCB_DSYNC;
if (flags & RWF_SYNC)
- ki->ki_flags |= (IOCB_DSYNC | IOCB_SYNC);
+ kiocb_flags |= (IOCB_DSYNC | IOCB_SYNC);
if (flags & RWF_APPEND)
- ki->ki_flags |= IOCB_APPEND;
+ kiocb_flags |= IOCB_APPEND;
+
+ ki->ki_flags |= kiocb_flags;
return 0;
}
--
2.24.0
On 01/08/2020 13:36, Pavel Begunkov wrote:
> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
> some memory writes and allows to replace most of the jumps with MOVEcc.
This one is pretty old, but looks like nobody in fsdevel cares.
Jens, any chance you can pick this up? For instancec, I don't like the
binary it generates for io_uring.
>
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
>
> v2: fast exit on flags == 0 (Matthew Wilcox)
>
> include/linux/fs.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8a00ba99284e..b7f1f1b7d691 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3282,22 +3282,28 @@ static inline int iocb_flags(struct file *file)
>
> static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags)
> {
> + int kiocb_flags = 0;
> +
> + if (!flags)
> + return 0;
> if (unlikely(flags & ~RWF_SUPPORTED))
> return -EOPNOTSUPP;
>
> if (flags & RWF_NOWAIT) {
> if (!(ki->ki_filp->f_mode & FMODE_NOWAIT))
> return -EOPNOTSUPP;
> - ki->ki_flags |= IOCB_NOWAIT;
> + kiocb_flags |= IOCB_NOWAIT;
> }
> if (flags & RWF_HIPRI)
> - ki->ki_flags |= IOCB_HIPRI;
> + kiocb_flags |= IOCB_HIPRI;
> if (flags & RWF_DSYNC)
> - ki->ki_flags |= IOCB_DSYNC;
> + kiocb_flags |= IOCB_DSYNC;
> if (flags & RWF_SYNC)
> - ki->ki_flags |= (IOCB_DSYNC | IOCB_SYNC);
> + kiocb_flags |= (IOCB_DSYNC | IOCB_SYNC);
> if (flags & RWF_APPEND)
> - ki->ki_flags |= IOCB_APPEND;
> + kiocb_flags |= IOCB_APPEND;
> +
> + ki->ki_flags |= kiocb_flags;
> return 0;
> }
>
>
--
Pavel Begunkov
On Sat, Aug 01, 2020 at 01:36:33PM +0300, Pavel Begunkov wrote:
> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
> some memory writes and allows to replace most of the jumps with MOVEcc.
>
> Signed-off-by: Pavel Begunkov <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
If you want to improve the codegen here further, I would suggest that
renumbering the IOCB flags to match the RWF flags would lead to better
codegen (can't do it the other way around; RWF flags are userspace ABI,
IOCB flags are not). iocb_flags() probably doesn't get any worse because
the IOCB_ flags don't have the same numbers as the O_ bits (which differ
by arch anyway).
On 8/1/20 9:37 AM, Matthew Wilcox wrote:
> On Sat, Aug 01, 2020 at 01:36:33PM +0300, Pavel Begunkov wrote:
>> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
>> some memory writes and allows to replace most of the jumps with MOVEcc.
>>
>> Signed-off-by: Pavel Begunkov <[email protected]>
>
> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
>
> If you want to improve the codegen here further, I would suggest that
> renumbering the IOCB flags to match the RWF flags would lead to better
> codegen (can't do it the other way around; RWF flags are userspace ABI,
> IOCB flags are not). iocb_flags() probably doesn't get any worse because
> the IOCB_ flags don't have the same numbers as the O_ bits (which differ
> by arch anyway).
Yeah that's not a bad idea, would kill a lot of branches.
--
Jens Axboe
On 8/1/20 4:36 AM, Pavel Begunkov wrote:
> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
> some memory writes and allows to replace most of the jumps with MOVEcc.
I've picked this one up.
--
Jens Axboe
On 01/08/2020 20:02, Jens Axboe wrote:
> On 8/1/20 4:36 AM, Pavel Begunkov wrote:
>> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
>> some memory writes and allows to replace most of the jumps with MOVEcc.
>
> I've picked this one up.
Thanks
--
Pavel Begunkov
On 01/08/2020 20:01, Jens Axboe wrote:
> On 8/1/20 9:37 AM, Matthew Wilcox wrote:
>> On Sat, Aug 01, 2020 at 01:36:33PM +0300, Pavel Begunkov wrote:
>>> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
>>> some memory writes and allows to replace most of the jumps with MOVEcc.
>>>
>>> Signed-off-by: Pavel Begunkov <[email protected]>
>>
>> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
>>
>> If you want to improve the codegen here further, I would suggest that
>> renumbering the IOCB flags to match the RWF flags would lead to better
>> codegen (can't do it the other way around; RWF flags are userspace ABI,
>> IOCB flags are not). iocb_flags() probably doesn't get any worse because
>> the IOCB_ flags don't have the same numbers as the O_ bits (which differ
>> by arch anyway).
>
> Yeah that's not a bad idea, would kill a lot of branches.
Is that common here to do so? I've done this for io_uring flags a while
ago, but left RWF alone at the time, reluctant to check for possible
complications (e.g. bit magic).
--
Pavel Begunkov
On 01/08/2020 18:37, Matthew Wilcox wrote:
> On Sat, Aug 01, 2020 at 01:36:33PM +0300, Pavel Begunkov wrote:
>> Use a local var to collect flags in kiocb_set_rw_flags(). That spares
>> some memory writes and allows to replace most of the jumps with MOVEcc.
>>
>> Signed-off-by: Pavel Begunkov <[email protected]>
>
> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
Thanks for reviewing it
>
> If you want to improve the codegen here further, I would suggest that
> renumbering the IOCB flags to match the RWF flags would lead to better
> codegen (can't do it the other way around; RWF flags are userspace ABI,
> IOCB flags are not). iocb_flags() probably doesn't get any worse because
> the IOCB_ flags don't have the same numbers as the O_ bits (which differ
> by arch anyway).
>
--
Pavel Begunkov