On Fri, Jan 26, 2024 at 05:28:58PM +0800, Zhaoyang Huang wrote:

On Fri, Jan 26, 2024 at 4:55 PM Matthew Wilcox [off-list ref] wrote:

quoted

On Fri, Jan 26, 2024 at 03:59:48PM +0800, Zhaoyang Huang wrote:

quoted

loop more mm and fs guys for more comments

I agree with everything Damien said.  But also ...

ok, I will find a way to solve this problem.

quoted

quoted

quoted

+bool BIO_ADD_FOLIO(struct bio *bio, struct folio *folio, size_t len,
+               size_t off)

You don't add any users of these functions.  It's hard to assess whether
this is the right API when there are no example users.

Actually, the code has been tested on ext4 and f2fs by patchv2 on a
v6.6 6GB android system where I get the test result posted on the
commit message. These APIs is to keep block layer clean and wrap
things up for fs.

well, where's patch v2?  i don't see it in my inbox.  i'm not going
to go hunting around the email lists for it.  this is not good enough.

quoted

why are BIO_ADD_PAGE and BIO_ADD_FOLIO so very different from each
other?

These two API just repeat the same thing that bio_add_page and
bio_add_folio do.

what?

here's the patch you sent.  these two functions do wildly different
things:

+bool BIO_ADD_FOLIO(struct bio *bio, struct folio *folio, size_t len,
+		size_t off)
+{
+	int class, level, hint, activity;
+
+	if (len > UINT_MAX || off > UINT_MAX)
+		return false;
+
+	class = IOPRIO_PRIO_CLASS(bio->bi_ioprio);
+	level = IOPRIO_PRIO_LEVEL(bio->bi_ioprio);
+	hint = IOPRIO_PRIO_HINT(bio->bi_ioprio);
+	activity = IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio);
+
+	activity += (bio->bi_vcnt + 1 <= IOPRIO_NR_ACTIVITY &&
+			PageWorkingset(&folio->page)) ? 1 : 0;
+	if (activity >= bio->bi_vcnt / 2)
+		class = IOPRIO_CLASS_RT;
+	else if (activity >= bio->bi_vcnt / 4)
+		class = max(IOPRIO_PRIO_CLASS(get_current_ioprio()), IOPRIO_CLASS_BE);
+
+	bio->bi_ioprio = IOPRIO_PRIO_VALUE_ACTIVITY(class, level, hint, activity);
+
+	return bio_add_page(bio, &folio->page, len, off) > 0;
+}
+
+int BIO_ADD_PAGE(struct bio *bio, struct page *page,
+		unsigned int len, unsigned int offset)
+{
+	int class, level, hint, activity;
+
+	if (bio_add_page(bio, page, len, offset) > 0) {
+		class = IOPRIO_PRIO_CLASS(bio->bi_ioprio);
+		level = IOPRIO_PRIO_LEVEL(bio->bi_ioprio);
+		hint = IOPRIO_PRIO_HINT(bio->bi_ioprio);
+		activity = IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio);
+		activity += (bio->bi_vcnt <= IOPRIO_NR_ACTIVITY && PageWorkingset(page)) ? 1 : 0;
+		bio->bi_ioprio = IOPRIO_PRIO_VALUE_ACTIVITY(class, level, hint, activity);
+	}
+
+	return len;
+}

did you change one and forget to change the other?

These white spaces are trimmed by vim, I will change them back in next version.

vim doesn't do that by default.