| Message ID | 1548678679-18122-1-git-send-email-zhengbin13@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/filemap: pass inclusive 'end_byte' parameter to filemap_range_has_page | expand |
On Mon, Jan 28, 2019 at 08:31:19PM +0800, zhengbin wrote: > The 'end_byte' parameter of filemap_range_has_page is required to be > inclusive, so follow the rule. Reviewed-by: Matthew Wilcox <willy@infradead.org> Fixes: 6be96d3ad34a ("fs: return if direct I/O will trigger writeback") Adding the people in the sign-off chain to the Cc. > Signed-off-by: zhengbin <zhengbin13@huawei.com> > --- > mm/filemap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 9f5e323..a236bf3 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -3081,7 +3081,7 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from) > if (iocb->ki_flags & IOCB_NOWAIT) { > /* If there are pages to writeback, return */ > if (filemap_range_has_page(inode->i_mapping, pos, > - pos + write_len)) > + pos + write_len - 1)) > return -EAGAIN; > } else { > written = filemap_write_and_wait_range(mapping, pos, > -- > 2.7.4 >
On Mon, Jan 28, 2019 at 12:18:05PM -0800, Matthew Wilcox wrote: > On Mon, Jan 28, 2019 at 08:31:19PM +0800, zhengbin wrote: > > The 'end_byte' parameter of filemap_range_has_page is required to be > > inclusive, so follow the rule. > > Reviewed-by: Matthew Wilcox <willy@infradead.org> > Fixes: 6be96d3ad34a ("fs: return if direct I/O will trigger writeback") > > Adding the people in the sign-off chain to the Cc. This looks correct to me: Acked-by: Christoph Hellwig <hch@lst.de> I wish we'd kill these stupid range calling conventions, though - offset + len is a lot more intuitive, and we already use it very widely all over the kernel.
On 2/1/19 12:43 AM, Christoph Hellwig wrote: > On Mon, Jan 28, 2019 at 12:18:05PM -0800, Matthew Wilcox wrote: >> On Mon, Jan 28, 2019 at 08:31:19PM +0800, zhengbin wrote: >>> The 'end_byte' parameter of filemap_range_has_page is required to be >>> inclusive, so follow the rule. >> >> Reviewed-by: Matthew Wilcox <willy@infradead.org> >> Fixes: 6be96d3ad34a ("fs: return if direct I/O will trigger writeback") >> >> Adding the people in the sign-off chain to the Cc. > > This looks correct to me: > > Acked-by: Christoph Hellwig <hch@lst.de> Ditto > I wish we'd kill these stupid range calling conventions, though - > offset + len is a lot more intuitive, and we already use it very > widely all over the kernel. Wholeheartedly agree on that, it's a horrible interface that goes counter to the whole "easy to use, hard to misuse" mantra.
On Fri, Feb 01, 2019 at 08:43:59AM +0100, Christoph Hellwig wrote: > On Mon, Jan 28, 2019 at 12:18:05PM -0800, Matthew Wilcox wrote: > > On Mon, Jan 28, 2019 at 08:31:19PM +0800, zhengbin wrote: > > > The 'end_byte' parameter of filemap_range_has_page is required to be > > > inclusive, so follow the rule. > > > > Reviewed-by: Matthew Wilcox <willy@infradead.org> > > Fixes: 6be96d3ad34a ("fs: return if direct I/O will trigger writeback") > > > > Adding the people in the sign-off chain to the Cc. > > This looks correct to me: > > Acked-by: Christoph Hellwig <hch@lst.de> > > I wish we'd kill these stupid range calling conventions, though - > offset + len is a lot more intuitive, and we already use it very > widely all over the kernel. It has its own problems though; you have to check that offset + len - 1 doesn't wrap past zero. Really, it's the transition from (offset, len) to (min, max) that needs to be avoided as much as possible within a subsystem.
diff --git a/mm/filemap.c b/mm/filemap.c index 9f5e323..a236bf3 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -3081,7 +3081,7 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from) if (iocb->ki_flags & IOCB_NOWAIT) { /* If there are pages to writeback, return */ if (filemap_range_has_page(inode->i_mapping, pos, - pos + write_len)) + pos + write_len - 1)) return -EAGAIN; } else { written = filemap_write_and_wait_range(mapping, pos,
The 'end_byte' parameter of filemap_range_has_page is required to be inclusive, so follow the rule. Signed-off-by: zhengbin <zhengbin13@huawei.com> --- mm/filemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4