All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>,
	Michal Piotrowski <michal.k.k.piotrowski@gmail.com>,
	Andrew Morton <akpm@osdl.org>, Neil Brown <neilb@cse.unsw.edu.au>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [patch-mm] Workaround for RAID breakage
Date: Tue, 16 Jan 2007 21:07:58 +1100	[thread overview]
Message-ID: <20070116100758.GC3938@kernel.dk> (raw)
In-Reply-To: <1168936038.2941.182.camel@localhost.localdomain>

On Tue, Jan 16 2007, Thomas Gleixner wrote:
> On Tue, 2007-01-16 at 11:41 +1100, Jens Axboe wrote:
> > > AFAICS this is intentional to avoid checks all over the place, but the
> > > underflow check is missing. All we need to do is make sure, that in case
> > > of ioc->plugged == 0 we return early and bug, if there is either a queue
> > > plugged in or the plugged_list is not empty.
> > > 
> > > Jens ?
> > 
> > It should not go negative, that would be a bug elsewhere. So it's
> > interesting if it does, we should definitely put a WARN_ON() check in
> > there for that.
> 
> Well. All offenders come via queue_sync_plugs(). queue_sync_plugs()
> calls blk_unplug_current().

Ah, again a problem because the #plug branch wasn't pushed to #for-akpm
in due time. That problem was fixed already :/

> One path which triggers is blk_sync_queue(). This happens e.g. in the
> cleanup of the floppy check. There are other call pathes too.
> 
> The other is raid md_super_write(). It is not plugged and calls with the
> barrier bit set, which executes the unlikely path in __make_request():
> 
>         if (unlikely(bio_barrier(bio))) {
>                 queue_sync_plugs(q);
>                 spin_lock_irq(q->queue_lock);
>                 goto get_rq;
>         }
> 
> So you either need checks all over the place to avoid calling
> blk_unplug_current(), or you prevent the unplug below 0 like I did. The
> BUG_ON()s I added should catch any real invalid callers. But it's up to
> you.

blk_replug_current_nested() should do it. I'll make sure the branch is
updated for next -mm.

The BUG_ON()'s are indeed correct, they should just be moved further
down the function.

-- 
Jens Axboe


      reply	other threads:[~2007-01-16 10:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-12 13:33 2.6.20-rc4-mm1 md problem Michal Piotrowski
2007-01-12 15:52 ` Rafael J. Wysocki
2007-01-12 17:40   ` Michal Piotrowski
2007-01-12 17:58     ` Michal Piotrowski
2007-01-14 21:59       ` Jens Axboe
2007-01-15  7:17 ` Ingo Molnar
2007-01-15  8:08   ` Thomas Gleixner
2007-01-15 18:03     ` [patch-mm] Workaround for RAID breakage Thomas Gleixner
2007-01-15 20:17       ` Michal Piotrowski
2007-01-16  0:41       ` Jens Axboe
2007-01-16  8:27         ` Thomas Gleixner
2007-01-16 10:07           ` Jens Axboe [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=20070116100758.GC3938@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.k.k.piotrowski@gmail.com \
    --cc=mingo@elte.hu \
    --cc=neilb@cse.unsw.edu.au \
    --cc=tglx@linutronix.de \
    /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.