FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 02-02-2010, 05:17 PM
Mikulas Patocka
 
Default If all legs fail, return an error

On Tue, 2 Feb 2010, Mikulas Patocka wrote:

>
>
> On Mon, 1 Feb 2010, Takahiro Yasui wrote:
>
> > > This fixes bug https://bugzilla.redhat.com/show_bug.cgi?id=555197
> > >
> > > Please submit this patch before 2.6.33 goes out. It fixes a bug when old
> > > LVM (<= 2.02.51) is used, that doesn't pass errors_handled flag to
> > > dm-raid1.
> > >
> > > It doesn't need to be backported to RHEL 5.5, because lvm always passes a
> > > flag to handle errors there.
> >
> > Don't we need to backport it to RHEL 5.5? If lvm is the only user of dm-raid1,
> > we don't need to backport it to RHEL 5.5. But if not, we need to.
> >
> > > Don't lose writes if errors are not handled and log fails
> > >
> > > If the log fails and errors are not handled by dmeventd, the writes
> > > are successfully finished without being actually written to the device.
> > >
> > > This code path is taken:
> > > do_writes:
> > > bio_list_merge(&ms->failures, &sync);
> > > do_failures:
> > > if (!get_valid_mirror(ms)) (false)
> > > else if (errors_handled(ms)) (false)
> > > else bio_endio(bio, 0);
> > >
> > > The logic in do_failures is based on presuming that the write was already
> > > tried --- if it succeeded at least on one leg and errors are not handled,
> > > it is reported as success.
> > >
> > > However, bio can be added to the failures queue without being submitted, in
> > > do_writes.
> > >
> > > This patch changes it so that bios are added to the failures list only if
> > > errors are handled --- then, they will be held with hold_bio() called from
> > > do_failures.
> >
> > I agree that bios should be issued by do_write() when ms->log_failures is set,
> > but do we need to add bios to the failures queue? As you mentioned, the failures
> > queue should be used to bios which are already handled. Therefore, I think
> > bios are better to handled directly by hold_bio() instead of adding them to
> > the failures queue as bios for nosync regions are done.
> >
> > - if (unlikely(ms->log_failure)) {
> > - spin_lock_irq(&ms->lock);
> > - bio_list_merge(&ms->failures, &sync);
> > - spin_unlock_irq(&ms->lock);
> > - wakeup_mirrord(ms);
> > - } else
> > - while ((bio = bio_list_pop(&sync)))
> > + while ((bio = bio_list_pop(&sync)))
> > + if (unlikely(ms->log_failure) && errors_handled(ms))
> > + hold_bio(ms, bio);
> > + else
> > do_write(ms, bio);
>
> I thought about this too, but I'd decided to put the bios on the failures
> queue rather than holding them for this reason: if all the legs fail, it
> is better to terminate the bio with -EIO than to hold it. If all the legs
> fail, you can't save anything anyway and the less things you are holding,
> the less possibility for deadlocks exists.
>
> So I put the bios to the failure queue and do_failures will terminate them
> with -EIO if all the legs failred and hold them if we use dmeventd and
> there is at least one live leg.
>
> Mikulas

BTW. if you want to follow this principle (abort bio if all legs fail),
you can also apply this patch, that does it for out-of-sync regions. It is
not critical, so it can wait for 2.6.34.

Mikulas

---

If all legs fail, return an error rather than holding the bio

Add the bio to the failures list instead of holding it. It is processed
in do_failures, do_failures tests first if all legs failed and returns
the bio with -EIO in this case. If there is some leg alive and errors
are handled by dmeventd, do_failures calls hold_bio.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
drivers/md/dm-raid1.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

Index: linux-2.6.33-rc5-devel/drivers/md/dm-raid1.c
================================================== =================
--- linux-2.6.33-rc5-devel.orig/drivers/md/dm-raid1.c 2010-02-02 17:23:08.000000000 +0100
+++ linux-2.6.33-rc5-devel/drivers/md/dm-raid1.c 2010-02-02 17:23:40.000000000 +0100
@@ -737,9 +737,12 @@ static void do_writes(struct mirror_set
dm_rh_delay(ms->rh, bio);

while ((bio = bio_list_pop(&nosync))) {
- if (unlikely(ms->leg_failure) && errors_handled(ms))
- hold_bio(ms, bio);
- else {
+ if (unlikely(ms->leg_failure) && errors_handled(ms)) {
+ spin_lock_irq(&ms->lock);
+ bio_list_add(&ms->failures, bio);
+ spin_unlock_irq(&ms->lock);
+ wakeup_mirrord(ms);
+ } else {
map_bio(get_default_mirror(ms), bio);
generic_make_request(bio);
}

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-02-2010, 06:47 PM
Takahiro Yasui
 
Default If all legs fail, return an error

On 02/02/10 13:17, Mikulas Patocka wrote:
> BTW. if you want to follow this principle (abort bio if all legs fail),
> you can also apply this patch, that does it for out-of-sync regions. It is
> not critical, so it can wait for 2.6.34.

Looks good to me.

Reviewed-by: Takahiro Yasui <tyasui@redhat.com>

Thanks,
Taka

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 09:21 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org