| Message ID | 20221125070959.49027-1-zhangjiachen.jaycee@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | filemap: Fix some misleading comments | expand |
On Fri, 25 Nov 2022 15:09:59 +0800 Jiachen Zhang <zhangjiachen.jaycee@bytedance.com> wrote: > The users of filemap_write_and_wait_range() and file_write_and_wait_range() > interfaces should set the lend parameter to LLONG_MAX, rather than -1, to > indicate they want to writeback to the very end-of-file, as several kernel > code paths are checking the 'wbc->range_end == LLONG_MAX' conditions. Unclear. LLONG_MAX differs from -1 on 64-bit and differs differently on 32-bit. > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -661,7 +661,8 @@ EXPORT_SYMBOL_GPL(filemap_range_has_writeback); > * Write out and wait upon file offsets lstart->lend, inclusive. > * > * Note that @lend is inclusive (describes the last byte to be written) so > - * that this function can be used to write to the very end-of-file (end = -1). > + * that this function can be used to write to the very end-of-file (@lend = > + * LLONG_MAX). > * The write(2) manpage says "According to POSIX.1, if count is greater than SSIZE_MAX, the result is implementation-defined; see NOTES for the upper limit on Linux." And filemap_fdatawrite_wbc() enforces LONG_MAX, which differs from LLONG_MAX on 32-bit. I suspect more research is needed here.
On Sat, Nov 26, 2022 at 8:52 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Fri, 25 Nov 2022 15:09:59 +0800 Jiachen Zhang <zhangjiachen.jaycee@bytedance.com> wrote: > > > The users of filemap_write_and_wait_range() and file_write_and_wait_range() > > interfaces should set the lend parameter to LLONG_MAX, rather than -1, to > > indicate they want to writeback to the very end-of-file, as several kernel > > code paths are checking the 'wbc->range_end == LLONG_MAX' conditions. > > Unclear. LLONG_MAX differs from -1 on 64-bit and differs differently > on 32-bit. > I think whether using -1 or LLONG_MAX causes no difference if there is no other code comparing 'wbc->range_end == LLONG_MAX'. There is no case in the kernel code using -1 for now, but maybe we'd better fix the misleading comments to prevent future misuse. > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -661,7 +661,8 @@ EXPORT_SYMBOL_GPL(filemap_range_has_writeback); > > * Write out and wait upon file offsets lstart->lend, inclusive. > > * > > * Note that @lend is inclusive (describes the last byte to be written) so > > - * that this function can be used to write to the very end-of-file (end = -1). > > + * that this function can be used to write to the very end-of-file (@lend = > > + * LLONG_MAX). > > * > > The write(2) manpage says "According to POSIX.1, if count is greater > than SSIZE_MAX, the result is implementation-defined; see NOTES for the > upper limit on Linux." And filemap_fdatawrite_wbc() enforces LONG_MAX, > which differs from LLONG_MAX on 32-bit. > > I suspect more research is needed here. The reason 'wbc.nr_to_write' might be set to LONG_MAX for filemap_fdatawrite_wbc() might be because 'nr_to_write' is defined as the 'long' type. Maybe it should be fine as 'lend' and 'range_end' are defined as type 'off_t'. Thanks, Jiachen
diff --git a/mm/filemap.c b/mm/filemap.c index 65eee6ec1066..c6d066a39425 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -661,7 +661,8 @@ EXPORT_SYMBOL_GPL(filemap_range_has_writeback); * Write out and wait upon file offsets lstart->lend, inclusive. * * Note that @lend is inclusive (describes the last byte to be written) so - * that this function can be used to write to the very end-of-file (end = -1). + * that this function can be used to write to the very end-of-file (@lend = + * LLONG_MAX). * * Return: error status of the address space. */ @@ -758,7 +759,8 @@ EXPORT_SYMBOL(file_check_and_advance_wb_err); * Write out and wait upon file offsets lstart->lend, inclusive. * * Note that @lend is inclusive (describes the last byte to be written) so - * that this function can be used to write to the very end-of-file (end = -1). + * that this function can be used to write to the very end-of-file (@lend = + * LLONG_MAX). * * After writing out and waiting on the data, we check and advance the * f_wb_err cursor to the latest value, and return any errors detected there.
The users of filemap_write_and_wait_range() and file_write_and_wait_range() interfaces should set the lend parameter to LLONG_MAX, rather than -1, to indicate they want to writeback to the very end-of-file, as several kernel code paths are checking the 'wbc->range_end == LLONG_MAX' conditions. Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com> --- mm/filemap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)