I was trying to remove a LVM logical volume on a hotplugged device
that had been removed, to also remove its VG, which resulted in:
[1728002.718174] ------------[ cut here ]------------
[1728002.718179] WARNING: CPU: 10 PID: 15454 at fs/block_dev.c:57
__blkdev_put+0xc1/0x220()
[1728002.718180] Modules linked in: dm_crypt mce_inject vfat fat bnep
bluetooth rfkill xt_CHECKSUM iptable_mangle ipt_MASQUERADE
nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc
ebtable_filter ebtables ip6table_filter ip6_tables fuse
snd_hda_codec_realtek x86_pkg_temp_thermal coretemp
snd_hda_codec_generic snd_hda_codec_hdmi kvm_intel snd_hda_intel kvm
snd_hda_controller iTCO_wdt xfs snd_hda_codec iTCO_vendor_support
snd_hwdep snd_seq mxm_wmi snd_seq_device crct10dif_pclmul snd_pcm
crc32_pclmul crc32c_intel snd_timer sb_edac snd libcrc32c mei_me
ghash_clmulni_intel lpc_ich edac_core serio_raw i2c_i801 soundcore mei
shpchp mfd_core nuvoton_cir rc_core wmi binfmt_misc uas usb_storage
radeon i2c_algo_bit drm_kms_helper ttm
[1728002.718209] e1000e drm firewire_ohci firewire_core ptp pps_core
crc_itu_t
[1728002.718213] CPU: 10 PID: 15454 Comm: umount Not tainted
4.0.4-301.fc22.x86_64 #1
[1728002.718214] Hardware name: /DX79SI, BIOS
SIX7910D.86A.1827.2013.0701.1715 07/01/2013
[1728002.718215] 0000000000000000 000000007fcff292 ffff88008f4abd98
ffffffff81782644
[1728002.718217] 0000000000000000 0000000000000000 ffff88008f4abdd8
ffffffff8109c66a
[1728002.718218] 0000000000000000 ffff8800c67584b8 ffff8800c6758340
ffff8800c6758430
[1728002.718220] Call Trace:
[1728002.718224] [<ffffffff81782644>] dump_stack+0x45/0x57
[1728002.718226] [<ffffffff8109c66a>] warn_slowpath_common+0x8a/0xc0
[1728002.718228] [<ffffffff8109c79a>] warn_slowpath_null+0x1a/0x20
[1728002.718230] [<ffffffff81258171>] __blkdev_put+0xc1/0x220
[1728002.718231] [<ffffffff81258760>] blkdev_put+0x50/0x130
[1728002.718234] [<ffffffff8121f9e1>] kill_block_super+0x41/0x80
[1728002.718236] [<ffffffff8121fd39>] deactivate_locked_super+0x49/0x80
[1728002.718238] [<ffffffff812201ac>] deactivate_super+0x6c/0x80
[1728002.718241] [<ffffffff8123ebb3>] cleanup_mnt+0x43/0xa0
[1728002.718245] [<ffffffff81140bd6>] ?
__audit_syscall_exit+0x1f6/0x290
[1728002.718246] [<ffffffff8123ec62>] __cleanup_mnt+0x12/0x20
[1728002.718249] [<ffffffff810b9a24>] task_work_run+0xc4/0xe0
[1728002.718252] [<ffffffff81013d0d>] do_notify_resume+0x9d/0xa0
[1728002.718255] [<ffffffff81788ee3>] int_signal+0x12/0x17
[1728002.718256] ---[ end trace 4975e97dd7331e63 ]---
Also the VG removal did not work of course.
-Andi
On Thu, Jun 18 2015 at 12:57pm -0400,
Andi Kleen <[email protected]> wrote:
>
> I was trying to remove a LVM logical volume on a hotplugged device
> that had been removed, to also remove its VG, which resulted in:
>
> [1728002.718174] ------------[ cut here ]------------
> [1728002.718179] WARNING: CPU: 10 PID: 15454 at fs/block_dev.c:57
> __blkdev_put+0xc1/0x220()
> [1728002.718180] Modules linked in: dm_crypt mce_inject vfat fat bnep
> bluetooth rfkill xt_CHECKSUM iptable_mangle ipt_MASQUERADE
> nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
> nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc
> ebtable_filter ebtables ip6table_filter ip6_tables fuse
> snd_hda_codec_realtek x86_pkg_temp_thermal coretemp
> snd_hda_codec_generic snd_hda_codec_hdmi kvm_intel snd_hda_intel kvm
> snd_hda_controller iTCO_wdt xfs snd_hda_codec iTCO_vendor_support
> snd_hwdep snd_seq mxm_wmi snd_seq_device crct10dif_pclmul snd_pcm
> crc32_pclmul crc32c_intel snd_timer sb_edac snd libcrc32c mei_me
> ghash_clmulni_intel lpc_ich edac_core serio_raw i2c_i801 soundcore mei
> shpchp mfd_core nuvoton_cir rc_core wmi binfmt_misc uas usb_storage
> radeon i2c_algo_bit drm_kms_helper ttm
> [1728002.718209] e1000e drm firewire_ohci firewire_core ptp pps_core
> crc_itu_t
> [1728002.718213] CPU: 10 PID: 15454 Comm: umount Not tainted
> 4.0.4-301.fc22.x86_64 #1
> [1728002.718214] Hardware name: /DX79SI, BIOS
> SIX7910D.86A.1827.2013.0701.1715 07/01/2013
> [1728002.718215] 0000000000000000 000000007fcff292 ffff88008f4abd98
> ffffffff81782644
> [1728002.718217] 0000000000000000 0000000000000000 ffff88008f4abdd8
> ffffffff8109c66a
> [1728002.718218] 0000000000000000 ffff8800c67584b8 ffff8800c6758340
> ffff8800c6758430
> [1728002.718220] Call Trace:
> [1728002.718224] [<ffffffff81782644>] dump_stack+0x45/0x57
> [1728002.718226] [<ffffffff8109c66a>] warn_slowpath_common+0x8a/0xc0
> [1728002.718228] [<ffffffff8109c79a>] warn_slowpath_null+0x1a/0x20
> [1728002.718230] [<ffffffff81258171>] __blkdev_put+0xc1/0x220
> [1728002.718231] [<ffffffff81258760>] blkdev_put+0x50/0x130
> [1728002.718234] [<ffffffff8121f9e1>] kill_block_super+0x41/0x80
> [1728002.718236] [<ffffffff8121fd39>] deactivate_locked_super+0x49/0x80
> [1728002.718238] [<ffffffff812201ac>] deactivate_super+0x6c/0x80
> [1728002.718241] [<ffffffff8123ebb3>] cleanup_mnt+0x43/0xa0
> [1728002.718245] [<ffffffff81140bd6>] ?
> __audit_syscall_exit+0x1f6/0x290
> [1728002.718246] [<ffffffff8123ec62>] __cleanup_mnt+0x12/0x20
> [1728002.718249] [<ffffffff810b9a24>] task_work_run+0xc4/0xe0
> [1728002.718252] [<ffffffff81013d0d>] do_notify_resume+0x9d/0xa0
> [1728002.718255] [<ffffffff81788ee3>] int_signal+0x12/0x17
> [1728002.718256] ---[ end trace 4975e97dd7331e63 ]---
Hmm, so you have a filesystem active on it too?
> Also the VG removal did not work of course.
Once you resolve the filesystem piece, from vgremove man page:
"vgremove allows you to remove one or more volume groups. If one or
more physical volumes in the volume group are lost, consider vgreduce
--removemissing to make the volume group metadata consistent again."
> Hmm, so you have a filesystem active on it too?
I unmounted it before.
>
> > Also the VG removal did not work of course.
>
> Once you resolve the filesystem piece, from vgremove man page:
>
> "vgremove allows you to remove one or more volume groups. If one or
> more physical volumes in the volume group are lost, consider vgreduce
> --removemissing to make the volume group metadata consistent again."
Well in any case there should not be WARN()s.
-Andi
--
[email protected] -- Speaking for myself only.
On Thu, Jun 18 2015 at 2:08pm -0400,
Andi Kleen <[email protected]> wrote:
> > Hmm, so you have a filesystem active on it too?
>
> I unmounted it before.
>
> >
> > > Also the VG removal did not work of course.
> >
> > Once you resolve the filesystem piece, from vgremove man page:
> >
> > "vgremove allows you to remove one or more volume groups. If one or
> > more physical volumes in the volume group are lost, consider vgreduce
> > --removemissing to make the volume group metadata consistent again."
>
> Well in any case there should not be WARN()s.
Yes well I don't even know what WARN_ON you're hitting. You're running
a 4.0.4 fedora kernel. Which WARN_ON() is triggering? The
WARN_ON_ONCE() in bdev_write_inode()? -- likely since the only caller of
bdev_write_inode is __blkdev_put...
/**
* write_inode_now - write an inode to disk
* @inode: inode to write to disk
* @sync: whether the write should be synchronous or not
*
* This function commits an inode to disk immediately if it is dirty. This is
* primarily needed by knfsd.
*
* The caller must either have a ref on the inode or must have set I_WILL_FREE.
*/
So I have no idea why bdev_write_inode() is using WARN_ON_ONCE.. makes
since that write_inode_now() will fail if the disk no longer exists. SO
the WARN_ON_ONCE seems misplaced.
Git blame shows its all hch's fault:
564f00f6c (Christoph Hellwig 2015-01-14 10:42:33 +0100 57) WARN_ON_ONCE(write_inode_now(inode, true));
564f00f6c block_dev: only write bdev inode on close
On Thu, Jun 18, 2015 at 02:16:19PM -0400, Mike Snitzer wrote:
> On Thu, Jun 18 2015 at 2:08pm -0400,
> Andi Kleen <[email protected]> wrote:
>
> > > Hmm, so you have a filesystem active on it too?
> >
> > I unmounted it before.
> >
> > >
> > > > Also the VG removal did not work of course.
> > >
> > > Once you resolve the filesystem piece, from vgremove man page:
> > >
> > > "vgremove allows you to remove one or more volume groups. If one or
> > > more physical volumes in the volume group are lost, consider vgreduce
> > > --removemissing to make the volume group metadata consistent again."
> >
> > Well in any case there should not be WARN()s.
>
> Yes well I don't even know what WARN_ON you're hitting. You're running
> a 4.0.4 fedora kernel. Which WARN_ON() is triggering? The
> WARN_ON_ONCE() in bdev_write_inode()? -- likely since the only caller of
> bdev_write_inode is __blkdev_put...
>
> /**
> * write_inode_now - write an inode to disk
> * @inode: inode to write to disk
> * @sync: whether the write should be synchronous or not
> *
> * This function commits an inode to disk immediately if it is dirty. This is
> * primarily needed by knfsd.
> *
> * The caller must either have a ref on the inode or must have set I_WILL_FREE.
> */
>
> So I have no idea why bdev_write_inode() is using WARN_ON_ONCE.. makes
> since that write_inode_now() will fail if the disk no longer exists. SO
> the WARN_ON_ONCE seems misplaced.
>
> Git blame shows its all hch's fault:
>
> 564f00f6c (Christoph Hellwig 2015-01-14 10:42:33 +0100 57) WARN_ON_ONCE(write_inode_now(inode, true));
>
> 564f00f6c block_dev: only write bdev inode on close
I can reproduce it too in a Fedora 22 virtual machine. I just have to do
"umount <mnt>" after hot unplugging the virtIO disk.
But it does not happen without lvm. If I create ext4 fs directly on
/dev/vda1, then when disk goes way, fs automatically gets unmounted.
But same does not happen when /dev/vda1 is added to a volume group and
I carve out a logical volume and create and mount fs.
In that case if I do umount after device has gone away, I can see above
WARN(). And it does seem to be coming from.
WARN_ON_ONCE(write_inode_now(inode, true))
If we failed to write back inode, then warning about it sounds right?
What's wrong with that? Should it be just a kernel log of level KERN_WARN
instead?
Thanks
Vivek
On Thu, Jun 18 2015 at 3:08pm -0400,
Vivek Goyal <[email protected]> wrote:
> On Thu, Jun 18, 2015 at 02:16:19PM -0400, Mike Snitzer wrote:
> > On Thu, Jun 18 2015 at 2:08pm -0400,
> > Andi Kleen <[email protected]> wrote:
> >
> > > > Hmm, so you have a filesystem active on it too?
> > >
> > > I unmounted it before.
> > >
> > > >
> > > > > Also the VG removal did not work of course.
> > > >
> > > > Once you resolve the filesystem piece, from vgremove man page:
> > > >
> > > > "vgremove allows you to remove one or more volume groups. If one or
> > > > more physical volumes in the volume group are lost, consider vgreduce
> > > > --removemissing to make the volume group metadata consistent again."
> > >
> > > Well in any case there should not be WARN()s.
> >
> > Yes well I don't even know what WARN_ON you're hitting. You're running
> > a 4.0.4 fedora kernel. Which WARN_ON() is triggering? The
> > WARN_ON_ONCE() in bdev_write_inode()? -- likely since the only caller of
> > bdev_write_inode is __blkdev_put...
> >
> > /**
> > * write_inode_now - write an inode to disk
> > * @inode: inode to write to disk
> > * @sync: whether the write should be synchronous or not
> > *
> > * This function commits an inode to disk immediately if it is dirty. This is
> > * primarily needed by knfsd.
> > *
> > * The caller must either have a ref on the inode or must have set I_WILL_FREE.
> > */
> >
> > So I have no idea why bdev_write_inode() is using WARN_ON_ONCE.. makes
> > since that write_inode_now() will fail if the disk no longer exists. SO
> > the WARN_ON_ONCE seems misplaced.
> >
> > Git blame shows its all hch's fault:
> >
> > 564f00f6c (Christoph Hellwig 2015-01-14 10:42:33 +0100 57) WARN_ON_ONCE(write_inode_now(inode, true));
> >
> > 564f00f6c block_dev: only write bdev inode on close
>
> I can reproduce it too in a Fedora 22 virtual machine. I just have to do
> "umount <mnt>" after hot unplugging the virtIO disk.
>
> But it does not happen without lvm. If I create ext4 fs directly on
> /dev/vda1, then when disk goes way, fs automatically gets unmounted.
>
> But same does not happen when /dev/vda1 is added to a volume group and
> I carve out a logical volume and create and mount fs.
>
> In that case if I do umount after device has gone away, I can see above
> WARN(). And it does seem to be coming from.
write_inode_now() should fail.. the device is no longer there. No idea
how virtio-blk avoids it, devil is in the blkdev refcount details I'm
sure.
> WARN_ON_ONCE(write_inode_now(inode, true))
>
> If we failed to write back inode, then warning about it sounds right?
A warning is fine.. not a WARN_ON(). Pretty alarming backtrace spew but
maybe I'm missing something and DM's blkdev refcount mgmt couldn't
trigger this WARN_ON()? I fail to see how to avoid it given the device
isn't thre so write_inode_now() fails.
> What's wrong with that? Should it be just a kernel log of level KERN_WARN
> instead?
Ideally, but I honestly don't have all the details paged in my head to
say definitively. First need to answer how vitrio-blk isn't hitting
this (and DM is). Could it be that __blkdev_put isn't getting called
for virtio-blk!?
> In that case if I do umount after device has gone away, I can see above
> WARN(). And it does seem to be coming from.
>
> WARN_ON_ONCE(write_inode_now(inode, true))
>
> If we failed to write back inode, then warning about it sounds right?
WARN() is for detecting kernel internal consistency problems, like
potential bugs. It's not for handling IO errors or really
everything that can happen on a non buggy kernel.
> What's wrong with that? Should it be just a kernel log of level KERN_WARN
> instead?
Yes. Something like that.
-Andi
--
[email protected] -- Speaking for myself only.
On Thu, Jun 18, 2015 at 03:08:15PM -0400, Vivek Goyal wrote:
> On Thu, Jun 18, 2015 at 02:16:19PM -0400, Mike Snitzer wrote:
> > On Thu, Jun 18 2015 at 2:08pm -0400,
> > Andi Kleen <[email protected]> wrote:
> >
> > > > Hmm, so you have a filesystem active on it too?
> > >
> > > I unmounted it before.
> > >
> > > >
> > > > > Also the VG removal did not work of course.
> > > >
> > > > Once you resolve the filesystem piece, from vgremove man page:
> > > >
> > > > "vgremove allows you to remove one or more volume groups. If one or
> > > > more physical volumes in the volume group are lost, consider vgreduce
> > > > --removemissing to make the volume group metadata consistent again."
> > >
> > > Well in any case there should not be WARN()s.
> >
> > Yes well I don't even know what WARN_ON you're hitting. You're running
> > a 4.0.4 fedora kernel. Which WARN_ON() is triggering? The
> > WARN_ON_ONCE() in bdev_write_inode()? -- likely since the only caller of
> > bdev_write_inode is __blkdev_put...
> >
> > /**
> > * write_inode_now - write an inode to disk
> > * @inode: inode to write to disk
> > * @sync: whether the write should be synchronous or not
> > *
> > * This function commits an inode to disk immediately if it is dirty. This is
> > * primarily needed by knfsd.
> > *
> > * The caller must either have a ref on the inode or must have set I_WILL_FREE.
> > */
> >
> > So I have no idea why bdev_write_inode() is using WARN_ON_ONCE.. makes
> > since that write_inode_now() will fail if the disk no longer exists. SO
> > the WARN_ON_ONCE seems misplaced.
> >
> > Git blame shows its all hch's fault:
> >
> > 564f00f6c (Christoph Hellwig 2015-01-14 10:42:33 +0100 57) WARN_ON_ONCE(write_inode_now(inode, true));
> >
> > 564f00f6c block_dev: only write bdev inode on close
>
> I can reproduce it too in a Fedora 22 virtual machine. I just have to do
> "umount <mnt>" after hot unplugging the virtIO disk.
>
> But it does not happen without lvm. If I create ext4 fs directly on
> /dev/vda1, then when disk goes way, fs automatically gets unmounted.
Let me correct myself. It is happening without lvm too. So I just
exported a virtio disk in guest. Created a partition /dev/vda1. Created
fs ext4 and mounted /dev/vda1. Opened a text file, scribbled something
into it, closed file and got out of mount directory. And now I removed
the disk and tried to umount /dev/vda1 and and boom, I get the error.
Last time I did not notice it because it is WARN_ON_ONCE() and I had
not rebooted my machine.
So problem does not seem to be related to device mapper.
To me it looks like that we should just get rid of WARN_ON_ONCE() a
and replace it with something like, printk(KERN_WARN: "write_inode_now() failed.").
Jun 18 16:56:35 vm2-f22 kernel: Buffer I/O error on dev vda1, logical
block 557056, lost sync page write
Jun 18 16:56:35 vm2-f22 kernel: JBD2: Error -5 detected when updating
journal superblock for vda1-8.
Jun 18 16:56:35 vm2-f22 kernel: Buffer I/O error on dev vda1, logical
block 0, lost sync page write
Jun 18 16:56:35 vm2-f22 kernel: ------------[ cut here ]------------
Jun 18 16:56:35 vm2-f22 kernel: WARNING: CPU: 5 PID: 1870 at
fs/block_dev.c:56 __blkdev_put+0xc0/0x220()
Jun 18 16:56:35 vm2-f22 kernel: Modules linked in: nf_conntrack_netbios_ns
nf_conntrack_broadcast ip6t_rpfilter ip6t_REJECT nf_reject_ipv6
xt_conntrack ebtable_nat ebtable_broute bridge ebtable_filter ebtables
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle
ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_security iptable_raw snd_hda_codec_generic
snd_hda_intel snd_hda_controller virtio_balloon snd_hda_codec snd_hwdep
snd_seq snd_seq_device snd_pcm snd_hda_core snd_timer snd soundcore
crct10dif_pclmul ppdev crc32_pclmul parport_pc crc32c_intel acpi_cpufreq
parport pvpanic i2c_piix4 serio_raw ghash_clmulni_intel nfsd auth_rpcgss
nfs_acl lockd grace sunrpc qxl drm_kms_helper ttm 8021q
Jun 18 16:56:35 vm2-f22 kernel: drm garp stp virtio_console virtio_blk
llc 8139too mrp virtio_pci 8139cp virtio_ring ata_generic mii virtio
pata_acpi
Jun 18 16:56:35 vm2-f22 kernel: CPU: 5 PID: 1870 Comm: umount Not tainted
4.1.0-rc8+ #3
Jun 18 16:56:35 vm2-f22 kernel: Hardware name: Red Hat KVM, BIOS 0.5.1
01/01/2011
Jun 18 16:56:35 vm2-f22 kernel: 0000000000000000 00000000a79061d2
ffff88021816fd98 ffffffff817fc09b
Jun 18 16:56:35 vm2-f22 kernel: 0000000000000000 0000000000000000
ffff88021816fdd8 ffffffff8109f47a
Jun 18 16:56:35 vm2-f22 kernel: 0000000000000000 ffff880236c287f8
ffff880236c28680 ffff880236c28770
Jun 18 16:56:35 vm2-f22 kernel: Call Trace:
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff817fc09b>] dump_stack+0x45/0x57
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff8109f47a>]
warn_slowpath_common+0x8a/0xc0
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff8109f5aa>]
warn_slowpath_null+0x1a/0x20
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81259910>]
__blkdev_put+0xc0/0x220
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81259f00>]
blkdev_put+0x50/0x130
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff812212b1>]
kill_block_super+0x41/0x80
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81221609>]
deactivate_locked_super+0x49/0x80
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81221a6c>]
deactivate_super+0x6c/0x80
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81240383>]
cleanup_mnt+0x43/0xa0
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81240432>]
__cleanup_mnt+0x12/0x20
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff810bc594>]
task_work_run+0xd4/0xf0
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81013d15>]
do_notify_resume+0x95/0xa0
Jun 18 16:56:35 vm2-f22 kernel: [<ffffffff81802a3c>] int_signal+0x12/0x17
Jun 18 16:56:35 vm2-f22 kernel: ---[ end trace cf189e68bb5b80cd ]---
Thanks
Vivek
On Thu, Jun 18, 2015 at 03:28:21PM -0400, Mike Snitzer wrote:
> > WARN_ON_ONCE(write_inode_now(inode, true))
> >
> > If we failed to write back inode, then warning about it sounds right?
>
> A warning is fine.. not a WARN_ON(). Pretty alarming backtrace spew but
> maybe I'm missing something and DM's blkdev refcount mgmt couldn't
> trigger this WARN_ON()? I fail to see how to avoid it given the device
> isn't thre so write_inode_now() fails.
>
> > What's wrong with that? Should it be just a kernel log of level KERN_WARN
> > instead?
>
> Ideally, but I honestly don't have all the details paged in my head to
> say definitively. First need to answer how vitrio-blk isn't hitting
> this (and DM is). Could it be that __blkdev_put isn't getting called
> for virtio-blk!?
Just a warnings if fine. In fact we can probably remove that as well
as it will happen after a hot removal all the time.
On Fri, Jun 19, 2015 at 08:47:21AM +0200, Christoph Hellwig wrote:
> On Thu, Jun 18, 2015 at 03:28:21PM -0400, Mike Snitzer wrote:
> > > WARN_ON_ONCE(write_inode_now(inode, true))
> > >
> > > If we failed to write back inode, then warning about it sounds right?
> >
> > A warning is fine.. not a WARN_ON(). Pretty alarming backtrace spew but
> > maybe I'm missing something and DM's blkdev refcount mgmt couldn't
> > trigger this WARN_ON()? I fail to see how to avoid it given the device
> > isn't thre so write_inode_now() fails.
> >
> > > What's wrong with that? Should it be just a kernel log of level KERN_WARN
> > > instead?
> >
> > Ideally, but I honestly don't have all the details paged in my head to
> > say definitively. First need to answer how vitrio-blk isn't hitting
> > this (and DM is). Could it be that __blkdev_put isn't getting called
> > for virtio-blk!?
>
> Just a warnings if fine. In fact we can probably remove that as well
> as it will happen after a hot removal all the time.
[CC Tejun]
Does following patch look good?
Thanks
Vivek
Subject: fs/block_dev.c: Warn on inode writeback failure instead of WARN_ON()
If a block device is hot removed and later last reference to devices
is put, we try to writeback the dirty inode. But device is gone and
that writeback fails.
Currently we do a WARN_ON() which does not seem to be the right thing.
Convert it to a ratelimited kernel warning.
Reported-by: Andi Kleen <[email protected]>
Signed-off-by: Vivek Goyal <[email protected]>
---
fs/block_dev.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Index: rhvgoyal-linux/fs/block_dev.c
===================================================================
--- rhvgoyal-linux.orig/fs/block_dev.c 2015-06-18 15:54:52.339383237 -0400
+++ rhvgoyal-linux/fs/block_dev.c 2015-06-22 12:55:47.642504742 -0400
@@ -48,12 +48,17 @@ inline struct block_device *I_BDEV(struc
}
EXPORT_SYMBOL(I_BDEV);
-static void bdev_write_inode(struct inode *inode)
+static void bdev_write_inode(struct block_device *bdev)
{
+ struct inode *inode = bdev->bd_inode;
+
spin_lock(&inode->i_lock);
while (inode->i_state & I_DIRTY) {
spin_unlock(&inode->i_lock);
- WARN_ON_ONCE(write_inode_now(inode, true));
+ if (write_inode_now(inode, true)) {
+ char name[BDEVNAME_SIZE] = "";
+ pr_warn_ratelimited("VFS: Dirty inode writeback failed for block device %s.\n", bdevname(bdev, name));
+ }
spin_lock(&inode->i_lock);
}
spin_unlock(&inode->i_lock);
@@ -1489,7 +1494,7 @@ static void __blkdev_put(struct block_de
* ->release can cause the queue to disappear, so flush all
* dirty data before.
*/
- bdev_write_inode(bdev->bd_inode);
+ bdev_write_inode(bdev);
}
if (bdev->bd_contains == bdev) {
if (disk->fops->release)
Hello,
On Mon, Jun 22, 2015 at 01:46:48PM -0400, Vivek Goyal wrote:
> Subject: fs/block_dev.c: Warn on inode writeback failure instead of WARN_ON()
>
> If a block device is hot removed and later last reference to devices
> is put, we try to writeback the dirty inode. But device is gone and
> that writeback fails.
>
> Currently we do a WARN_ON() which does not seem to be the right thing.
> Convert it to a ratelimited kernel warning.
Yeah, looks good to me. Just one nit.
...
> + if (write_inode_now(inode, true)) {
> + char name[BDEVNAME_SIZE] = "";
> + pr_warn_ratelimited("VFS: Dirty inode writeback failed for block device %s.\n", bdevname(bdev, name));
This wasn't reported before either but maybe we wanna report the errno
too? Also, don't we usually break the line for parameters?
pr_..("long format string going over 80 col...\n",
param0, param1, ...);
Thanks.
--
tejun
On Mon, Jun 22, 2015 at 01:52:55PM -0400, Tejun Heo wrote:
> Hello,
>
> On Mon, Jun 22, 2015 at 01:46:48PM -0400, Vivek Goyal wrote:
> > Subject: fs/block_dev.c: Warn on inode writeback failure instead of WARN_ON()
> >
> > If a block device is hot removed and later last reference to devices
> > is put, we try to writeback the dirty inode. But device is gone and
> > that writeback fails.
> >
> > Currently we do a WARN_ON() which does not seem to be the right thing.
> > Convert it to a ratelimited kernel warning.
>
> Yeah, looks good to me. Just one nit.
>
> ...
> > + if (write_inode_now(inode, true)) {
> > + char name[BDEVNAME_SIZE] = "";
> > + pr_warn_ratelimited("VFS: Dirty inode writeback failed for block device %s.\n", bdevname(bdev, name));
>
> This wasn't reported before either but maybe we wanna report the errno
> too? Also, don't we usually break the line for parameters?
>
> pr_..("long format string going over 80 col...\n",
> param0, param1, ...);
Hi Tejun,
Will do. I have been doing some go programming in docker recently and this
has been a side effect of that. :-) Will format it correctly.
Thanks
Vivek
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>