All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Sitsofe Wheeler <sitsofe@yahoo.com>,
	Borislav Petkov <bp@alien8.de>, Meelis Roos <mroos@linux.ee>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kay Sievers <kay.sievers@vrfy.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RESEND 2/3 v2.6.39-rc7] block: make disk_block_events() properly wait for work cancellation
Date: Wed, 18 May 2011 02:46:30 -0700	[thread overview]
Message-ID: <BANLkTi=0_bv7s6=i2v-iP0vQmHHT3=tm8w@mail.gmail.com> (raw)
In-Reply-To: <20110518050729.GA16870@mtj.dyndns.org>

On Tue, May 17, 2011 at 10:07 PM, Tejun Heo <tj@kernel.org> wrote:
>
>> Just make the semaphore protect the count - and you're done.
>
> Yeah, with that gone, we don't even need the open-coding inside
> disk_check_events().  It can simply call syncing block and unblock.
> But, do you want that in -rc7?  Unnecessarily complicated as the
> current code may be, converting the lock to mutex is a larger change
> than adding an outer mutex and I think it would be better to do that
> during the next cycle.

Quite frankly. right now I think I need to just release 2.6.39, and
then for 2.6.40 merge the trivial

  mutex_lock(&ev->mutex);
  if (!ev->block++)
    cancel_delayed_work_sync(&ev->dwork);
  mutex_unlock(&ev->mutex);

with a cc: stable for backporting.

I'd _much_ prefer simple obvious code than have a outer mutex etc.
Just make the rule be that "blocked" is protected by the new
semaphore. I don't think it's used very often, and anybody who wants
to block disk events needs to be in blockable context in order to wait
for the delayed work cancel, right? So we can't be in some atomic
context inside some other spinlock anyway, afaik. And there can be no
lock order issues, since this would always be a new inner lock.

Hmm?

                          Linus

  reply	other threads:[~2011-05-18  9:47 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-17 10:27 [PATCH RESEND 1/3 v2.6.39-rc7] block: don't use non-syncing event blocking in disk_check_events() Tejun Heo
2011-05-17 10:28 ` [PATCH RESEND 2/3 v2.6.39-rc7] block: remove non-syncing __disk_block_events() and fold it into disk_block_events() Tejun Heo
2011-05-17 10:28   ` [PATCH RESEND 2/3 v2.6.39-rc7] block: make disk_block_events() properly wait for work cancellation Tejun Heo
2011-05-17 14:46     ` Linus Torvalds
2011-05-17 15:11       ` Tejun Heo
2011-05-17 15:15         ` Linus Torvalds
2011-05-17 15:27           ` Tejun Heo
2011-05-17 22:40             ` Linus Torvalds
2011-05-18  5:07               ` Tejun Heo
2011-05-18  9:46                 ` Linus Torvalds [this message]
2011-05-18 10:04                   ` Tejun Heo
2011-05-18 11:07                     ` Tejun Heo
2011-05-18 10:26                   ` Jens Axboe
2011-05-17 15:47     ` [PATCH UPDATED " Tejun Heo
2011-05-17 19:34       ` Jens Axboe
2011-05-17 20:22         ` Borislav Petkov

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='BANLkTi=0_bv7s6=i2v-iP0vQmHHT3=tm8w@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=bp@alien8.de \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mroos@linux.ee \
    --cc=sitsofe@yahoo.com \
    --cc=tj@kernel.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.