| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160126082904.GS24938@quack.suse.cz>
Date: Tue, 26 Jan 2016 09:29:04 +0100
From: Jan Kara <jack@...e.cz>
To: "Williams, Dan J" <dan.j.williams@...el.com>
Cc: "kirill@...temov.name" <kirill@...temov.name>,
"jack@...e.cz" <jack@...e.cz>,
"syzkaller@...glegroups.com" <syzkaller@...glegroups.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"kcc@...gle.com" <kcc@...gle.com>,
"vbabka@...e.cz" <vbabka@...e.cz>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"dvyukov@...gle.com" <dvyukov@...gle.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"gthelen@...gle.com" <gthelen@...gle.com>,
"willy@...ux.intel.com" <willy@...ux.intel.com>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"mhocko@...e.com" <mhocko@...e.com>,
"jack@...e.com" <jack@...e.com>,
"glider@...gle.com" <glider@...gle.com>,
"sasha.levin@...cle.com" <sasha.levin@...cle.com>,
"Wilcox, Matthew R" <matthew.r.wilcox@...el.com>,
"j-nomura@...jp.nec.com" <j-nomura@...jp.nec.com>
Subject: Re: mm: WARNING in __delete_from_page_cache
On Tue 26-01-16 03:42:34, Williams, Dan J wrote:
> On Mon, 2016-01-25 at 13:22 +0100, Jan Kara wrote:
> [..]
> > Thanks. Despite the huge list of recipients the author of the changes
> > hasn't been CCed :) I've added Dan to CC since he wrote DAX support
> > for
> > block devices. It seems somehow the write didn't go through the DAX
> > path
> > but through the standard page cache write path. Ah, I see, only
> > file->f_mapping->host has S_DAX set but io_is_direct() which decides
> > whether DAX or pagecache path should be used for writes uses file-
> > >f_inode
> > which is something different for block devices...
>
> Thanks, yes, the following silences the warning for me:
>
> 8<----- (git am --scissors)
> Subject: fs, block: force direct-I/O for dax-enabled block devices
>
> From: Dan Williams <dan.j.williams@...el.com>
>
> Similar to the file I/O path, re-direct all I/O to the DAX path for I/O
> to a block-device special file.
>
> Otherwise, we confuse the DAX code that does not expect to find live
> data in the page cache:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 7676 at mm/filemap.c:217
> __delete_from_page_cache+0x9f6/0xb60()
> Modules linked in:
> CPU: 0 PID: 7676 Comm: a.out Not tainted 4.4.0+ #276
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> 00000000ffffffff ffff88006d3f7738 ffffffff82999e2d 0000000000000000
> ffff8800620a0000 ffffffff86473d20 ffff88006d3f7778 ffffffff81352089
> ffffffff81658d36 ffffffff86473d20 00000000000000d9 ffffea0000009d60
> Call Trace:
> [< inline >] __dump_stack lib/dump_stack.c:15
> [<ffffffff82999e2d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
> [<ffffffff81352089>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
> [<ffffffff813522b9>] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
> [<ffffffff81658d36>] __delete_from_page_cache+0x9f6/0xb60 mm/filemap.c:217
> [<ffffffff81658fb2>] delete_from_page_cache+0x112/0x200 mm/filemap.c:244
> [<ffffffff818af369>] __dax_fault+0x859/0x1800 fs/dax.c:487
> [<ffffffff8186f4f6>] blkdev_dax_fault+0x26/0x30 fs/block_dev.c:1730
> [< inline >] wp_pfn_shared mm/memory.c:2208
> [<ffffffff816e9145>] do_wp_page+0xc85/0x14f0 mm/memory.c:2307
> [< inline >] handle_pte_fault mm/memory.c:3323
> [< inline >] __handle_mm_fault mm/memory.c:3417
> [<ffffffff816ecec3>] handle_mm_fault+0x2483/0x4640 mm/memory.c:3446
> [<ffffffff8127eff6>] __do_page_fault+0x376/0x960 arch/x86/mm/fault.c:1238
> [<ffffffff8127f738>] trace_do_page_fault+0xe8/0x420 arch/x86/mm/fault.c:1331
> [<ffffffff812705c4>] do_async_page_fault+0x14/0xd0 arch/x86/kernel/kvm.c:264
> [<ffffffff86338f78>] async_page_fault+0x28/0x30 arch/x86/entry/entry_64.S:986
> [<ffffffff86336c36>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> ---[ end trace dae21e0f85f1f98c ]---
>
> Cc: Matthew Wilcox <willy@...ux.intel.com>
> Cc: Ross Zwisler <ross.zwisler@...ux.intel.com>
> Fixes: 5a023cdba50c ("block: enable dax for raw block devices")
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Reported-by: Kirill A. Shutemov <kirill@...temov.name>
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
The patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/block_dev.c | 5 -----
> include/linux/fs.h | 12 +++++++++++-
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7b9cd49622b1..277008617b2d 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -156,11 +156,6 @@ blkdev_get_block(struct inode *inode, sector_t iblock,
> return 0;
> }
>
> -static struct inode *bdev_file_inode(struct file *file)
> -{
> - return file->f_mapping->host;
> -}
> -
> static ssize_t
> blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1a2046275cdf..a4c4314eed48 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1237,6 +1237,11 @@ static inline struct inode *file_inode(const struct file *f)
> return f->f_inode;
> }
>
> +static inline struct inode *bdev_file_inode(struct file *file)
> +{
> + return file->f_mapping->host;
> +}
> +
> static inline int locks_lock_file_wait(struct file *filp, struct file_lock *fl)
> {
> return locks_lock_inode_wait(file_inode(filp), fl);
> @@ -2907,7 +2912,12 @@ extern void replace_mount_options(struct super_block *sb, char *options);
>
> static inline bool io_is_direct(struct file *filp)
> {
> - return (filp->f_flags & O_DIRECT) || IS_DAX(file_inode(filp));
> + struct inode *inode = file_inode(filp);
> +
> + if (S_ISBLK(inode->i_mode))
> + inode = bdev_file_inode(filp);
> +
> + return (filp->f_flags & O_DIRECT) || IS_DAX(inode);
> }
>
> static inline int iocb_flags(struct file *file)
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists