mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Paolo Valente <paolo.valente@linaro.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org,
	Linux-Kernal <linux-kernel@vger.kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	broonie@kernel.org
Subject: Re: [PATCH BUGFIX] block, bfq: access and cache blkg data only when safe
Date: Wed, 24 May 2017 10:50:23 -0400	[thread overview]
Message-ID: <20170524145023.GA24798@htj.duckdns.org> (raw)
In-Reply-To: <6E15A7C4-398F-4A93-A717-C499B3900EF0@linaro.org>

Hello, Paolo.

On Wed, May 24, 2017 at 12:53:26PM +0100, Paolo Valente wrote:
> Exact, but even after all blkgs, as well as the cfq_group and pd, are
> gone, the children cfq_queues of the gone cfq_group continue to point
> to unexisting objects, until new cfq_set_requests are executed for
> those cfq_queues.  To try to make this statement clearer, here is the
> critical sequence for a cfq_queue, say cfqq, belonging to a cfq_group,
> say cfqg:
> 
> 1 cfq_set_request for a request rq of cfqq
> 2 removal of (the process associated with cfqq) from bfqg
> 3 destruction of the blkg that bfqg is associated with
> 4 destruction of the blkcg the above blkg belongs to
> 5 destruction of the pd pointed to by cfqg, and of cfqg itself
> !!!-> from now on cfqq->cfqg is a dangling reference <-!!!
> 6 execution of cfq functions, different from cfq_set_request, on cfqq
> 	. cfq_insert, cfq_dispatch, cfq_completed_rq, ...
> 7 execution of a new cfq_set_request for cfqq
> -> now cfqq->cfqg is again a sane pointer <-
> 
> Every function executed at step 6 sees a dangling reference for
> cfqq->cfqg.
> 
> My fix for caching data doesn't solve this more serious problem.
> 
> Where have I been mistaken?

Hmmm... cfq_set_request() invokes cfqg_get() which increases refcnt on
the blkg, which should pin everything down till the request is done,
so none of the above objects can be destroyed before the request is
done.

Thanks.

-- 
tejun

  parent reply	other threads:[~2017-05-24 14:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-19  8:39 Paolo Valente
2017-05-19 14:37 ` Jens Axboe
2017-05-22 20:20   ` Paolo Valente
2017-05-19 14:54 ` Tejun Heo
2017-05-20  7:27   ` Paolo Valente
2017-05-23 20:42     ` Tejun Heo
2017-05-24 11:53       ` Paolo Valente
2017-05-24 14:24         ` Paolo Valente
2017-05-24 14:50         ` Tejun Heo [this message]
2017-05-24 16:43           ` Paolo Valente
2017-05-24 16:47             ` Tejun Heo
2017-05-25  7:10               ` Paolo Valente
2017-05-25 14:37                 ` Tejun Heo
2017-06-05  8:11 Paolo Valente
2017-06-08 15:30 ` Paolo Valente
2017-06-08 15:52   ` Jens Axboe

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=20170524145023.GA24798@htj.duckdns.org \
    --to=tj@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=broonie@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paolo.valente@linaro.org \
    --cc=ulf.hansson@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®