Skip to contentSkip to conversation

bdev_read_page

DISCUSSION Minchan Kim 2 participants 3 messages Nov 4, 2014 Memory management

lore
Share & tools
lore
#0 lore raw

Hello,

[1] b07b0aaf54ace05, zram: implement rw_page operation of zram

After I merged [1] and was testing zram, I got following warning.

[  179.987592] zram0: detected capacity change from 2147483648 to 0
[  179.987570] page:ffffea00008d6300 count:2 mapcount:0 mapping:ffff880025348e88 index:0x0
[  179.987570] flags: 0x100000000020002(error|mappedtodisk)
[  179.987570] page dumped because: VM_BUG_ON_PAGE(!PageLocked(page))
[  179.987570] ------------[ cut here ]------------
[  179.987570] kernel BUG at mm/filemap.c:747!
[  179.987570] invalid opcode: 0000 [#1] SMP 
[  179.987570] Dumping ftrace buffer:
[  179.987570]    (ftrace buffer empty)
[  179.987570] Modules linked in:
[  179.987570] CPU: 10 PID: 23080 Comm: udisks-part-id Not tainted 3.18.0-rc2+ #584
[  179.987570] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  179.987570] task: ffff88001de74200 ti: ffff88001b220000 task.ti: ffff88001b220000
[  179.987570] RIP: 0010:[<ffffffff8114ae12>]  [<ffffffff8114ae12>] unlock_page+0x82/0x90
[  179.987570] RSP: 0018:ffff88001b223998  EFLAGS: 00010246
[  179.987570] RAX: 0000000000000036 RBX: ffff88001e009a50 RCX: 000000000910090f
[  179.987570] RDX: 0000000000000910 RSI: 0000000000000001 RDI: ffffffff81600a7f
[  179.987570] RBP: ffff88001b223998 R08: 0000000000000001 R09: 0000000000000001
[  179.987570] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000001
[  179.987570] R13: ffff88001e0099c0 R14: 00000000fffffffb R15: ffff88001e0099c0
[  179.987570] FS:  00007fcb17c73800(0000) GS:ffff880027f40000(0000) knlGS:0000000000000000
[  179.987570] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  179.987570] CR2: 00007fff1629ebc8 CR3: 000000001dc4f000 CR4: 00000000000006e0
[  179.987570] Stack:
[  179.987570]  ffff88001b2239a8 ffffffff8114afde ffff88001b2239d8 ffffffff811f2e52
[  179.987570]  ffff88001e0099c0 00000000fffffffb 0000000000000000 ffff880025348e88
[  179.987570]  ffff88001b223a08 ffffffff812e9ad3 ffff88001b223a08 ffffffff8109b6b3
[  179.987570] Call Trace:
[  179.987570]  [<ffffffff8114afde>] page_endio+0x1e/0x80
[  179.987570]  [<ffffffff811f2e52>] mpage_end_io+0x42/0x60
[  179.987570]  [<ffffffff812e9ad3>] bio_endio+0x53/0xa0
[  179.987570]  [<ffffffff8109b6b3>] ? up_read+0x23/0x40
[  179.987570]  [<ffffffff81413f09>] zram_make_request+0x2a9/0x3c0
[  179.987570]  [<ffffffff812ee330>] generic_make_request+0xc0/0x100
[  179.987570]  [<ffffffff812ee3e5>] submit_bio+0x75/0x140
[  179.987570]  [<ffffffff812ee375>] ? submit_bio+0x5/0x140
[  179.987570]  [<ffffffff811f2e03>] mpage_bio_submit+0x33/0x40
[  179.987570]  [<ffffffff811f3de5>] mpage_readpages+0xf5/0x110
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff811ee190>] ? blkdev_write_begin+0x30/0x30
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff811ee1ad>] blkdev_readpages+0x1d/0x20
[  179.987570]  [<ffffffff81158ed4>] __do_page_cache_readahead+0x204/0x290
[  179.987570]  [<ffffffff81158d99>] ? __do_page_cache_readahead+0xc9/0x290
[  179.987570]  [<ffffffff8114b045>] ? find_get_entry+0x5/0x130
[  179.987570]  [<ffffffff811593cd>] force_page_cache_readahead+0x7d/0xb0
[  179.987570]  [<ffffffff81159443>] page_cache_sync_readahead+0x43/0x50
[  179.987570]  [<ffffffff8114d0e1>] generic_file_read_iter+0x451/0x650
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff811ed947>] blkdev_read_iter+0x37/0x40
[  179.987570]  [<ffffffff811b4358>] new_sync_read+0x78/0xb0
[  179.987570]  [<ffffffff811b555b>] vfs_read+0xab/0x180
[  179.987570]  [<ffffffff811b5682>] SyS_read+0x52/0xb0
[  179.987570]  [<ffffffff816011d2>] system_call_fastpath+0x12/0x17
[  179.987570] Code: 00 4c 8b 82 f8 00 00 00 31 d2 48 d3 ee 48 8d 3c f6 48 89 c6 49 8d 3c f8 e8 dc 9f f4 ff 5d c3 48 c7 c6 88 2c a0 81 e8 1e a8 02 00 <0f> 0b 66 66 66 2e 0f 1f 84 00 00 00 00 00 e8 bb 83 4b 00 55 48 
[  179.987570] RIP  [<ffffffff8114ae12>] unlock_page+0x82/0x90
[  179.987570]  RSP <ffff88001b223998>
[  179.987570] ---[ end trace 39c73c1d9da87ec4 ]---

