* [PATCH] block/psi: remove PSI annotations from submit_bio
@ 2022-03-09 9:43 cgel.zte
2022-03-09 19:18 ` Johannes Weiner
0 siblings, 1 reply; 7+ messages in thread
From: cgel.zte @ 2022-03-09 9:43 UTC (permalink / raw)
To: axboe, viro, hannes
Cc: linux-block, linux-kernel, linux-fsdevel, akpm, Yang Yang, Ran Xiaokai
From: Yang Yang <yang.yang29@zte.com.cn>
psi tracks the time spent submitting the IO of refaulting pages[1].
But after we tracks refault stalls from swap_readpage[2][3], there
is no need to do so anymore. Since swap_readpage already includes
IO submitting time.
[1] commit b8e24a9300b0 ("block: annotate refault stalls from IO submission")
[2] commit 937790699be9 ("mm/page_io.c: annotate refault stalls from swap_readpage")
[3] commit 2b413a1a728f ("mm: page_io: fix psi memory pressure error on cold swapins")
Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
---
block/bio.c | 8 --------
block/blk-core.c | 17 -----------------
fs/direct-io.c | 2 --
include/linux/blk_types.h | 1 -
4 files changed, 28 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 3c57b3ba727d..efbbeed348e3 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1034,9 +1034,6 @@ void __bio_add_page(struct bio *bio, struct page *page,
bio->bi_iter.bi_size += len;
bio->bi_vcnt++;
-
- if (!bio_flagged(bio, BIO_WORKINGSET) && unlikely(PageWorkingset(page)))
- bio_set_flag(bio, BIO_WORKINGSET);
}
EXPORT_SYMBOL_GPL(__bio_add_page);
@@ -1252,9 +1249,6 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
* fit into the bio, or are requested in @iter, whatever is smaller. If
* MM encounters an error pinning the requested pages, it stops. Error
* is returned only if 0 pages could be pinned.
- *
- * It's intended for direct IO, so doesn't do PSI tracking, the caller is
- * responsible for setting BIO_WORKINGSET if necessary.
*/
int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
{
@@ -1273,8 +1267,6 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
ret = __bio_iov_iter_get_pages(bio, iter);
} while (!ret && iov_iter_count(iter) && !bio_full(bio, 0));
- /* don't account direct I/O as memory stall */
- bio_clear_flag(bio, BIO_WORKINGSET);
return bio->bi_vcnt ? 0 : ret;
}
EXPORT_SYMBOL_GPL(bio_iov_iter_get_pages);
diff --git a/block/blk-core.c b/block/blk-core.c
index ddac62aebc55..faf7d950a4d5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -37,7 +37,6 @@
#include <linux/t10-pi.h>
#include <linux/debugfs.h>
#include <linux/bpf.h>
-#include <linux/psi.h>
#include <linux/part_stat.h>
#include <linux/sched/sysctl.h>
#include <linux/blk-crypto.h>
@@ -911,22 +910,6 @@ void submit_bio(struct bio *bio)
}
}
- /*
- * If we're reading data that is part of the userspace workingset, count
- * submission time as memory stall. When the device is congested, or
- * the submitting cgroup IO-throttled, submission can be a significant
- * part of overall IO time.
- */
- if (unlikely(bio_op(bio) == REQ_OP_READ &&
- bio_flagged(bio, BIO_WORKINGSET))) {
- unsigned long pflags;
-
- psi_memstall_enter(&pflags);
- submit_bio_noacct(bio);
- psi_memstall_leave(&pflags);
- return;
- }
-
submit_bio_noacct(bio);
}
EXPORT_SYMBOL(submit_bio);
diff --git a/fs/direct-io.c b/fs/direct-io.c
index aef06e607b40..5cac8c8869c5 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -419,8 +419,6 @@ static inline void dio_bio_submit(struct dio *dio, struct dio_submit *sdio)
unsigned long flags;
bio->bi_private = dio;
- /* don't account direct I/O as memory stall */
- bio_clear_flag(bio, BIO_WORKINGSET);
spin_lock_irqsave(&dio->bio_lock, flags);
dio->refcount++;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 0c7a9a1f06c8..ab50c59b02ce 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -314,7 +314,6 @@ enum {
BIO_NO_PAGE_REF, /* don't put release vec pages */
BIO_CLONED, /* doesn't own data */
BIO_BOUNCED, /* bio is a bounce bio */
- BIO_WORKINGSET, /* contains userspace workingset pages */
BIO_QUIET, /* Make BIO Quiet */
BIO_CHAIN, /* chained bio, ->bi_remaining in effect */
BIO_REFFED, /* bio has elevated ->bi_cnt */
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-09 9:43 [PATCH] block/psi: remove PSI annotations from submit_bio cgel.zte
@ 2022-03-09 19:18 ` Johannes Weiner
2022-03-10 6:46 ` Christoph Hellwig
2022-03-16 3:59 ` CGEL
0 siblings, 2 replies; 7+ messages in thread
From: Johannes Weiner @ 2022-03-09 19:18 UTC (permalink / raw)
To: cgel.zte
Cc: axboe, viro, linux-block, linux-kernel, linux-fsdevel, akpm,
Yang Yang, Ran Xiaokai
On Wed, Mar 09, 2022 at 09:43:24AM +0000, cgel.zte@gmail.com wrote:
> From: Yang Yang <yang.yang29@zte.com.cn>
>
> psi tracks the time spent submitting the IO of refaulting pages[1].
> But after we tracks refault stalls from swap_readpage[2][3], there
> is no need to do so anymore. Since swap_readpage already includes
> IO submitting time.
>
> [1] commit b8e24a9300b0 ("block: annotate refault stalls from IO submission")
> [2] commit 937790699be9 ("mm/page_io.c: annotate refault stalls from swap_readpage")
> [3] commit 2b413a1a728f ("mm: page_io: fix psi memory pressure error on cold swapins")
>
> Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
> Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
It's still needed by file cache refaults!
NAK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-09 19:18 ` Johannes Weiner
@ 2022-03-10 6:46 ` Christoph Hellwig
2022-03-10 16:16 ` Johannes Weiner
2022-03-16 3:59 ` CGEL
1 sibling, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2022-03-10 6:46 UTC (permalink / raw)
To: Johannes Weiner
Cc: cgel.zte, axboe, viro, linux-block, linux-kernel, linux-fsdevel,
akpm, Yang Yang, Ran Xiaokai
On Wed, Mar 09, 2022 at 02:18:19PM -0500, Johannes Weiner wrote:
> On Wed, Mar 09, 2022 at 09:43:24AM +0000, cgel.zte@gmail.com wrote:
> > From: Yang Yang <yang.yang29@zte.com.cn>
> >
> > psi tracks the time spent submitting the IO of refaulting pages[1].
> > But after we tracks refault stalls from swap_readpage[2][3], there
> > is no need to do so anymore. Since swap_readpage already includes
> > IO submitting time.
> >
> > [1] commit b8e24a9300b0 ("block: annotate refault stalls from IO submission")
> > [2] commit 937790699be9 ("mm/page_io.c: annotate refault stalls from swap_readpage")
> > [3] commit 2b413a1a728f ("mm: page_io: fix psi memory pressure error on cold swapins")
> >
> > Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
> > Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>
> It's still needed by file cache refaults!
Can we get proper annotations for those please? These bio-level hooks are
horrible and a maintainance nightmware.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-10 6:46 ` Christoph Hellwig
@ 2022-03-10 16:16 ` Johannes Weiner
2022-03-10 16:18 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Weiner @ 2022-03-10 16:16 UTC (permalink / raw)
To: Christoph Hellwig
Cc: cgel.zte, axboe, viro, linux-block, linux-kernel, linux-fsdevel,
akpm, Yang Yang, Ran Xiaokai
On Wed, Mar 09, 2022 at 10:46:52PM -0800, Christoph Hellwig wrote:
> On Wed, Mar 09, 2022 at 02:18:19PM -0500, Johannes Weiner wrote:
> > On Wed, Mar 09, 2022 at 09:43:24AM +0000, cgel.zte@gmail.com wrote:
> > > From: Yang Yang <yang.yang29@zte.com.cn>
> > >
> > > psi tracks the time spent submitting the IO of refaulting pages[1].
> > > But after we tracks refault stalls from swap_readpage[2][3], there
> > > is no need to do so anymore. Since swap_readpage already includes
> > > IO submitting time.
> > >
> > > [1] commit b8e24a9300b0 ("block: annotate refault stalls from IO submission")
> > > [2] commit 937790699be9 ("mm/page_io.c: annotate refault stalls from swap_readpage")
> > > [3] commit 2b413a1a728f ("mm: page_io: fix psi memory pressure error on cold swapins")
> > >
> > > Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
> > > Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> >
> > It's still needed by file cache refaults!
>
> Can we get proper annotations for those please? These bio-level hooks are
> horrible and a maintainance nightmware.
The first version did that, but it was sprawling and not well-received:
https://lkml.org/lkml/2019/7/22/1261
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-10 16:16 ` Johannes Weiner
@ 2022-03-10 16:18 ` Christoph Hellwig
2022-03-10 17:31 ` Johannes Weiner
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2022-03-10 16:18 UTC (permalink / raw)
To: Johannes Weiner
Cc: Christoph Hellwig, cgel.zte, axboe, viro, linux-block,
linux-kernel, linux-fsdevel, akpm, Yang Yang, Ran Xiaokai
On Thu, Mar 10, 2022 at 11:16:41AM -0500, Johannes Weiner wrote:
> The first version did that, but it was sprawling and not well-received:
>
> https://lkml.org/lkml/2019/7/22/1261
Well, Dave's comments are spot on. Except that we replaced it with
something even more horrible and not something sensible as he suggested.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-10 16:18 ` Christoph Hellwig
@ 2022-03-10 17:31 ` Johannes Weiner
0 siblings, 0 replies; 7+ messages in thread
From: Johannes Weiner @ 2022-03-10 17:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: cgel.zte, axboe, viro, linux-block, linux-kernel, linux-fsdevel,
akpm, Yang Yang, Ran Xiaokai
On Thu, Mar 10, 2022 at 08:18:31AM -0800, Christoph Hellwig wrote:
> On Thu, Mar 10, 2022 at 11:16:41AM -0500, Johannes Weiner wrote:
> > The first version did that, but it was sprawling and not well-received:
> >
> > https://lkml.org/lkml/2019/7/22/1261
>
> Well, Dave's comments are spot on. Except that we replaced it with
> something even more horrible and not something sensible as he suggested.
Confused. I changed it the way Dave suggested, to which he replied
"this is much cleaner and easier to maintain". Are we reading
different threads? Care to elaborate?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/psi: remove PSI annotations from submit_bio
2022-03-09 19:18 ` Johannes Weiner
2022-03-10 6:46 ` Christoph Hellwig
@ 2022-03-16 3:59 ` CGEL
1 sibling, 0 replies; 7+ messages in thread
From: CGEL @ 2022-03-16 3:59 UTC (permalink / raw)
To: Johannes Weiner
Cc: axboe, viro, linux-block, linux-kernel, linux-fsdevel, akpm,
Yang Yang, Ran Xiaokai
On Wed, Mar 09, 2022 at 02:18:19PM -0500, Johannes Weiner wrote:
> On Wed, Mar 09, 2022 at 09:43:24AM +0000, cgel.zte@gmail.com wrote:
> > From: Yang Yang <yang.yang29@zte.com.cn>
> >
> > psi tracks the time spent submitting the IO of refaulting pages[1].
> > But after we tracks refault stalls from swap_readpage[2][3], there
> > is no need to do so anymore. Since swap_readpage already includes
> > IO submitting time.
> >
> > [1] commit b8e24a9300b0 ("block: annotate refault stalls from IO submission")
> > [2] commit 937790699be9 ("mm/page_io.c: annotate refault stalls from swap_readpage")
> > [3] commit 2b413a1a728f ("mm: page_io: fix psi memory pressure error on cold swapins")
> >
> > Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
> > Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>
> It's still needed by file cache refaults!
>
> NAK
What about submit_bio() only counts file cache refaults?
This will reduce redundant calling of psi_memstall_enter()
for swap_readpage().
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-16 3:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 9:43 [PATCH] block/psi: remove PSI annotations from submit_bio cgel.zte
2022-03-09 19:18 ` Johannes Weiner
2022-03-10 6:46 ` Christoph Hellwig
2022-03-10 16:16 ` Johannes Weiner
2022-03-10 16:18 ` Christoph Hellwig
2022-03-10 17:31 ` Johannes Weiner
2022-03-16 3:59 ` CGEL
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.