On 5/4/23 18:12, 胡玮文 wrote:
Hi Xiubo,
I just send a patch to solve the same issue[1]. My patch bound the
request by i_size, which is orthogonal to your changes. To incorporate
both, I propose the following patch.
Also, since this is a performance regression, I think we should backport
it to stable versions?
Another comment inline.
Hu Weiwen
[1]: https://lore.kernel.org/ceph-devel/20230504082510.247-1-sehuww@xxxxxxxxxxxxxxxx/
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 6bb251a4d613..d1d8e2562182 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -187,16 +187,30 @@ static void ceph_netfs_expand_readahead(struct netfs_io_request *rreq)
struct inode *inode = rreq->inode;
struct ceph_inode_info *ci = ceph_inode(inode);
struct ceph_file_layout *lo = &ci->i_layout;
+ unsigned long max_pages = inode->i_sb->s_bdi->ra_pages;
+ unsigned long max_len = max_pages << PAGE_SHIFT;
+ loff_t end = rreq->start + rreq->len, new_end;
u32 blockoff;
u64 blockno;
- /* Expand the start downward */
- blockno = div_u64_rem(rreq->start, lo->stripe_unit, &blockoff);
- rreq->start = blockno * lo->stripe_unit;
- rreq->len += blockoff;
+ /* Readahead is disabled */
+ if (!max_pages)
+ return;
+
+ /* Try to expand the length forward by rounding up it to the next block */
+ new_end = round_up(end, lo->stripe_unit);
+ /* But do not exceed the file size,
+ * unless the original request already exceeds it. */
+ new_end = min(new_end, rreq->i_size);
+ if (new_end > end && new_end <= rreq->start + max_len)
+ rreq->len = new_end - rreq->start;
- /* Now, round up the length to the next block */
- rreq->len = roundup(rreq->len, lo->stripe_unit);
+ /* Try to expand the start downward */
+ blockno = div_u64_rem(rreq->start, lo->stripe_unit, &blockoff);
+ if (rreq->len + blockoff <= max_len) {
+ rreq->start = blockno * lo->stripe_unit;
+ rreq->len += blockoff;
+ }
}
static bool ceph_netfs_clamp_length(struct netfs_io_subrequest *subreq)
On Thu, May 04, 2023 at 01:23:06PM +0800, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>
Blindly expanding the readahead windows will cause unneccessary
pagecache thrashing and also will introdue the network workload.
We should disable expanding the windows if the readahead is disabled
and also shouldn't expand the windows too much.
Expanding forward firstly instead of expanding backward for possible
sequential reads.
URL: https://www.spinics.net/lists/ceph-users/msg76183.html
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
V2:
- fix possible cross-block issue pointed out by Ilya.
fs/ceph/addr.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index ca4dc6450887..03a326da8fd8 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -188,16 +188,28 @@ static void ceph_netfs_expand_readahead(struct netfs_io_request *rreq)
struct inode *inode = rreq->inode;
struct ceph_inode_info *ci = ceph_inode(inode);
struct ceph_file_layout *lo = &ci->i_layout;
+ unsigned long max_pages = inode->i_sb->s_bdi->ra_pages;
+ unsigned long max_len = max_pages << PAGE_SHIFT;
+ unsigned long len;
u32 blockoff;
u64 blockno;
- /* Expand the start downward */
- blockno = div_u64_rem(rreq->start, lo->stripe_unit, &blockoff);
- rreq->start = blockno * lo->stripe_unit;
- rreq->len += blockoff;
+ /* Readahead is disabled */
+ if (!max_pages)
+ return;
+
+ /* Try to expand the length forward by rounding up it to the next block */
+ div_u64_rem(rreq->start + rreq->len, lo->stripe_unit, &blockoff);
+ len = lo->stripe_unit - blockoff;
This would expand the request by a whole block, if the original request
is already aligned (blockoff == 0).
Hi 胡玮文
Good catch.
The above patch looks better and I will revise it.
Thanks
- Xiubo
+ if (rreq->len + len <= max_len)
+ rreq->len += len;
- /* Now, round up the length to the next block */
- rreq->len = roundup(rreq->len, lo->stripe_unit);
+ /* Try to expand the start downward */
+ blockno = div_u64_rem(rreq->start, lo->stripe_unit, &blockoff);
+ if (rreq->len + blockoff <= max_len) {
+ rreq->start = blockno * lo->stripe_unit;
+ rreq->len += blockoff;
+ }
}
static bool ceph_netfs_clamp_length(struct netfs_io_subrequest *subreq)
--
2.40.0
