bug in bio_map_user_iov

--_003_4A89C5156A744C0B9A818FAB6C36AFF0riverbedcom_
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

Hello all,

I've been doing some work with a Linux kernel module that enables zero-copy=
I/O to block devices using a custom user/kernel interface. In porting this=
from an older Linux kernel to a more modern release, I ran into an issue w=
hen interacting with MD devices that I traced back to what I believe to be =
a problem in bio_map_user_iov.

The problem and fix are described in the first attached patch. The second i=
s a simple test module and user program which triggers the bug and validate=
s the fix.

Thanks,
-m

ps. This is my first attempt at pushing a patch upstream so please forgive =
any newbie mistakes.


--_003_4A89C5156A744C0B9A818FAB6C36AFF0riverbedcom_
Content-Type: application/octet-stream;
name="0001-kernel-fix-bug-in-bio_map_user_iov-on-MD-devices. patch"
Content-Description: 0001-kernel-fix-bug-in-bio_map_user_iov-on-MD-devices.patch
Content-Disposition: attachment;
filename="0001-kernel-fix-bug-in-bio_map_user_iov-on-MD-devi ces.patch";
size=1462; creation-date="Wed, 17 Nov 2010 13:24:48 GMT";
modification-date="Wed, 17 Nov 2010 13:24:48 GMT"
Content-Transfer-Encoding: base64

RnJvbSAzYmZiYzhlZDQxYzkyMzJlOGZhMGQ1NWU0MDkzOTM1ZDBhZjMyYTc1 IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNaWNoYWVsIERlbW1lciA8ZGVtbWVyQHJp dmVyYmVkLmNvbT4K
RGF0ZTogV2VkLCAxNyBOb3YgMjAxMCAxMToyMjo0NCAtMDgwMApTdWJqZWN0 OiBbUEFUQ0ggMS8y
XSBrZXJuZWw6IGZpeCBidWcgaW4gYmlvX21hcF91c2VyX2lvdiBvbiBNRCBk ZXZpY2VzCgpXaGls
ZSBidWlsZGluZyB1cCB0aGUgaW92ZWMgdG8gcGFzcyB0byB0aGUgbG93ZXIg bGV2ZWwgZGV2aWNl
LApfX2Jpb19tYXBfdXNlcl9pb3YgY2FsbHMgX19iaW9fYWRkX3BhZ2Ugd2hp Y2ggdHJpZXMgdG8g
bWVyZ2UKY29uc2VjdXRpdmUgYmxvY2sgcG9pbnRlcnMgcmF0aGVyIHRoYW4g c3BsaXR0aW5nIHRo
ZW0gaW50bwptdWx0aXBsZSBpb3YgZW50cmllcywgZ2l2aW5nIHRoZSB1bmRl cmx5aW5nIGRyaXZl
ciBhbiBvcHBvcnR1bml0eQp0byByZXN0cmljdCB0aGVzZSBtZXJnZXMgdGhy b3VnaCB0aGUgbWVy
Z2VfYnZlY19mbi4KCkZvciBNRCBkZXZpY2VzLCB0aGUgbWVyZ2UgZnVuY3Rp b24gbG9va3MgdGhy
b3VnaCB0aGUgYmlvIGF0IHRoZQpiaV9iZGV2LCBob3dldmVyIGJpb19tYXBf dXNlcl9pb3YgZG9l
c24ndCBhc3NpZ24gdGhlIGJsb2NrX2RldmljZQpwb2ludGVyIHRvIHRoZSBi aW8gdW50aWwgYWZ0
ZXIgYWxsIHRoZSBtZXJnaW5nIGlzIGRvbmUuIFRoaXMgcGF0Y2gKc2ltcGx5 IHJlb3JkZXJzIHRo
ZSBhc3NpZ25tZW50IHNvIHRoYXQgYmlvIG9wZXJhdGlvbnMgb24gTUQgZGV2 aWNlcwpkb24ndCBj
cmFzaCBvbiBhIG51bGwgcG9pbnRlci4KLS0tCiBmcy9iaW8uYyB8ICAgIDMg KystCiAxIGZpbGVz
IGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkKCmRp ZmYgLS1naXQgYS9m
cy9iaW8uYyBiL2ZzL2Jpby5jCmluZGV4IDRiZDQ1NGYuLjdmMjk1ODIgMTAw NjQ0Ci0tLSBhL2Zz
L2Jpby5jCisrKyBiL2ZzL2Jpby5jCkBAIC05ODksNiArOTg5LDggQEAgc3Rh dGljIHN0cnVjdCBi
aW8gKl9fYmlvX21hcF91c2VyX2lvdihzdHJ1Y3QgcmVxdWVzdF9xdWV1ZSAq cSwKIAlpZiAoIWJp
bykKIAkJcmV0dXJuIEVSUl9QVFIoLUVOT01FTSk7CiAKKwliaW8tPmJpX2Jk ZXYgPSBiZGV2Owor
CiAJcmV0ID0gLUVOT01FTTsKIAlwYWdlcyA9IGtjYWxsb2MobnJfcGFnZXMs IHNpemVvZihzdHJ1
Y3QgcGFnZSAqKSwgZ2ZwX21hc2spOwogCWlmICghcGFnZXMpCkBAIC0xMDQ2 LDcgKzEwNDgsNiBA
QCBzdGF0aWMgc3RydWN0IGJpbyAqX19iaW9fbWFwX3VzZXJfaW92KHN0cnVj dCByZXF1ZXN0X3F1
ZXVlICpxLAogCWlmICghd3JpdGVfdG9fdm0pCiAJCWJpby0+YmlfcncgfD0g UkVRX1dSSVRFOwog
Ci0JYmlvLT5iaV9iZGV2ID0gYmRldjsKIAliaW8tPmJpX2ZsYWdzIHw9ICgx IDw8IEJJT19VU0VS
X01BUFBFRCk7CiAJcmV0dXJuIGJpbzsKIAotLSAKMS43LjEKCg==

