| Message ID | 7d493d4872b75fc59556a63ee62c43b30c661ff9.1579223790.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: optimise kiocb_set_rw_flags() | expand |
On Fri, Jan 17, 2020 at 04:16:41AM +0300, Pavel Begunkov wrote: > kiocb_set_rw_flags() generates a poor code with several memory writes > and a lot of jumps. Help compilers to optimise it. > > Tested with gcc 9.2 on x64-86, and as a result, it its output now is a > plain code without jumps accumulating in a register before a memory > write. Nice! > static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags) > { > + int kiocb_flags = 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; > + > + if (kiocb_flags) > + ki->ki_flags |= kiocb_flags; > return 0; > } Might it generate even better code to do ... int kiocb_flags = 0; + if (!flags) + return 0; if (unlikely(flags & ~RWF_SUPPORTED)) return -EOPNOTSUPP; ... - if (kiocb_flags) - ki->ki_flags |= kiocb_flags; + ki->ki_flags |= kiocb_flags;
On 17/01/2020 04:21, Matthew Wilcox wrote: > On Fri, Jan 17, 2020 at 04:16:41AM +0300, Pavel Begunkov wrote: >> kiocb_set_rw_flags() generates a poor code with several memory writes >> and a lot of jumps. Help compilers to optimise it. >> >> Tested with gcc 9.2 on x64-86, and as a result, it its output now is a >> plain code without jumps accumulating in a register before a memory >> write. > > Nice! > >> static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags) >> { >> + int kiocb_flags = 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; >> + >> + if (kiocb_flags) >> + ki->ki_flags |= kiocb_flags; >> return 0; >> } > > Might it generate even better code to do ... Good idea, thanks! I'll resend > > int kiocb_flags = 0; > > + if (!flags) > + return 0; > if (unlikely(flags & ~RWF_SUPPORTED)) > return -EOPNOTSUPP; > > ... > > - if (kiocb_flags) > - ki->ki_flags |= kiocb_flags; > + ki->ki_flags |= kiocb_flags; >
diff --git a/include/linux/fs.h b/include/linux/fs.h index 98e0349adb52..c3db8c80aed4 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3402,22 +3402,27 @@ 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 (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; + + if (kiocb_flags) + ki->ki_flags |= kiocb_flags; return 0; }
kiocb_set_rw_flags() generates a poor code with several memory writes and a lot of jumps. Help compilers to optimise it. Tested with gcc 9.2 on x64-86, and as a result, it its output now is a plain code without jumps accumulating in a register before a memory write. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- include/linux/fs.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)