All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Priebe - Profihost AG <s.priebe-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
To: Kent Overstreet <kmo-PEzghdH756F8UrSeD/g0lQ@public.gmane.org>
Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: kernel 3.10 + applied bcache patches unable to handle kernel NULL pointer dereference
Date: Fri, 12 Jul 2013 10:44:44 +0200	[thread overview]
Message-ID: <51DFC1FC.6000002@profihost.ag> (raw)
In-Reply-To: <51DFC16D.60607-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>

This one was the working commit for 3.10:

From 023d8db36affa2489a83052dbb7c969d0941f06f Mon Sep 17 00:00:00 2001
From: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Date: Wed, 26 Jun 2013 17:25:38 -0700
Subject: [PATCH 17/17] bcache: FUA fixes

Journal writes need to be marked FUA, not just REQ_FLUSH. And btree node
writes have... weird ordering requirements.

Signed-off-by: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 drivers/md/bcache/btree.c   |   19 +++++++++++++++++--
 drivers/md/bcache/journal.c |    2 +-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 09fb8a2..803af59 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -328,10 +328,25 @@ static void do_btree_node_write(struct btree *b)

        b->bio->bi_end_io       = btree_node_write_endio;
        b->bio->bi_private      = &b->io.cl;
-       b->bio->bi_rw   = REQ_META|WRITE_SYNC;
-       b->bio->bi_size = set_blocks(i, b->c) * block_bytes(b->c);
+       b->bio->bi_rw           = REQ_META|WRITE_SYNC|REQ_FUA;
+       b->bio->bi_size         = set_blocks(i, b->c) * block_bytes(b->c);
        bch_bio_map(b->bio, i);

+       /*
+        * If we're appending to a leaf node, we don't technically need
FUA -
+        * this write just needs to be persisted before the next journal
write,
+        * which will be marked FLUSH|FUA.
+        *
+        * Similarly if we're writing a new btree root - the pointer is
going to
+        * be in the next journal entry.
+        *
+        * But if we're writing a new btree node (that isn't a root) or
+        * appending to a non leaf btree node, we need either FUA or a flush
+        * when we write the parent with the new pointer. FUA is cheaper
than a
+        * flush, and writes appending to leaf nodes aren't blocking
anything so
+        * just make all btree node writes FUA to keep things sane.
+        */
+
        bkey_copy(&k.key, &b->key);
        SET_PTR_OFFSET(&k.key, 0, PTR_OFFSET(&k.key, 0) + bset_offset(b,
i));

diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 5ca2214..4b25066 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -620,7 +620,7 @@ static void journal_write_unlocked(struct closure *cl)
                bio_reset(bio);
                bio->bi_sector  = PTR_OFFSET(k, i);
                bio->bi_bdev    = ca->bdev;
-               bio->bi_rw      = REQ_WRITE|REQ_SYNC|REQ_META|REQ_FLUSH;
+               bio->bi_rw      =
REQ_WRITE|REQ_SYNC|REQ_META|REQ_FLUSH|REQ_FUA;
                bio->bi_size    = sectors << 9;

                bio->bi_end_io  = journal_write_endio;
-- 
1.7.10.4

Stefan

Am 12.07.2013 10:42, schrieb Stefan Priebe - Profihost AG:
> it's this one:
> -               bch_writeback_add(dc, bio_sectors(bio));
> +               bch_writeback_add(dc);
> 
> from cache: FUA fixes
> 
> Journal writes need to be marked FUA, not just REQ_FLUSH. And btree node
> writes have... weird ordering requirements.
> 
> in 3.10 bch_writeback_add still needs to arguments.
> 
> Stefan
> 
> Am 12.07.2013 10:38, schrieb Stefan Priebe - Profihost AG:
>> Does not seem to build:
>>   CC [M]  drivers/md/bcache/request.o
>> drivers/md/bcache/request.c: In function ‘request_write’:
>> drivers/md/bcache/request.c:1057: error: too few arguments to function
>> ‘bch_writeback_add’
>> make[3]: *** [drivers/md/bcache/request.o] Error 1
>> make[2]: *** [drivers/md/bcache] Error 2
>>
>> Am 12.07.2013 10:31, schrieb Stefan Priebe - Profihost AG:
>>> Am 12.07.2013 10:19, schrieb Kent Overstreet:
>>>> On Fri, Jul 12, 2013 at 08:11:45AM +0200, Stefan Priebe - Profihost AG wrote:
>>>>> Am 12.07.2013 03:40, schrieb Kent Overstreet:
>>>>>> On Thu, Jul 04, 2013 at 01:45:42PM +0200, Stefan Priebe - Profihost AG wrote:
>>>>>>> fix is this one: http://pastebin.com/raw.php?i=LBFJRvEt
>>>>>>
>>>>>> Oh I see, the patch that introduced this is an old version of the patch
>>>>>> - the bcache-for-3.11 had this fixed, and I forgot to update that
>>>>>> branch. Thanks, I'll apply it there.
>>>>>
>>>>> thanks - which commit id is the fix in? i can't find it.
>>>>
>>>> The patch that introduced it wasn't ever destined for 3.10 anyways, and
>>>> I just deleted the bcache/bcache-3.9 branches because they'd gotten out
>>>> of sync and weren't getting updated with bcache-dev/bcache-for-3.11.
>>>
>>> Thanks!
>>>
>>>> So just stick with bcache-for-3.10, I just backported all the important
>>>> bugfixes to it - I'll be sending that stuff to Greg K-H for 3.10.1 in
>>>> the next couple days too.
>>> Thanks!
>>>
>>> Stefan
>>>

      parent reply	other threads:[~2013-07-12  8:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-04  9:39 kernel 3.10 + applied bcache patches unable to handle kernel NULL pointer dereference Stefan Priebe - Profihost AG
     [not found] ` <51D542EB.7070404-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-04 11:45   ` Stefan Priebe - Profihost AG
     [not found]     ` <51D56066.80106-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-12  1:40       ` Kent Overstreet
2013-07-12  6:11         ` Stefan Priebe - Profihost AG
     [not found]           ` <51DF9E21.3060601-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-12  8:19             ` Kent Overstreet
2013-07-12  8:31               ` Stefan Priebe - Profihost AG
     [not found]                 ` <51DFBEEF.3020106-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-12  8:38                   ` Stefan Priebe - Profihost AG
     [not found]                     ` <51DFC087.4080402-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-12  8:42                       ` Stefan Priebe - Profihost AG
     [not found]                         ` <51DFC16D.60607-2Lf/h1ldwEHR5kwTpVNS9A@public.gmane.org>
2013-07-12  8:44                           ` Stefan Priebe - Profihost AG [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=51DFC1FC.6000002@profihost.ag \
    --to=s.priebe-2lf/h1ldwehr5kwtpvns9a@public.gmane.org \
    --cc=kmo-PEzghdH756F8UrSeD/g0lQ@public.gmane.org \
    --cc=linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.