From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "mingo@kernel.org" <mingo@kernel.org>,
"longman@redhat.com" <longman@redhat.com>,
"rostedt@goodmis.org" <rostedt@goodmis.org>,
"axboe@kernel.dk" <axboe@kernel.dk>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH v4] blktrace: Fix potentail deadlock between delete & sysfs ops
Date: Fri, 18 Aug 2017 20:18:13 +0000 [thread overview]
Message-ID: <1503087492.2622.13.camel@wdc.com> (raw)
In-Reply-To: <1503086474-20187-1-git-send-email-longman@redhat.com>
On Fri, 2017-08-18 at 16:01 -0400, Waiman Long wrote:
> [ ... ]
> Instead of using bd_mutex, a new global blktrace mutex is now used
> to protect against concurrent access, creation and destruction of the
> blk_trace structure that is used only in the blktrace.c file. As
> blktrace files will not be frequently accessed, using a global mutex
> should not cause any performance problem.
> [ ... ]
> +/*
> + * The bd_mutex was used previously for protecting blk_trace structure.
> + * That could lead to deadlock with concurrent block device deletion and
> + * sysfs access. So a global blktrace_mutex is now used instead for
> + * protecting the blk_trace structure.
> + *
> + * The references to the opened sysfs or device files should prevent the
> + * underlying block device from being removed.
> + */
> +static DEFINE_MUTEX(blktrace_mutex);
Hello Waiman,
Thanks for having addressed my previous comment. Regarding this patch: sorry
but I don't think it's a good idea to use a global mutex for serializing
accesses to tracing data of a single block device. Global mutexes create
unwanted lock dependencies between different block devices. Additionally, on
multiprocessor systems global mutexes can cause cache line ping-pong between
processors and hence can cause a severe slowdown. Please make blktrace_mutex
per block device instead of global.
Thanks,
Bart.
next prev parent reply other threads:[~2017-08-18 20:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-18 20:01 Waiman Long
2017-08-18 20:18 ` Bart Van Assche [this message]
2017-08-18 20:35 ` Waiman Long
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=1503087492.2622.13.camel@wdc.com \
--to=bart.vanassche@wdc.com \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.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®