erofs: fix refcount on the metabuf used for inode lookup
Subsystems (2)
Sandeep Dhavale
reviewerPATCH 1/1 erofs: fix refcount on the metabuf used for inode lookup
In erofs_find_target_block() when erofs_dirnamecmp() returns 0, we do not assign the target metabuf. This causes the caller erofs_namei()'s erofs_put_metabuf() at the end to be not effective leaving the refcount on the page.
As the page from metabuf (buf->page) is never put, such page cannot be migrated or reclaimed. Fix it now by putting the metabuf from previous loop and assigning the current metabuf to target before returning so caller erofs_namei() can do the final put as it was intended.
--- fs/erofs/namei.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/erofs/namei.c +4 -1
@@ -132,7 +132,10 @@ static void *erofs_find_target_block(struct erofs_buf *target,if (!diff) { *_ndirents =0 ; - goto out; +if (!IS_ERR (candidate)) +erofs_put_metabuf (target); + *target = buf; +return de; }else if (diff >0 ) { head = mid +1 ; startprfx = matched;
Signature · 1 line
2.44.0.rc0.258.g7320e95886-goog
Hi Sandeep,
Quoted 10 lines · Sandeep Dhavale
In
erofs_find_target_block()whenerofs_dirnamecmp()returns 0, we do not assign the target metabuf. This causes the caller erofs_namei()'serofs_put_metabuf()at the end to be not effective leaving the refcount on the page.
As the page from metabuf (buf->page) is never put, such page cannot be migrated or reclaimed. Fix it now by putting the metabuf from previous loop and assigning the current metabuf to target before returning so callererofs_namei()can do the final put as it was intended.
Quoted 3 lines
Fixes 500edd095648 ("erofs: use meta buffers for inode lookup")Signed-off-by Sandeep Dhavale <[email protected]>
Many thanks for the catch!
Quoted 10 lines
--- fs/erofs/namei.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)fs/erofs/namei.c +0
-0@@ -132,7 +132,10 @@ static void *erofs_find_target_block(struct erofs_buf *target,
fs/erofs/namei.c static void *erofs_find_target_block(struct erofs_buf *target, · line 141
Quoted 10 lines
fs/erofs/namei.c +4
-1if (!diff) { *_ndirents =0 ; - goto out; +if (!IS_ERR (candidate)) +erofs_put_metabuf (target); + *target = buf; +return de; }else if (diff >0 ) { head = mid +1 ; startprfx = matched;
The fix is correct, yet I tend to try to reorganize this snippet for simplicity, how about the following diff (untested)?
If it looks good to you, could you resend a formal patch? Thanks!
Thanks,
Gao Xiang
fs/erofs/namei.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
fs/erofs/namei.c +14 -15
@@ -129,25 +129,24 @@ static void *erofs_find_target_block(struct erofs_buf *target,/* string comparison without already matched prefix */ diff =erofs_dirnamecmp (name, &dname, &matched); - -if (!diff) { - *_ndirents =0 ; - goto out; - }else if (diff >0 ) { - head = mid +1 ; - startprfx = matched; - -if (!IS_ERR (candidate)) -erofs_put_metabuf (target); - *target = buf; - candidate = de; - *_ndirents = ndirents; - }else { +if (diff <0 ) {erofs_put_metabuf (&buf); - back = mid -1 ; endprfx = matched; +continue ; } + +if (!IS_ERR (candidate)) +erofs_put_metabuf (target); + *target = buf; +if (!diff) { + *_ndirents =0 ; +return de; + } + head = mid +1 ; + startprfx = matched; + candidate = de; + *_ndirents = ndirents;continue ; } out:/* free if the candidate is valid */
--
2.39.3
Sandeep Dhavale
reviewerQuoted 3 lines
If it looks good to you, could you resend a formal patch? Thanks!
Hi Gao,
This looks better and more readable. I will send a v2.
Thanks,
Sandeep.
Quoted 2 lines
Thanks,
Gao Xiang