--_003_4A89C5156A744C0B9A818FAB6C36AFF0riverbedcom_
Content-Type: application/octet-stream;
name="0002-Test-program-to-verify-the-bio_map_user-bug-fix-f or-.patch"
Content-Description: 0002-Test-program-to-verify-the-bio_map_user-bug-fix-for-.pa tch
Content-Disposition: attachment;
filename="0002-Test-program-to-verify-the-bio_map_user-bug-f ix-for-.patch";
size=4399; creation-date="Wed, 17 Nov 2010 13:24:48 GMT";
modification-date="Wed, 17 Nov 2010 13:24:48 GMT"
Content-Transfer-Encoding: base64

RnJvbSA1NjNkOTI1ZmE5M2EwNGQxMmQxYWQ4NjliMzA2MTkzMTk4ZjY4OGMx IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNaWNoYWVsIERlbW1lciA8ZGVtbWVyQHJp dmVyYmVkLmNvbT4K
RGF0ZTogV2VkLCAxNyBOb3YgMjAxMCAxMToxMTowOSAtMDgwMApTdWJqZWN0 OiBbUEFUQ0ggMi8y
XSBUZXN0IHByb2dyYW0gdG8gdmVyaWZ5IHRoZSBiaW9fbWFwX3VzZXIgYnVn IGZpeCBmb3IgbWQg
ZGV2aWNlcy4KClVzYWdlOgpjZCBiaW9fbWFwX3VzZXJfYnVnCmluc21vZCBi aW9fYnVnLmtvCm1r
bm9kIC9kZXYvYmlvX2J1ZyBjIDk5OSAxCi4vYmlvX2J1Z190ZXN0IC9kZXYv bWRYCi0tLQogYmlv
X21hcF91c2VyX2J1Zy9NYWtlZmlsZSAgICAgICB8ICAgMTYgKysrKysrKysK IGJpb19tYXBfdXNl
cl9idWcvYmlvX2J1Zy5oICAgICAgfCAgICA1ICsrKwogYmlvX21hcF91c2Vy X2J1Zy9iaW9fYnVn
X21vZC5jICB8ICAgNzQgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrCiBi
aW9fbWFwX3VzZXJfYnVnL2Jpb19idWdfdGVzdC5jIHwgICA0NCArKysrKysr KysrKysrKysrKysr
KysrKwogNCBmaWxlcyBjaGFuZ2VkLCAxMzkgaW5zZXJ0aW9ucygrKSwgMCBk ZWxldGlvbnMoLSkK
IGNyZWF0ZSBtb2RlIDEwMDY0NCBiaW9fbWFwX3VzZXJfYnVnL01ha2VmaWxl CiBjcmVhdGUgbW9k
ZSAxMDA2NDQgYmlvX21hcF91c2VyX2J1Zy9iaW9fYnVnLmgKIGNyZWF0ZSBt b2RlIDEwMDY0NCBi
aW9fbWFwX3VzZXJfYnVnL2Jpb19idWdfbW9kLmMKIGNyZWF0ZSBtb2RlIDEw MDY0NCBiaW9fbWFw
X3VzZXJfYnVnL2Jpb19idWdfdGVzdC5jCgpkaWZmIC0tZ2l0IGEvYmlvX21h cF91c2VyX2J1Zy9N
YWtlZmlsZSBiL2Jpb19tYXBfdXNlcl9idWcvTWFrZWZpbGUKbmV3IGZpbGUg bW9kZSAxMDA2NDQK
aW5kZXggMDAwMDAwMC4uODA3NTZjNQotLS0gL2Rldi9udWxsCisrKyBiL2Jp b19tYXBfdXNlcl9i
dWcvTWFrZWZpbGUKQEAgLTAsMCArMSwxNiBAQAorCitvYmotbTo9IGJpb19i dWcubworCitiaW9f
YnVnLW9ianMgOj0gYmlvX2J1Z19tb2QubworCithbGw6IGJpb19idWcua28g YmlvX2J1Z190ZXN0
CisKK2Jpb19idWcua286CisJbWFrZSAtQyAuLiBNPWBwd2RgIG1vZHVsZXMK KworYmlvX2J1Z190
ZXN0OiBiaW9fYnVnX3Rlc3QuYworCSQoQ0MpIC1XYWxsIC1nICQ8IC1vICRA CisKK2NsZWFuOgor
CW1ha2UgLUMgLi4gTT1gcHdkYCBjbGVhbgorCXJtIC1mIGJpb19idWdfdGVz dApkaWZmIC0tZ2l0
IGEvYmlvX21hcF91c2VyX2J1Zy9iaW9fYnVnLmggYi9iaW9fbWFwX3VzZXJf YnVnL2Jpb19idWcu
aApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi45N2FhMGI5 Ci0tLSAvZGV2L251
bGwKKysrIGIvYmlvX21hcF91c2VyX2J1Zy9iaW9fYnVnLmgKQEAgLTAsMCAr MSw1IEBACisKK3N0
cnVjdCBiaW9fbWFwX3VzZXJfYnVnX2FyZ3MgeworCWNoYXIgZGV2bmFtZVs2 NF07CisJdW5zaWdu
ZWQgbG9uZyB1c2VyYWRkcjsKK307CmRpZmYgLS1naXQgYS9iaW9fbWFwX3Vz ZXJfYnVnL2Jpb19i
dWdfbW9kLmMgYi9iaW9fbWFwX3VzZXJfYnVnL2Jpb19idWdfbW9kLmMKbmV3 IGZpbGUgbW9kZSAx
MDA2NDQKaW5kZXggMDAwMDAwMC4uZDAxYmY1YgotLS0gL2Rldi9udWxsCisr KyBiL2Jpb19tYXBf
dXNlcl9idWcvYmlvX2J1Z19tb2QuYwpAQCAtMCwwICsxLDc0IEBACisjaW5j bHVkZSA8bGludXgv
aW5pdC5oPgorI2luY2x1ZGUgPGxpbnV4L21vZHVsZS5oPgorI2luY2x1ZGUg PGxpbnV4L3BvbGwu
aD4KKyNpbmNsdWRlIDxsaW51eC9ibGtkZXYuaD4KKyNpbmNsdWRlIDxsaW51 eC92ZXJzaW9uLmg+
CisjaW5jbHVkZSA8bGludXgvZXJyLmg+CisKKyNpbmNsdWRlICJiaW9fYnVn LmgiCisKK3N0YXRp
YyBsb25nCitidWdfaW9jdGwoc3RydWN0IGZpbGUgKmZpbGUsIHVuc2lnbmVk IGludCBjLCB1bnNp
Z25lZCBsb25nIHYpCit7CisgICAgc3RydWN0IGJpb19tYXBfdXNlcl9idWdf YXJncyBhcmdzOwor
ICAgIHN0cnVjdCBibG9ja19kZXZpY2UgKmJkZXY7CisgICAgc3RydWN0IHJl cXVlc3RfcXVldWUg
KnE7CisgICAgc3RydWN0IGJpbyAqYmlvOworCisgICAgaWYgKGNvcHlfZnJv bV91c2VyKCZhcmdz
LCAodm9pZCopdiwgc2l6ZW9mKGFyZ3MpKSkgeworICAgICAgICByZXR1cm4g LUVGQVVMVDsKKyAg
ICB9CisKKyAgICBwcmludGsoInRlc3RpbmcgYmlvX21hcF91c2VyIGJ1ZyBv biAlcy4uLlxuIiwg
YXJncy5kZXZuYW1lKTsKKworICAgIGJkZXYgPSBvcGVuX2JkZXZfZXhjbHVz aXZlKGFyZ3MuZGV2
bmFtZSwgRk1PREVfUkVBRHxGTU9ERV9XUklURSwgKHZvaWQqKTB4MDEwMTAx MDEpOworICAgIGlm
IChJU19FUlIoYmRldikpIHsKKwlwcmludGsoImVycm9yIG9wZW5pbmcgYmxv Y2sgZGV2aWNlXG4i
KTsKKwlyZXR1cm4gLUVJTlZBTDsKKyAgICB9CQorCisgICAgcSA9IGJkZXZf Z2V0X3F1ZXVlKGJk
ZXYpOworICAgIGlmIChxID09IE5VTEwpIHsKKyAgICAgICAgcHJpbnRrKCJi bG9jayBkZXZpY2Ug
aGFzIHF1ZXVlIG1pc3NpbmcuXG4iKTsKKyAgICAgICAgcmV0dXJuIC1FRkFV TFQ7CisgICAgfQor
ICAgIAorICAgIC8vIHdpbGwgY3Jhc2ggaGVyZQorICAgIGJpbyA9IGJpb19t YXBfdXNlcihxLCBi
ZGV2LCBhcmdzLnVzZXJhZGRyLCA0MDk2LCAwLCBHRlBfS0VSTkVMKTsKKwor ICAgIGNsb3NlX2Jk
ZXZfZXhjbHVzaXZlKGJkZXYsIEZNT0RFX1JFQUR8Rk1PREVfV1JJVEUpOwor ICAgIAorICAgIHBy
aW50aygidGVzdGluZyBiaW9fbWFwX3VzZXIgYnVnIG9uICVzLi4uIG9rXG4i LCBhcmdzLmRldm5h
bWUpOworICAgIHJldHVybiAwOworfQorCitzdGF0aWMgc3RydWN0IGZpbGVf b3BlcmF0aW9ucyBi
aW9fYnVnX2ZvcHMgPSB7CisgICAgb3duZXI6IFRISVNfTU9EVUxFLAorICAg IHVubG9ja2VkX2lv
Y3RsOiBidWdfaW9jdGwKK307CisKK3N0YXRpYyBpbnQgX19pbml0IAorYnVn X2luaXQodm9pZCkg
Cit7ICAgIAorICAgIHByaW50aygicmVnaXN0ZXJpbmcgZGV2aWNlXG4iKTsK KworICAgIGlmIChy
ZWdpc3Rlcl9jaHJkZXYoOTk5LCAiYmlvX2J1ZyIsICZiaW9fYnVnX2ZvcHMp KSB7CisgICAgICAg
IHByaW50aygiRVJST1IgcmVnaXN0ZXJfY2hyZGV2IGZvciAldSBpbiAvZGV2 LyVzLlxuIiwKKyAg
ICAgICAgICAgICAgIDk5OSwgImJpb19idWciKTsKKyAgICB9CisKKyAgICBy ZXR1cm4gMDsKK30K
Kworc3RhdGljIHZvaWQgX19leGl0IAorYnVnX2V4aXQodm9pZCkgCit7Cisg ICAgcHJpbnRrKCJ1
bnJlZ2lzdGVyaW5nIGRldmljZVxuIik7CisgICAgdW5yZWdpc3Rlcl9jaHJk ZXYoOTk5LCAiYmlv
X2J1ZyIpOworfQorCisKK21vZHVsZV9pbml0KGJ1Z19pbml0KTsKK21vZHVs ZV9leGl0KGJ1Z19l
eGl0KTsKKworTU9EVUxFX0xJQ0VOU0UoIkdQTCIpOwpkaWZmIC0tZ2l0IGEv YmlvX21hcF91c2Vy
X2J1Zy9iaW9fYnVnX3Rlc3QuYyBiL2Jpb19tYXBfdXNlcl9idWcvYmlvX2J1 Z190ZXN0LmMKbmV3
IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uMWY5N2I3MQotLS0g L2Rldi9udWxsCisr
KyBiL2Jpb19tYXBfdXNlcl9idWcvYmlvX2J1Z190ZXN0LmMKQEAgLTAsMCAr MSw0NCBAQAorCisj
aW5jbHVkZSA8c3RkaW8uaD4KKyNpbmNsdWRlIDxlcnJuby5oPgorI2luY2x1 ZGUgPGZjbnRsLmg+
CisjaW5jbHVkZSA8bWFsbG9jLmg+CisjaW5jbHVkZSA8c3RyaW5nLmg+Cisj aW5jbHVkZSA8c3lz
L2lvY3RsLmg+CisKKyNpbmNsdWRlICJiaW9fYnVnLmgiCisKK2ludCBtYWlu KGludCBhcmdjLCBj
aGFyKiBhcmd2W10pIHsKKwlpbnQgZmQ7CisJc3RydWN0IGJpb19tYXBfdXNl cl9idWdfYXJncyBh
cmdzOworCXZvaWQqIHA7CisKKwlpZiAoYXJnYyA8IDIpIHsKKwkJcHJpbnRm KCJ1c2FnZTogJXMg
PGRldm5hbWU+XG4iLCBhcmd2WzBdKTsKKwkJcmV0dXJuIDE7CisJfQkKKwor CWZkID0gb3Blbigi
L2Rldi9iaW9fYnVnIiwgT19SRFdSKTsKKwlpZiAoZmQgPCAwKSB7CisJCXBy aW50ZigiZXJyb3Ig
b3BlbmluZyBkZXZpY2U6ICVzXG4iLCBzdHJlcnJvcihlcnJubykpOworCQly ZXR1cm4gMTsKKwl9
CisKKwlwID0gbWVtYWxpZ24oNDA5NiwgNDA5Nik7CisJaWYgKHAgPT0gMCkg eworCQlwcmludGYo
ImVycm9yIGluIG1lbWFsaWduOiAlc1xuIiwgc3RyZXJyb3IoZXJybm8pKTsK KwkJcmV0dXJuIDE7
CisJfQorCisJc3RybmNweShhcmdzLmRldm5hbWUsIGFyZ3ZbMV0sIHNpemVv ZihhcmdzLmRldm5h
bWUpKTsKKwlhcmdzLnVzZXJhZGRyID0gKHVuc2lnbmVkIGxvbmcpcDsKKwor CWlmIChpb2N0bChm
ZCwgOTk5LCAmYXJncykgIT0gMCkgeworCQlwcmludGYoImVycm9yIGluIGlv Y3RsOiAlc1xuIiwg
c3RyZXJyb3IoZXJybm8pKTsKKwkJcmV0dXJuIDE7CisJfQorCisJcmV0dXJu IDA7Cit9CisKKwot
LSAKMS43LjEKCg==