[

The reason was that read I/O caused bdev_read_page could be failed by driver's internal problem so driver could set the page as PageError and then unlock the page by page_endio. However, do_mpage_readpage retry the I/O with BIO path without any locking/cleaning PG_error so if it fails again, we could encounter above warning.

Should we solve it with introducing page_endio_nolock which will not unlock the page in case of read-failure and use the function in rw_page functions? It relies on retrying logic of caller so it's ugly.

Another soulution is we can clean PG_error and locks the page again before the going bio path but I'm not sure it doesn't have any side-effect after releasing the lock. If it doesn't have any side-effect, it would be best.

The simplest solution I can think of is to bail out in case of fail of bdev_read_page without going on another route but you seem to be careful when I saw the comment of bdev_read_page.

"Errors returned by this function are usually "soft", eg out of memory, or
queue full; callers should try a different route to read this page rather than propagate an error back up the stack."

Actually, I'm not sure how much such fallback retrial really makes forward progress?

Anyway, I'd like to listen the opinions.

Thanks.

Signature · 8 lines
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw

SXQgd29ya3MgYmV0dGVyIHdoZW4geW91IGVtYWlsIG15IExpbnV4IGFjY291bnQgaW5zdGVhZCBv ZiBteSBFeGNoYW5nZSBhY2NvdW50Lg0KDQpJIHRoaW5rIHRoZSBwcm9ibGVtIGlzIHRoYXQgeW91 IHNldCB0aGUgcGFnZSBlcnJvciwgYW5kIGFsc28gcmV0dXJuIGZhaWx1cmUgZnJvbSBiZGV2X3Jl YWRfcGFnZSgpIHdoaWNoIGNhdXNlcyB0aGUgcGFnZSB0byBiZSBzdWJtaXR0ZWQgZm9yIEkvTyB0 d2ljZS4NCg0KQXNzdW1pbmcgdGhlIGVycm9yIGlzICdoYXJkJyAoaWUgcmVhZGluZyB0aGUgcGFn ZSBhZ2FpbiB3aWxsIG5vdCBzdWNjZWVkKSwgeW91IHNob3VsZCBzZXQgdGhlIHBhZ2UgZXJyb3Is IGFuZCByZXR1cm4gMCBmcm9tIHlvdXIgcndfcGFnZSBvcGVyYXRpb24sIGluZGljYXRpbmcgdGhh dCB0aGUgc3VibWlzc2lvbiB3YXMgc3VjY2Vzc2Z1bCBidXQgdGhlIHJlYWQgZmFpbGVkLiAgSWYg dGhlIHJlYWQgbWlnaHQgc3VjY2VlZCwgeW91IHNob3VsZCBub3QgY2FsbCBwYWdlX2VuZGlvKCks IGluc3RlYWQgcmV0dXJuaW5nIGFuIGVycm5vIGZyb20gcndfcGFnZSgpLg0KDQpJJ20gbm93IHdv bmRlcmluZyBpZiB0aGUgcmV0dXJuIGNvZGUgZnJvbSByd19wYWdlIHNob3VsZCBiZSBmbGlwcGVk IHRvIGJlIDEgb24gc3VjY2VzcyBhbmQgMCBmb3IgZmFpbHVyZSwgc28gd2UgZG9uJ3QgZ2V0IGRy aXZlciB3cml0ZXJzIHRyeWluZyB0byByZXR1cm4gZXJyb3IgY29kZXM/DQoNCi0tLS0tT3JpZ2lu YWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBNaW5jaGFuIEtpbSBbbWFpbHRvOm1pbmNoYW5Aa2VybmVs Lm9yZ10gDQpTZW50OiBNb25kYXksIE5vdmVtYmVyIDAzLCAyMDE0IDU6NDkgUE0NClRvOiBXaWxj b3gsIE1hdHRoZXcgUg0KQ2M6IERhdmUgQ2hpbm5lcjsgQW5kcmV3IE1vcnRvbjsga2FyYW0ubGVl OyBsaW51eC1tbUBrdmFjay5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmcNClN1Ympl Y3Q6IGJkZXZfcmVhZF9wYWdlDQoNCkhlbGxvLA0KDQpbMV0gYjA3YjBhYWY1NGFjZTA1LCB6cmFt OiBpbXBsZW1lbnQgcndfcGFnZSBvcGVyYXRpb24gb2YgenJhbQ0KDQpBZnRlciBJIG1lcmdlZCBb MV0gYW5kIHdhcyB0ZXN0aW5nIHpyYW0sIEkgZ290IGZvbGxvd2luZyB3YXJuaW5nLg0KDQpbICAx NzkuOTg3NTkyXSB6cmFtMDogZGV0ZWN0ZWQgY2FwYWNpdHkgY2hhbmdlIGZyb20gMjE0NzQ4MzY0 OCB0byAwDQpbICAxNzkuOTg3NTcwXSBwYWdlOmZmZmZlYTAwMDA4ZDYzMDAgY291bnQ6MiBtYXBj b3VudDowIG1hcHBpbmc6ZmZmZjg4MDAyNTM0OGU4OCBpbmRleDoweDANClsgIDE3OS45ODc1NzBd IGZsYWdzOiAweDEwMDAwMDAwMDAyMDAwMihlcnJvcnxtYXBwZWR0b2Rpc2spDQpbICAxNzkuOTg3 NTcwXSBwYWdlIGR1bXBlZCBiZWNhdXNlOiBWTV9CVUdfT05fUEFHRSghUGFnZUxvY2tlZChwYWdl KSkNClsgIDE3OS45ODc1NzBdIC0tLS0tLS0tLS0tLVsgY3V0IGhlcmUgXS0tLS0tLS0tLS0tLQ0K WyAgMTc5Ljk4NzU3MF0ga2VybmVsIEJVRyBhdCBtbS9maWxlbWFwLmM6NzQ3IQ0KWyAgMTc5Ljk4 NzU3MF0gaW52YWxpZCBvcGNvZGU6IDAwMDAgWyMxXSBTTVAgDQpbICAxNzkuOTg3NTcwXSBEdW1w aW5nIGZ0cmFjZSBidWZmZXI6DQpbICAxNzkuOTg3NTcwXSAgICAoZnRyYWNlIGJ1ZmZlciBlbXB0 eSkNClsgIDE3OS45ODc1NzBdIE1vZHVsZXMgbGlua2VkIGluOg0KWyAgMTc5Ljk4NzU3MF0gQ1BV OiAxMCBQSUQ6IDIzMDgwIENvbW06IHVkaXNrcy1wYXJ0LWlkIE5vdCB0YWludGVkIDMuMTguMC1y YzIrICM1ODQNClsgIDE3OS45ODc1NzBdIEhhcmR3YXJlIG5hbWU6IEJvY2hzIEJvY2hzLCBCSU9T IEJvY2hzIDAxLzAxLzIwMTENClsgIDE3OS45ODc1NzBdIHRhc2s6IGZmZmY4ODAwMWRlNzQyMDAg dGk6IGZmZmY4ODAwMWIyMjAwMDAgdGFzay50aTogZmZmZjg4MDAxYjIyMDAwMA0KWyAgMTc5Ljk4 NzU3MF0gUklQOiAwMDEwOls8ZmZmZmZmZmY4MTE0YWUxMj5dICBbPGZmZmZmZmZmODExNGFlMTI+ XSB1bmxvY2tfcGFnZSsweDgyLzB4OTANClsgIDE3OS45ODc1NzBdIFJTUDogMDAxODpmZmZmODgw MDFiMjIzOTk4ICBFRkxBR1M6IDAwMDEwMjQ2DQpbICAxNzkuOTg3NTcwXSBSQVg6IDAwMDAwMDAw MDAwMDAwMzYgUkJYOiBmZmZmODgwMDFlMDA5YTUwIFJDWDogMDAwMDAwMDAwOTEwMDkwZg0KWyAg MTc5Ljk4NzU3MF0gUkRYOiAwMDAwMDAwMDAwMDAwOTEwIFJTSTogMDAwMDAwMDAwMDAwMDAwMSBS REk6IGZmZmZmZmZmODE2MDBhN2YNClsgIDE3OS45ODc1NzBdIFJCUDogZmZmZjg4MDAxYjIyMzk5 OCBSMDg6IDAwMDAwMDAwMDAwMDAwMDEgUjA5OiAwMDAwMDAwMDAwMDAwMDAxDQpbICAxNzkuOTg3 NTcwXSBSMTA6IDAwMDAwMDAwMDAwMDAwMDEgUjExOiAwMDAwMDAwMDAwMDAwMDAwIFIxMjogMDAw MDAwMDAwMDAwMDAwMQ0KWyAgMTc5Ljk4NzU3MF0gUjEzOiBmZmZmODgwMDFlMDA5OWMwIFIxNDog MDAwMDAwMDBmZmZmZmZmYiBSMTU6IGZmZmY4ODAwMWUwMDk5YzANClsgIDE3OS45ODc1NzBdIEZT OiAgMDAwMDdmY2IxN2M3MzgwMCgwMDAwKSBHUzpmZmZmODgwMDI3ZjQwMDAwKDAwMDApIGtubEdT OjAwMDAwMDAwMDAwMDAwMDANClsgIDE3OS45ODc1NzBdIENTOiAgMDAxMCBEUzogMDAwMCBFUzog MDAwMCBDUjA6IDAwMDAwMDAwODAwNTAwM2INClsgIDE3OS45ODc1NzBdIENSMjogMDAwMDdmZmYx NjI5ZWJjOCBDUjM6IDAwMDAwMDAwMWRjNGYwMDAgQ1I0OiAwMDAwMDAwMDAwMDAwNmUwDQpbICAx NzkuOTg3NTcwXSBTdGFjazoNClsgIDE3OS45ODc1NzBdICBmZmZmODgwMDFiMjIzOWE4IGZmZmZm ZmZmODExNGFmZGUgZmZmZjg4MDAxYjIyMzlkOCBmZmZmZmZmZjgxMWYyZTUyDQpbICAxNzkuOTg3 NTcwXSAgZmZmZjg4MDAxZTAwOTljMCAwMDAwMDAwMGZmZmZmZmZiIDAwMDAwMDAwMDAwMDAwMDAg ZmZmZjg4MDAyNTM0OGU4OA0KWyAgMTc5Ljk4NzU3MF0gIGZmZmY4ODAwMWIyMjNhMDggZmZmZmZm ZmY4MTJlOWFkMyBmZmZmODgwMDFiMjIzYTA4IGZmZmZmZmZmODEwOWI2YjMNClsgIDE3OS45ODc1 NzBdIENhbGwgVHJhY2U6DQpbICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMTRhZmRlPl0gcGFn ZV9lbmRpbysweDFlLzB4ODANClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZmODExZjJlNTI+XSBt cGFnZV9lbmRfaW8rMHg0Mi8weDYwDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMmU5YWQz Pl0gYmlvX2VuZGlvKzB4NTMvMHhhMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTA5YjZi Mz5dID8gdXBfcmVhZCsweDIzLzB4NDANClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZmODE0MTNm MDk+XSB6cmFtX21ha2VfcmVxdWVzdCsweDJhOS8weDNjMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZm ZmZmZmY4MTJlZTMzMD5dIGdlbmVyaWNfbWFrZV9yZXF1ZXN0KzB4YzAvMHgxMDANClsgIDE3OS45 ODc1NzBdICBbPGZmZmZmZmZmODEyZWUzZTU+XSBzdWJtaXRfYmlvKzB4NzUvMHgxNDANClsgIDE3 OS45ODc1NzBdICBbPGZmZmZmZmZmODEyZWUzNzU+XSA/IHN1Ym1pdF9iaW8rMHg1LzB4MTQwDQpb ICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMWYyZTAzPl0gbXBhZ2VfYmlvX3N1Ym1pdCsweDMz LzB4NDANClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZmODExZjNkZTU+XSBtcGFnZV9yZWFkcGFn ZXMrMHhmNS8weDExMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTFlZDdhMD5dID8gSV9C REVWKzB4MTAvMHgxMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTFlZDdhMD5dID8gSV9C REVWKzB4MTAvMHgxMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTYwMzI0MT5dID8gZnRy YWNlX2NhbGwrMHg1LzB4MmYNClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZmODE2MDMyNDE+XSA/ IGZ0cmFjZV9jYWxsKzB4NS8weDJmDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMWVlMTkw Pl0gPyBibGtkZXZfd3JpdGVfYmVnaW4rMHgzMC8weDMwDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZm ZmZmZjgxMWVkN2EwPl0gPyBJX0JERVYrMHgxMC8weDEwDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZm ZmZmZjgxMWVlMWFkPl0gYmxrZGV2X3JlYWRwYWdlcysweDFkLzB4MjANClsgIDE3OS45ODc1NzBd ICBbPGZmZmZmZmZmODExNThlZDQ+XSBfX2RvX3BhZ2VfY2FjaGVfcmVhZGFoZWFkKzB4MjA0LzB4 MjkwDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMTU4ZDk5Pl0gPyBfX2RvX3BhZ2VfY2Fj aGVfcmVhZGFoZWFkKzB4YzkvMHgyOTANClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZmODExNGIw NDU+XSA/IGZpbmRfZ2V0X2VudHJ5KzB4NS8weDEzMA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZm ZmY4MTE1OTNjZD5dIGZvcmNlX3BhZ2VfY2FjaGVfcmVhZGFoZWFkKzB4N2QvMHhiMA0KWyAgMTc5 Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTE1OTQ0Mz5dIHBhZ2VfY2FjaGVfc3luY19yZWFkYWhlYWQr MHg0My8weDUwDQpbICAxNzkuOTg3NTcwXSAgWzxmZmZmZmZmZjgxMTRkMGUxPl0gZ2VuZXJpY19m aWxlX3JlYWRfaXRlcisweDQ1MS8weDY1MA0KWyAgMTc5Ljk4NzU3MF0gIFs8ZmZmZmZmZmY4MTYw MzI0MT5dID8gZnRyYWNlX2NhbGwrMHg1LzB4MmYNClsgIDE3OS45ODc1NzBdICBbPGZmZmZmZmZm ODExZWQ5NDc+XSBibGtkZXZfcmVhZF9pdGVyKzB4MzcvMHg0MA0KWyAgMTc5Ljk4NzU3MF0gIFs8 ZmZmZmZmZmY4MTFiNDM1OD5dIG5ld19zeW5jX3JlYWQrMHg3OC8weGIwDQpbICAxNzkuOTg3NTcw XSAgWzxmZmZmZmZmZjgxMWI1NTViPl0gdmZzX3JlYWQrMHhhYi8weDE4MA0KWyAgMTc5Ljk4NzU3 MF0gIFs8ZmZmZmZmZmY4MTFiNTY4Mj5dIFN5U19yZWFkKzB4NTIvMHhiMA0KWyAgMTc5Ljk4NzU3 MF0gIFs8ZmZmZmZmZmY4MTYwMTFkMj5dIHN5c3RlbV9jYWxsX2Zhc3RwYXRoKzB4MTIvMHgxNw0K WyAgMTc5Ljk4NzU3MF0gQ29kZTogMDAgNGMgOGIgODIgZjggMDAgMDAgMDAgMzEgZDIgNDggZDMg ZWUgNDggOGQgM2MgZjYgNDggODkgYzYgNDkgOGQgM2MgZjggZTggZGMgOWYgZjQgZmYgNWQgYzMg NDggYzcgYzYgODggMmMgYTAgODEgZTggMWUgYTggMDIgMDAgPDBmPiAwYiA2NiA2NiA2NiAyZSAw ZiAxZiA4NCAwMCAwMCAwMCAwMCAwMCBlOCBiYiA4MyA0YiAwMCA1NSA0OCANClsgIDE3OS45ODc1 NzBdIFJJUCAgWzxmZmZmZmZmZjgxMTRhZTEyPl0gdW5sb2NrX3BhZ2UrMHg4Mi8weDkwDQpbICAx NzkuOTg3NTcwXSAgUlNQIDxmZmZmODgwMDFiMjIzOTk4Pg0KWyAgMTc5Ljk4NzU3MF0gLS0tWyBl bmQgdHJhY2UgMzljNzNjMWQ5ZGE4N2VjNCBdLS0tDQpbDQoNClRoZSByZWFzb24gd2FzIHRoYXQg cmVhZCBJL08gY2F1c2VkIGJkZXZfcmVhZF9wYWdlIGNvdWxkIGJlIGZhaWxlZCBieQ0KZHJpdmVy J3MgaW50ZXJuYWwgcHJvYmxlbSBzbyBkcml2ZXIgY291bGQgc2V0IHRoZSBwYWdlIGFzIFBhZ2VF cnJvcg0KYW5kIHRoZW4gdW5sb2NrIHRoZSBwYWdlIGJ5IHBhZ2VfZW5kaW8uIEhvd2V2ZXIsIGRv X21wYWdlX3JlYWRwYWdlDQpyZXRyeSB0aGUgSS9PIHdpdGggQklPIHBhdGggd2l0aG91dCBhbnkg bG9ja2luZy9jbGVhbmluZyBQR19lcnJvcg0Kc28gaWYgaXQgZmFpbHMgYWdhaW4sIHdlIGNvdWxk IGVuY291bnRlciBhYm92ZSB3YXJuaW5nLg0KDQpTaG91bGQgd2Ugc29sdmUgaXQgd2l0aCBpbnRy b2R1Y2luZyBwYWdlX2VuZGlvX25vbG9jayB3aGljaA0Kd2lsbCBub3QgdW5sb2NrIHRoZSBwYWdl IGluIGNhc2Ugb2YgcmVhZC1mYWlsdXJlIGFuZCB1c2UgdGhlIGZ1bmN0aW9uDQppbiByd19wYWdl IGZ1bmN0aW9ucz8gSXQgcmVsaWVzIG9uIHJldHJ5aW5nIGxvZ2ljIG9mIGNhbGxlciBzbyBpdCdz DQp1Z2x5Lg0KDQpBbm90aGVyIHNvdWx1dGlvbiBpcyB3ZSBjYW4gY2xlYW4gUEdfZXJyb3IgYW5k IGxvY2tzIHRoZSBwYWdlDQphZ2FpbiBiZWZvcmUgdGhlIGdvaW5nIGJpbyBwYXRoIGJ1dCBJJ20g bm90IHN1cmUgaXQgZG9lc24ndCBoYXZlDQphbnkgc2lkZS1lZmZlY3QgYWZ0ZXIgcmVsZWFzaW5n IHRoZSBsb2NrLiBJZiBpdCBkb2Vzbid0IGhhdmUgYW55DQpzaWRlLWVmZmVjdCwgaXQgd291bGQg YmUgYmVzdC4NCg0KVGhlIHNpbXBsZXN0IHNvbHV0aW9uIEkgY2FuIHRoaW5rIG9mIGlzIHRvIGJh aWwgb3V0IGluIGNhc2Ugb2YNCmZhaWwgb2YgYmRldl9yZWFkX3BhZ2Ugd2l0aG91dCBnb2luZyBv biBhbm90aGVyIHJvdXRlIGJ1dCB5b3Ugc2VlbQ0KdG8gYmUgY2FyZWZ1bCB3aGVuIEkgc2F3IHRo ZSBjb21tZW50IG9mIGJkZXZfcmVhZF9wYWdlLg0KDQoiRXJyb3JzIHJldHVybmVkIGJ5IHRoaXMg ZnVuY3Rpb24gYXJlIHVzdWFsbHkgInNvZnQiLCBlZyBvdXQgb2YgbWVtb3J5LCBvcg0KIHF1ZXVl IGZ1bGw7IGNhbGxlcnMgc2hvdWxkIHRyeSBhIGRpZmZlcmVudCByb3V0ZSB0byByZWFkIHRoaXMg cGFnZSByYXRoZXINCiB0aGFuIHByb3BhZ2F0ZSBhbiBlcnJvciBiYWNrIHVwIHRoZSBzdGFjay4i DQoNCkFjdHVhbGx5LCBJJ20gbm90IHN1cmUgaG93IG11Y2ggc3VjaCBmYWxsYmFjayByZXRyaWFs IHJlYWxseSBtYWtlcyBmb3J3YXJkDQpwcm9ncmVzcz8NCg0KQW55d2F5LCBJJ2QgbGlrZSB0byBs aXN0ZW4gdGhlIG9waW5pb25zLg0KDQpUaGFua3MuDQoNCg0KLS0gDQpLaW5kIHJlZ2FyZHMsDQpN aW5jaGFuIEtpbQ0K

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw
Quoted 1 line · Matthew R

It works better when you email my Linux account instead of my Exchange account.

Finally, I got realized what's different between linux.intel.com and intel.com. :) I will use it later.

Quoted 4 lines

I think the problem is that you set the page error, and also return failure from bdev_read_page() which causes the page to be submitted for I/O twice.

Assuming the error is 'hard' (ie reading the page again will not succeed), you should set the page error, and return 0 from your rw_page operation, indicating that the submission was successful but the read failed. If the read might succeed, you should not call page_endio(), instead returning an errno from rw_page().

Thanks for the advise.
I hope there is a comment about that around rw_page.

Quoted 2 lines

I'm now wondering if the return code from rw_page should be flipped to be 1 on success and 0 for failure, so we don't get driver writers trying to return error codes?

I totally got confused so it's a good idea if you don't care of error propagation. boolean would be more clear.

Quoted 113 lines

-----Original Message-----
From: Minchan Kim [mailto:[email protected]]
Sent: Monday, November 03, 2014 5:49 PM
To: Wilcox, Matthew R
Cc: Dave Chinner; Andrew Morton; karam.lee; [email protected]; [email protected]
Subject: bdev_read_page

Hello,

[1] b07b0aaf54ace05, zram: implement rw_page operation of zram

After I merged [1] and was testing zram, I got following warning.

[  179.987592] zram0: detected capacity change from 2147483648 to 0
[  179.987570] page:ffffea00008d6300 count:2 mapcount:0 mapping:ffff880025348e88 index:0x0
[  179.987570] flags: 0x100000000020002(error|mappedtodisk)
[  179.987570] page dumped because: VM_BUG_ON_PAGE(!PageLocked(page))
[  179.987570] ------------[ cut here ]------------
[  179.987570] kernel BUG at mm/filemap.c:747!
[  179.987570] invalid opcode: 0000 [#1] SMP 
[  179.987570] Dumping ftrace buffer:
[  179.987570]    (ftrace buffer empty)
[  179.987570] Modules linked in:
[  179.987570] CPU: 10 PID: 23080 Comm: udisks-part-id Not tainted 3.18.0-rc2+ #584
[  179.987570] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  179.987570] task: ffff88001de74200 ti: ffff88001b220000 task.ti: ffff88001b220000
[  179.987570] RIP: 0010:[<ffffffff8114ae12>]  [<ffffffff8114ae12>] unlock_page+0x82/0x90
[  179.987570] RSP: 0018:ffff88001b223998  EFLAGS: 00010246
[  179.987570] RAX: 0000000000000036 RBX: ffff88001e009a50 RCX: 000000000910090f
[  179.987570] RDX: 0000000000000910 RSI: 0000000000000001 RDI: ffffffff81600a7f
[  179.987570] RBP: ffff88001b223998 R08: 0000000000000001 R09: 0000000000000001
[  179.987570] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000001
[  179.987570] R13: ffff88001e0099c0 R14: 00000000fffffffb R15: ffff88001e0099c0
[  179.987570] FS:  00007fcb17c73800(0000) GS:ffff880027f40000(0000) knlGS:0000000000000000
[  179.987570] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  179.987570] CR2: 00007fff1629ebc8 CR3: 000000001dc4f000 CR4: 00000000000006e0
[  179.987570] Stack:
[  179.987570]  ffff88001b2239a8 ffffffff8114afde ffff88001b2239d8 ffffffff811f2e52
[  179.987570]  ffff88001e0099c0 00000000fffffffb 0000000000000000 ffff880025348e88
[  179.987570]  ffff88001b223a08 ffffffff812e9ad3 ffff88001b223a08 ffffffff8109b6b3
[  179.987570] Call Trace:
[  179.987570]  [<ffffffff8114afde>] page_endio+0x1e/0x80
[  179.987570]  [<ffffffff811f2e52>] mpage_end_io+0x42/0x60
[  179.987570]  [<ffffffff812e9ad3>] bio_endio+0x53/0xa0
[  179.987570]  [<ffffffff8109b6b3>] ? up_read+0x23/0x40
[  179.987570]  [<ffffffff81413f09>] zram_make_request+0x2a9/0x3c0
[  179.987570]  [<ffffffff812ee330>] generic_make_request+0xc0/0x100
[  179.987570]  [<ffffffff812ee3e5>] submit_bio+0x75/0x140
[  179.987570]  [<ffffffff812ee375>] ? submit_bio+0x5/0x140
[  179.987570]  [<ffffffff811f2e03>] mpage_bio_submit+0x33/0x40
[  179.987570]  [<ffffffff811f3de5>] mpage_readpages+0xf5/0x110
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff811ee190>] ? blkdev_write_begin+0x30/0x30
[  179.987570]  [<ffffffff811ed7a0>] ? I_BDEV+0x10/0x10
[  179.987570]  [<ffffffff811ee1ad>] blkdev_readpages+0x1d/0x20
[  179.987570]  [<ffffffff81158ed4>] __do_page_cache_readahead+0x204/0x290
[  179.987570]  [<ffffffff81158d99>] ? __do_page_cache_readahead+0xc9/0x290
[  179.987570]  [<ffffffff8114b045>] ? find_get_entry+0x5/0x130
[  179.987570]  [<ffffffff811593cd>] force_page_cache_readahead+0x7d/0xb0
[  179.987570]  [<ffffffff81159443>] page_cache_sync_readahead+0x43/0x50
[  179.987570]  [<ffffffff8114d0e1>] generic_file_read_iter+0x451/0x650
[  179.987570]  [<ffffffff81603241>] ? ftrace_call+0x5/0x2f
[  179.987570]  [<ffffffff811ed947>] blkdev_read_iter+0x37/0x40
[  179.987570]  [<ffffffff811b4358>] new_sync_read+0x78/0xb0
[  179.987570]  [<ffffffff811b555b>] vfs_read+0xab/0x180
[  179.987570]  [<ffffffff811b5682>] SyS_read+0x52/0xb0
[  179.987570]  [<ffffffff816011d2>] system_call_fastpath+0x12/0x17
[  179.987570] Code: 00 4c 8b 82 f8 00 00 00 31 d2 48 d3 ee 48 8d 3c f6 48 89 c6 49 8d 3c f8 e8 dc 9f f4 ff 5d c3 48 c7 c6 88 2c a0 81 e8 1e a8 02 00 <0f> 0b 66 66 66 2e 0f 1f 84 00 00 00 00 00 e8 bb 83 4b 00 55 48 
[  179.987570] RIP  [<ffffffff8114ae12>] unlock_page+0x82/0x90
[  179.987570]  RSP <ffff88001b223998>
[  179.987570] ---[ end trace 39c73c1d9da87ec4 ]---

