mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mikulas Patocka <mpatocka@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: andi@firstfloor.org, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, jens.axboe@oracle.com
Subject: Re: [SUGGESTION]: drop virtual merge accounting in I/O requests
Date: Mon, 14 Jul 2008 08:16:13 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0807140805380.10106@devserv.devel.redhat.com> (raw)
In-Reply-To: <20080713.174855.234796356.davem@davemloft.net>

On Sun, 13 Jul 2008, David Miller wrote:

> From: Mikulas Patocka <mpatocka@redhat.com>
> Date: Sun, 13 Jul 2008 19:53:01 -0400 (EDT)
>
>> There is no need to turn BIO merge off --- the problem is caused by
>> accounting of BIO merging in block layer, not by BIO merging itself.
>>
>> Just do #define BIO_VMERGE_BOUNDARY 0, and that disables the accounting,
>> but leaves merging as it is.
>
> For the thousanth time, the BIO_VMERGE_BOUNDARY code is useful
> and worked perfectly fine before segment boundary handling was
> added to the block layer.
>
> It's a regression, and as such should be fixed or the guilty
> code reverted.
>
> Since when do we say "sorry that got broken, turn it off, thanks"
> ?

And which was the supposed "working" kernel version?

In current kernel there are three conditions that can cause merge failure 
(and possible crash as a result):
1. dma_addr != dma_next --- skipping over an allocated entry
2. outs->dma_length + s->length > max_seg_size --- max segment size, this 
is what I was hitting
3. is_span_boundary(out_entry, base_shift, seg_boundary_size, outs, s) --- 
a devices that have special dma_get_seg_boundary(dev)

So, show me a sparc64 kernel where none of these conditions existed and 
where merge-accounting was bug-free.

I suppose that before condition (2) was added, the conditions (1) and (3) 
still existed, making crashes still possible, although not as common as 
with condition (2). But if you want to show me otherwise --- a bug-free 
implementation of merge accounting --- just do it.

> For the thousanth time, the BIO_VMERGE_BOUNDARY code is useful

Where? BIO_VMERGE_BOUNDARY has nothing to do with actual merging (the 
merging happens with or without it BIO_VMERGE_BOUNDARY).

As you mentioned ESP driver, it declares .sg_tablesize = SG_ALL, so 
BIO_VMERGE_BOUNDARY has no effect on the operation of this driver. Any 
other driver where BIO_VMERGE_BOUNDARY does matter?

Mikulas

  reply	other threads:[~2008-07-14 12:16 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-10 21:56 Mikulas Patocka
2008-07-10 22:59 ` Julian Calaby
2008-07-10 23:57   ` Mikulas Patocka
2008-07-11  6:20 ` FUJITA Tomonori
2008-07-11 10:52   ` Mikulas Patocka
2008-07-11 11:15     ` FUJITA Tomonori
2008-07-11 19:41       ` David Miller
2008-07-11 20:22         ` Mikulas Patocka
2008-07-12 12:30 ` Andi Kleen
2008-07-13 13:34   ` Mikulas Patocka
2008-07-13 13:50     ` Andi Kleen
2008-07-13 19:46       ` David Miller
2008-07-13 20:13         ` Andi Kleen
2008-07-13 23:53           ` Mikulas Patocka
2008-07-14  0:48             ` David Miller
2008-07-14 12:16               ` Mikulas Patocka [this message]
2008-07-14 12:28                 ` David Miller
2008-07-14 14:03                   ` Mikulas Patocka
2008-07-14 21:37                     ` David Miller
2008-07-14 23:16                       ` Mikulas Patocka
2008-07-15  1:31                         ` David Miller
2008-07-15 22:32                           ` Mikulas Patocka
2008-07-15 22:37                             ` David Miller
2008-07-15 22:59                               ` Mikulas Patocka
2008-07-15  2:40                     ` FUJITA Tomonori
2008-07-14  0:41           ` David Miller
2008-07-14  2:19             ` FUJITA Tomonori
2008-07-14  3:20               ` David Miller
2008-07-14 17:45                 ` FUJITA Tomonori
2008-07-14 21:26                   ` Mikulas Patocka
2008-07-15  2:40                     ` FUJITA Tomonori
2008-07-15 12:09                       ` Mikulas Patocka
2008-07-15 12:15                         ` Andi Kleen
2008-07-15 13:16                           ` Mikulas Patocka
2008-07-15 14:06                             ` Andi Kleen
2008-07-15 12:19                         ` FUJITA Tomonori
2008-07-16  3:10                   ` David Miller
2008-07-16  4:38                     ` FUJITA Tomonori

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=Pine.LNX.4.64.0807140805380.10106@devserv.devel.redhat.com \
    --to=mpatocka@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=davem@davemloft.net \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sparclinux@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome