On 1/7/2026 8:33 AM, Nanzhe Zhao wrote:
Hi Chao yu:
At 2026-01-06 17:31:20, "Chao Yu" <[email protected]> wrote:
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 66ab7a43a56f..ac569a396914 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2430,6 +2430,7 @@ static int f2fs_read_data_large_folio(struct inode *inode,
unsigned nrpages;
struct f2fs_folio_state *ffs;
int ret = 0;
+ bool folio_in_bio = false;
No need to initialize folio_in_bio?
Agreed. It's redundant since we reset it to false for each new folio before
processing.
@@ -2539,6 +2542,11 @@ static int f2fs_read_data_large_folio(struct inode
*inode,
}
trace_f2fs_read_folio(folio, DATA);
if (rac) {
+ if (!folio_in_bio) {
+ if (!ret)
+ folio_mark_uptodate(folio);
+ folio_unlock(folio);
+ }
err_out:
/* Nothing was submitted. */
if (!bio) {
if (!ret)
folio_mark_uptodate(folio);
folio_unlock(folio);
^^^^^^^^^^^^
If all folios in rac have not been mapped (hole case), will we unlock the folio
twice?
Are you worried the folio could be unlocked once in the if (rac) { ... } block
and then
unlocked again at err_out:? If so, I think that won't happen.
In such a case, every non-NULL folio will be unlocked exactly once by:
if (!folio_in_bio) {
if (!ret)
folio_mark_uptodate(folio);
folio_unlock(folio);
}
Specifically, after the last folio runs through the block above, the next call:
folio = readahead_folio(rac);
will return NULL. Then we go to next_folio:, and will directly hit:
if (!folio)
goto out;
This jumps straight to the out: label, skipping err_out: entirely.
Therefore, when ret is not an error code, the err_out: label will never be
reached.
If ret becomes an error code, then the current folio will immediately goto
err_out;
and be unlocked there once.
If rac is NULL (meaning we only read the single large folio passed in as the
function argument),
we won't enter the if (rac) { ... goto next_folio; } path at all, so we also
won't go to next_folio
and then potentially goto out;. In that case, it will naturally be unlocked
once at err_out:.
Or am I missing some edge case here?
Nanzhe,
Oh, yes, I think so, thanks for the explanation.
Thanks,
Thanks,
Nanzhe
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
