[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200416061048.GA1342@infradead.org>
Date: Wed, 15 Apr 2020 23:10:48 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Luis Chamberlain <mcgrof@...nel.org>
Cc: Christoph Hellwig <hch@...radead.org>,
Alan Jenkins <alan.christopher.jenkins@...il.com>,
axboe@...nel.dk, viro@...iv.linux.org.uk, bvanassche@....org,
gregkh@...uxfoundation.org, rostedt@...dmis.org, mingo@...hat.com,
jack@...e.cz, ming.lei@...hat.com, nstange@...e.de,
akpm@...ux-foundation.org, mhocko@...e.com, yukuai3@...wei.com,
linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Omar Sandoval <osandov@...com>,
Hannes Reinecke <hare@...e.com>,
Michal Hocko <mhocko@...nel.org>
Subject: Re: [PATCH 4/5] mm/swapfile: refcount block and queue before using
blkcg_schedule_throttle()
On Wed, Apr 15, 2020 at 01:19:15PM +0000, Luis Chamberlain wrote:
> > if (current->throttle_queue)
> > return;
> > + if (unlikely(current->flags & PF_KTHREAD))
> > + return;
> >
> > spin_lock(&swap_avail_lock);
> > plist_for_each_entry_safe(si, next, &swap_avail_heads[node],
> > avail_lists[node]) {
> > - if (si->bdev) {
> > - blkcg_schedule_throttle(bdev_get_queue(si->bdev),
> > - true);
> > - break;
> > + if (!si->bdev)
> > + continue;
> > + if (blk_get_queue(dev_get_queue(si->bdev))) {
> > + current->throttle_queue = dev_get_queue(si->bdev);
> > + current->use_memdelay = true;
> > + set_notify_resume(current);
> > }
> > + break;
> > }
> > spin_unlock(&swap_avail_lock);
> > }
>
> Sorry, its not clear to me who calls the respective blk_put_queue()
> here?
If you look at blkcg_schedule_throttle, it only puts the queue that
was in current->throttle_queue. But mem_cgroup_throttle_swaprate
exits early when current->throttle_queue is non-zero (first two lines
quote above). So when called from mem_cgroup_throttle_swaprate,
blkcg_schedule_throttle should never actually put a queue. Open
coding the few relevant lines from blkcg_schedule_throttle in
mem_cgroup_throttle_swaprate makes that obvious.
Powered by blists - more mailing lists