[

The reason was that read I/O caused bdev_read_page could be failed by driver's internal problem so driver could set the page as PageError and then unlock the page by page_endio. However, do_mpage_readpage retry the I/O with BIO path without any locking/cleaning PG_error so if it fails again, we could encounter above warning.

Should we solve it with introducing page_endio_nolock which will not unlock the page in case of read-failure and use the function in rw_page functions? It relies on retrying logic of caller so it's ugly.

Another soulution is we can clean PG_error and locks the page again before the going bio path but I'm not sure it doesn't have any side-effect after releasing the lock. If it doesn't have any side-effect, it would be best.

The simplest solution I can think of is to bail out in case of fail of bdev_read_page without going on another route but you seem to be careful when I saw the comment of bdev_read_page.

"Errors returned by this function are usually "soft", eg out of memory, or
queue full; callers should try a different route to read this page rather than propagate an error back up the stack."

Actually, I'm not sure how much such fallback retrial really makes forward progress?

Anyway, I'd like to listen the opinions.

Thanks.

Signature · 3 lines
Kind regards,
Minchan Kim
N?????r??zǧu???Ơ{???칻?&ޖ)??i???^n?r?????ݢj$??$????????~?'.)???,y?m????%?{??j+????צj)Z????f????{d??$??????����
Signature · 8 lines
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>