public inbox for linux-bcache@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox