Re: [BUG] xfs/104 triggered NULL pointer dereference in iomap based dio

2017-09-14 Thread Chandan Rajendra 
On Wednesday, September 13, 2017 10:25:46 PM IST Christoph Hellwig wrote:
> Does it work fine if you call sb_init_dio_done_wq unconditionally?
> 
> 

Yes, The issue isn't recreated when sb_init_dio_done_wq() is 
invoked unconditionally.

-- 
chandan

Re: [BUG] xfs/104 triggered NULL pointer dereference in iomap based dio

2017-09-13 Thread Chandan Rajendra 
On Wednesday, September 13, 2017 4:28:23 PM IST Eryu Guan wrote:
> Hi all,
> 
> Recently I noticed multiple crashes triggered by xfs/104 on ppc64 hosts
> in my upstream 4.13 kernel testings. The block layer is involved in the
> call trace so I add linux-block to cc list too. I append the full
> console log to the end of this mail.
> 
> Now I can reproduce the crash on x86_64 hosts too by running xfs/104
> many times (usually within 100 iterations). A git-bisect run (I ran it
> for 500 iterations before calling it good in bisect run to be safe)
> pointed the first bad commit to commit acdda3aae146 ("xfs: use
> iomap_dio_rw").
> 
> I confirmed the bisect result by checking out a new branch with commit
> acdda3aae146 as HEAD, xfs/104 would crash kernel within 100 iterations,
> then reverting HEAD, xfs/104 passed 1500 iterations.

I am able to recreate the issue on my ppc64 guest. I added some printk()
statements and got this,

xfs_fs_fill_super:1670: Filled up sb c006344db800.
iomap_dio_bio_end_io:784: sb = c006344db800; inode->i_sb->s_dio_done_wq =   
(null), >aio.work = c006344bb5b0.


In iomap_dio_rw(), I had added the following printk() statement,

   ret = sb_init_dio_done_wq(inode->i_sb);
if (ret < 0)
iomap_dio_set_error(dio, ret);
printk("%s:%d: sb = %p; Created s_dio_done_wq.\n",
__func__, __LINE__, inode->i_sb);

In the case of crash, I don't see the above message being printed. 

Btw, I am unable to recreate this issue on today's linux-next though. Maybe
it is because the race condition is accidently masked out.

I will continue debugging this and provide an update.

> 
> On one of my test vms, the crash happened as
> 
> [  340.419429] BUG: unable to handle kernel NULL pointer dereference at 
> 0102
> [  340.420408] IP: __queue_work+0x32/0x420
> 
> and that IP points to
> 
> (gdb) l *(__queue_work+0x32)
> 0x9cf32 is in __queue_work (kernel/workqueue.c:1383).
> 1378WARN_ON_ONCE(!irqs_disabled());
> 1379
> 1380debug_work_activate(work);
> 1381
> 1382/* if draining, only works from the same workqueue are 
> allowed */
> 1383if (unlikely(wq->flags & __WQ_DRAINING) &&
> 1384WARN_ON_ONCE(!is_chained_work(wq)))
> 1385return;
> 1386retry:
> 1387if (req_cpu == WORK_CPU_UNBOUND)
> 
> So looks like "wq" is null. The test vm is a kvm guest running 4.13
> kernel with 4 vcpus and 8G memory.
> 
> If more information is needed please let me know.
> 
> Thanks,
> Eryu
> 
> P.S. console log when crashing
> 
> [  339.746983] run fstests xfs/104 at 2017-09-13 17:38:26
> [  340.027352] XFS (vda6): Unmounting Filesystem
> [  340.207107] XFS (vda6): Mounting V5 Filesystem
> [  340.217553] XFS (vda6): Ending clean mount
> [  340.419429] BUG: unable to handle kernel NULL pointer dereference at 
> 0102
> [  340.420408] IP: __queue_work+0x32/0x420
> [  340.420408] PGD 215373067
> [  340.420408] P4D 215373067
> [  340.420408] PUD 21210d067
> [  340.420408] PMD 0
> [  340.420408]
> [  340.420408] Oops:  [#1] SMP
> [  340.420408] Modules linked in: xfs ip6t_rpfilter ipt_REJECT nf_reject_ipv4 
> nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 
> nf_defrag_ipv6 xt_conntrack nf_conntrack libcrc32c ip_set nfnetlink 
> ebtable_nat ebtable_broute bridge stp llc ip6table_mangle ip6table_security 
> ip6table_raw iptable_mangle iptable_security iptable_raw ebtable_filter 
> ebtables ip6table_filter ip6_tables iptable_filter btrfs xor raid6_pq ppdev 
> i2c_piix4 parport_pc i2c_core parport virtio_balloon pcspkr nfsd auth_rpcgss 
> nfs_acl lockd grace sunrpc ip_tables ext4 mbcache jbd2 ata_generic pata_acpi 
> virtio_net virtio_blk ata_piix libata virtio_pci virtio_ring serio_raw virtio 
> floppy
> [  340.420408] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.13.0 #64
> [  340.420408] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
> [  340.420408] task: 8b1d96222500 task.stack: b06bc0cb8000
> [  340.420408] RIP: 0010:__queue_work+0x32/0x420
> [  340.420408] RSP: 0018:8b1d9fd83d18 EFLAGS: 00010046
> [  340.420408] RAX: 0096 RBX: 0002 RCX: 
> 8b1d9489e6d8
> [  340.420408] RDX: 8b1d903c2090 RSI:  RDI: 
> 2000
> [  340.420408] RBP: 8b1d9fd83d58 R08: 0400 R09: 
> 0009
> [  340.420408] R10: 8b1d9532b400 R11:  R12: 
> 2000
> [  340.420408] R13:  R14: 8b1d903c2090 R15: 
> 7800
> [  340.420408] FS:  () GS:8b1d9fd8() 
> knlGS:
> [  340.420408] CS:  0010 DS:  ES:  CR0: 80050033
> [  340.420408] CR2: 0102 CR3: 0002152ce000 CR4: 
> 06e0
> [  340.420408] Call Trace:
> [  340.420408]  
> [  340.420408]  ? __slab_free+0x8e/0x260
> [

Re: [PATCH] mkfs: Remove messages printed when blocksize < physical sectorsize

2017-09-05 Thread Chandan Rajendra 
On Tuesday, September 5, 2017 12:12:08 PM IST Omar Sandoval wrote:
> On Mon, Sep 04, 2017 at 11:31:39PM -0700, Christoph Hellwig wrote:
> > On Tue, Sep 05, 2017 at 11:14:42AM +0530, Chandan Rajendra wrote:
> > > Linux kernel commit 6c6b6f28b3335fd85ec833ee0005d9c9dca6c003 (loop: set
> > > physical block size to PAGE_SIZE) now sets PAGE_SIZE as the default
> > > physical sector size of loop devices. On ppc64, this causes loop devices
> > > to have 64k as the physical sector size.
> > 
> > Eek.  We'll need to revert the loop change ASAP!
> 
> Most annoying patch series ever. It hasn't made it to Linus' tree yet,
> right? We can revert (although the later change depends on that), fold
> in a fix, or apply a fix on top of it, whatever Jens prefers.
> 
> 

My bad. 6c6b6f28b3335fd85ec833ee0005d9c9dca6c003 is the commit id from 
Linux-next. I don't see this commit in Linus's git tree.

-- 
chandan

Re: [PATCH] direct-io: don't introduce another read of inode->i_blkbits

2017-01-09 Thread Chandan Rajendra 
On Monday, January 09, 2017 04:42:58 PM Jeff Moyer wrote:
> Commit 20ce44d545844 ("do_direct_IO: Use inode->i_blkbits to compute
> block count to be cleaned") introduced a regression: if the block size
> of the block device is changed while a direct I/O request is being
> setup, it can result in a panic.  See commit ab73857e354ab ("direct-io:
> don't read inode->i_blkbits multiple times") for the reasoning, and
> commit b87570f5d3496 ("Fix a crash when block device is read and block
> size is changed at the same time") for a more detailed problem
> description and reproducer.
> 
> Fixes: 20ce44d545844
> Signed-off-by: Jeff Moyer <jmo...@redhat.com>
> 
> ---
> Chandan, can you please test this to ensure this still fixes your problem?

This patch fixes the failure,

Tested-by: Chandan Rajendra <chan...@linux.vnet.ibm.com>

-- 
chandan

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

[PATCH] do_direct_IO: Use inode->i_blkbits to compute block count to be cleaned

2017-01-08 Thread Chandan Rajendra 
The code currently uses sdio->blkbits to compute the number of blocks to
be cleaned. However sdio->blkbits is derived from the logical block size
of the underlying block device (Refer to the definition of
do_blockdev_direct_IO()). Due to this, generic/299 test would rarely
fail when executed on an ext4 filesystem with 64k as the block size and
when using a virtio based disk (having 512 byte as the logical block
size) inside a kvm guest.

This commit fixes the bug by using inode->i_blkbits to compute the
number of blocks to be cleaned.

Signed-off-by: Chandan Rajendra <chan...@linux.vnet.ibm.com>
---
 fs/direct-io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index aeae8c0..b20adf9 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -905,6 +905,7 @@ static inline void dio_zero_block(struct dio *dio, struct 
dio_submit *sdio,
 static int do_direct_IO(struct dio *dio, struct dio_submit *sdio,
struct buffer_head *map_bh)
 {
+   const unsigned i_blkbits = dio->inode->i_blkbits;
const unsigned blkbits = sdio->blkbits;
int ret = 0;
 
@@ -949,7 +950,7 @@ static int do_direct_IO(struct dio *dio, struct dio_submit 
*sdio,
clean_bdev_aliases(
map_bh->b_bdev,
map_bh->b_blocknr,
-   map_bh->b_size >> blkbits);
+   map_bh->b_size >> i_blkbits);
}
 
if (!sdio->blkfactor)
-- 
2.5.5

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