Linux Device Mapper development
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Mikulas Patocka <mpatocka@redhat.com>,
	Richard Henderson <rth@twiddle.net>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Matt Turner <mattst88@gmail.com>,
	Mike Snitzer <msnitzer@redhat.com>
Cc: linux-block@vger.kernel.org, dm-devel@redhat.com,
	linux-alpha@vger.kernel.org
Subject: Re: [PATCH] block: use 32-bit blk_status_t on Alpha
Date: Wed, 21 Mar 2018 10:50:50 -0600	[thread overview]
Message-ID: <502b7c1e-a95c-ff8d-64cd-9cbdd275fa1d@kernel.dk> (raw)
In-Reply-To: <alpine.LRH.2.02.1803211239490.23314@file01.intranet.prod.int.rdu2.redhat.com>

On 3/21/18 10:42 AM, Mikulas Patocka wrote:
> Early alpha processors cannot write a single byte or word; they read 8
> bytes, modify the value in registers and write back 8 bytes.
> 
> The type blk_status_t is defined as one byte, it is often written
> asynchronously by I/O completion routines, this asynchronous modification
> can corrupt content of nearby bytes if these nearby bytes can be written
> simultaneously by another CPU.
> 
> - one example of such corruption is the structure dm_io where
>   "blk_status_t status" is written by an asynchronous completion routine
>   and "atomic_t io_count" is modified synchronously
> - another example is the structure dm_buffer where "unsigned hold_count"
>   is modified synchronously from process context and "blk_status_t
>   write_error" is modified asynchronously from bio completion routine
> 
> This patch fixes the bug by changing the type blk_status_t to 32 bits if
> we are on Alpha and if we are compiling for a processor that doesn't have
> the byte-word-extension.

That's nasty. Is alpha the only problematic arch here?

As to the patch in question, normally I'd just say we should make it
unconditionally u32. But we pack so nicely in the bio, and I don't think
the bio itself has this issue as the rest of the members that share this
word are all set before the bio is submitted. But callers embedding
the status var in other structures don't necessarily have that
guarantee, as your dm examples show.

-- 
Jens Axboe

  reply	other threads:[~2018-03-21 16:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21 16:42 [PATCH] block: use 32-bit blk_status_t on Alpha Mikulas Patocka
2018-03-21 16:50 ` Jens Axboe [this message]
2018-03-21 17:00   ` Mikulas Patocka
2018-03-21 17:02     ` Jens Axboe

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=502b7c1e-a95c-ff8d-64cd-9cbdd275fa1d@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=ink@jurassic.park.msu.ru \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=mattst88@gmail.com \
    --cc=mpatocka@redhat.com \
    --cc=msnitzer@redhat.com \
    --cc=rth@twiddle.net \
    /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