--_003_4A89C5156A744C0B9A818FAB6C36AFF0riverbedcom_--
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: bug in bio_map_user_iov

On Wed, 17 Nov 2010 13:24:47 -0800
Michael Demmer wrote:

> Hello all,
>
> I've been doing some work with a Linux kernel module that enables zero-copy I/O to block devices using a custom user/kernel interface. In porting this from an older Linux kernel to a more modern release, I ran into an issue when interacting with MD devices that I traced back to what I believe to be a problem in bio_map_user_iov.
>
> The problem and fix are described in the first attached patch. The second is a simple test module and user program which triggers the bug and validates the fix.
>
> Thanks,
> -m
>
> ps. This is my first attempt at pushing a patch upstream so please forgive any newbie mistakes.
>

I think the real issue here is that bio_map_user is an interface that was
only intended to be used by bottom level devices like SCSI drivers etc.
It is a function that a device driver can use if it knows that it makes sense
to use it.

You are trying to use it as a generic interface that works for all block
devices, and it wasn't intended for that.

So while it is reasonably simple to 'fix' bio_map_user_iov, it is not
possible to 'fix' bio_map_kern_iov in the same way, because it doesn't have
access to the bdev at all.


So the question we should be asking is: are you really using the right
interface for the job? Is bio_map_user something that you really should be
using?
And to answer that, we would need to know what you are trying to do.

