All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michal Koutný" <mkoutny-IBi9RG/b67k@public.gmane.org>
To: Dan Schatzberg <schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	"open list:BLOCK LAYER"
	<linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"open list:CONTROL GROUP (CGROUP)"
	<cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"open list:MEMORY MANAGEMENT"
	<linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>,
	Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>,
	Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
Subject: Re: [PATCH 3/3] loop: Charge i/o to mem and blk cg
Date: Tue, 29 Jun 2021 12:26:21 +0200	[thread overview]
Message-ID: <YNr1TYfBwR/tEpEJ@blackbook> (raw)
In-Reply-To: <YNnZ7hIRIk9dJDry@dschatzberg-fedora-PC0Y6AEN>

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Mon, Jun 28, 2021 at 10:17:18AM -0400, Dan Schatzberg <schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Agreed that exporting int_active_memcg is an implementation detail,
> but would this prevent set_active_memcg from being inlined?

Non-inlining in the loop module doesn't seem like a big trouble. OTOH,
other callers may be more sensitive and would need to rely on inlining.
I can't currently think of a nice way to have both the exported and the
exlicitly inlined variant at once. It seems it's either API or perf
craft in the end but both are uncertain, so I guess the current approach
is fine in the end.

> Yes it is intentional. All requests (not just aio) go through the loop
> worker which grabs the blkcg reference in loop_queue_work() on
> construction. So I believe grabbing a reference per request is
> unnecessary.

Isn't there a window without the reference between loop_queue_rq and
loop_queue_work? I don't know, you seem to know better, so I'd suggest
dropping a comment line into the code explaining this.

Thanks,
Michal

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Michal Koutný" <mkoutny@suse.com>
To: Dan Schatzberg <schatzberg.dan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"open list:BLOCK LAYER" <linux-block@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:CONTROL GROUP (CGROUP)" <cgroups@vger.kernel.org>,
	"open list:MEMORY MANAGEMENT" <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH 3/3] loop: Charge i/o to mem and blk cg
Date: Tue, 29 Jun 2021 12:26:21 +0200	[thread overview]
Message-ID: <YNr1TYfBwR/tEpEJ@blackbook> (raw)
In-Reply-To: <YNnZ7hIRIk9dJDry@dschatzberg-fedora-PC0Y6AEN>

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On Mon, Jun 28, 2021 at 10:17:18AM -0400, Dan Schatzberg <schatzberg.dan@gmail.com> wrote:
> Agreed that exporting int_active_memcg is an implementation detail,
> but would this prevent set_active_memcg from being inlined?

Non-inlining in the loop module doesn't seem like a big trouble. OTOH,
other callers may be more sensitive and would need to rely on inlining.
I can't currently think of a nice way to have both the exported and the
exlicitly inlined variant at once. It seems it's either API or perf
craft in the end but both are uncertain, so I guess the current approach
is fine in the end.

> Yes it is intentional. All requests (not just aio) go through the loop
> worker which grabs the blkcg reference in loop_queue_work() on
> construction. So I believe grabbing a reference per request is
> unnecessary.

Isn't there a window without the reference between loop_queue_rq and
loop_queue_work? I don't know, you seem to know better, so I'd suggest
dropping a comment line into the code explaining this.

Thanks,
Michal

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-06-29 10:26 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 17:39 [PATCH V14 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
2021-06-10 17:39 ` Dan Schatzberg
     [not found] ` <20210610173944.1203706-1-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-06-10 17:39   ` [PATCH 1/3] loop: Use worker per cgroup instead of kworker Dan Schatzberg
2021-06-10 17:39     ` Dan Schatzberg
2021-06-10 17:39   ` [PATCH 2/3] mm: Charge active memcg when no mm is set Dan Schatzberg
2021-06-10 17:39     ` Dan Schatzberg
2021-06-25 14:47     ` Michal Koutný
2021-06-10 17:39 ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2021-06-10 17:39   ` Dan Schatzberg
2021-06-25 15:01   ` Michal Koutný
2021-06-28 14:17     ` Dan Schatzberg
2021-06-29 10:26       ` Michal Koutný [this message]
2021-06-29 10:26         ` Michal Koutný
2021-06-29 14:03         ` Dan Schatzberg
2021-06-29 14:03           ` Dan Schatzberg
2021-06-30  9:42           ` Michal Koutný
2021-06-30 14:49             ` Dan Schatzberg
  -- strict thread matches above, loose matches on Subject: below --
2021-06-03 14:57 [PATCH V13 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
     [not found] ` <20210603145707.4031641-1-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-06-03 14:57   ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2021-06-03 14:57     ` Dan Schatzberg
2021-04-02 19:16 [PATCH V12 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
     [not found] ` <20210402191638.3249835-1-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-04-02 19:16   ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2021-04-02 19:16     ` Dan Schatzberg
2021-04-02 19:16     ` Dan Schatzberg
     [not found]     ` <20210402191638.3249835-4-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-04-06  3:23       ` Ming Lei
2021-04-06  3:23         ` Ming Lei
2021-03-29 14:48 [PATCH V11 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
2021-03-29 14:48 ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2021-03-29 14:48   ` Dan Schatzberg
2021-03-29 14:48   ` Dan Schatzberg
2021-03-16 15:36 [PATCH v10 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
     [not found] ` <20210316153655.500806-1-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-03-16 15:36   ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2021-03-16 15:36     ` Dan Schatzberg
2021-03-16 15:36     ` Dan Schatzberg
     [not found]     ` <20210316153655.500806-4-schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2021-03-16 16:25       ` Shakeel Butt
2021-03-16 16:25         ` Shakeel Butt
2020-08-31 15:36 [PATCH v8 0/3] Charge loop device i/o to issuing cgroup Dan Schatzberg
2020-08-31 15:37 ` [PATCH 3/3] loop: Charge i/o to mem and blk cg Dan Schatzberg
2020-08-31 15:37   ` Dan Schatzberg
2020-08-31 15:37   ` Dan Schatzberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YNr1TYfBwR/tEpEJ@blackbook \
    --to=mkoutny-ibi9rg/b67k@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
    --cc=linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
    --cc=schatzberg.dan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.