From: Dario Faggioli <dario.faggioli@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>,
Jan Beulich <JBeulich@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
xen-devel@lists.xenproject.org, Wei Liu <wei.liu2@citrix.com>,
osstest service owner <osstest-admin@xenproject.org>
Subject: Re: [PATCH] xen: credit2: fix spinlock irq-safety violation
Date: Thu, 21 Sep 2017 02:29:12 +0200 [thread overview]
Message-ID: <1505953752.3483.14.camel@citrix.com> (raw)
In-Reply-To: <6facfc2d-c117-8e7c-6402-d5e30ef98cf2@citrix.com>
[-- Attachment #1.1: Type: text/plain, Size: 1771 bytes --]
On Wed, 2017-09-20 at 14:49 +0100, George Dunlap wrote:
> On 09/20/2017 12:59 PM, Jan Beulich wrote:
> > Hmm, killing the timer upfront is certainly fine, but is freeing
> > the
> > data before removing the element from the list safe not only
> > currently, but also going forward?
>
> I agree with Jan -- if you don't want to put the kill_timer() in the
> critical section, put it beforehand; but don't free the structure
> until
> after the sdom struct has been removed from the list.
>
In general, I agree, and so I'll do this.
In this case, considering what list we are talking about, and what it
is used for right now (i.e., only for debug dump!), there are no
dependencies between these two operations.
And in any scenario that I can anticipate, where such a dependency
would come into being, the level of restructuring of the code that is
necessary to use the list in any really useful way, would be
significant, and there may be a few other cases where a similar
dependency would also surface and become an issue, and that will
probably lead us to consider/remember this code here as well.
So, IOW, I wouldn't consider this a problem, in the specific case. But
since the net effect of this patch and what George suggests is the
same, and since I appreciate the value that it has, in principle, doing
changes like these with this approach, I'm fine with killing before and
freeing after the critical sections, and I'll send a patch for that
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
prev parent reply other threads:[~2017-09-21 0:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-19 2:11 [PATCH] xen: credit2: fix spinlock irq-safety violation Dario Faggioli
2017-09-19 8:23 ` Wei Liu
2017-09-20 9:16 ` Roger Pau Monné
2017-09-20 9:19 ` George Dunlap
2017-09-20 10:04 ` George Dunlap
2017-09-20 11:44 ` Dario Faggioli
2017-09-20 11:59 ` Jan Beulich
2017-09-20 13:49 ` George Dunlap
2017-09-21 0:29 ` Dario Faggioli [this message]
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=1505953752.3483.14.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=JBeulich@suse.com \
--cc=george.dunlap@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=osstest-admin@xenproject.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.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;
as well as URLs for NNTP newsgroup(s).