And why isn't O_DIRECT a suitable zero-copy interface for I/O to block
devices?

NeilBrown

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: bug in bio_map_user_iov

>> Hello all,
>>
>> I've been doing some work with a Linux kernel module that enables zero-copy I/O to block devices using a custom user/kernel interface. In porting this from an older Linux kernel to a more modern release, I ran into an issue when interacting with MD devices that I traced back to what I believe to be a problem in bio_map_user_iov.
>>
>> The problem and fix are described in the first attached patch. The second is a simple test module and user program which triggers the bug and validates the fix.
>>
>> Thanks,
>> -m
>>
>> ps. This is my first attempt at pushing a patch upstream so please forgive any newbie mistakes.
>>
>
> I think the real issue here is that bio_map_user is an interface that was
> only intended to be used by bottom level devices like SCSI drivers etc.
> It is a function that a device driver can use if it knows that it makes sense
> to use it.
>
> You are trying to use it as a generic interface that works for all block
> devices, and it wasn't intended for that.
>
> So while it is reasonably simple to 'fix' bio_map_user_iov, it is not
> possible to 'fix' bio_map_kern_iov in the same way, because it doesn't have
> access to the bdev at all.
>
>
> So the question we should be asking is: are you really using the right
> interface for the job? Is bio_map_user something that you really should be
> using?
> And to answer that, we would need to know what you are trying to do.
>
> And why isn't O_DIRECT a suitable zero-copy interface for I/O to block
> devices?

Thanks -- I understand your concerns, but unfortunately I can't really go into more detail about our use case since it's something we use internally. Given which, I understand your potential reluctance to take the patch.

Best,
-m



--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: bug in bio_map_user_iov

>>>>> "Michael" == Michael Demmer writes:

Michael> Thanks -- I understand your concerns, but unfortunately I
Michael> can't really go into more detail about our use case since
Michael> it's something we use internally. Given which, I understand
Michael> your potential reluctance to take the patch.

Duh, they're an IP accelerator company, so of course the want to
maximize speed from their RAID array on the appliance to the various
ethernet interfaces.

John
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html