Re: [PULL] Btrfs for 4.7, part 2
Chris, Sorry for the delay due to vacation. more below.. On 05/29/2016 08:21 PM, Chris Mason wrote: On Sat, May 28, 2016 at 01:14:13PM +0800, Anand Jain wrote: On 05/27/2016 11:42 PM, Chris Mason wrote: I'm getting errors from btrfs fi show -d, after the very last round of device replaces. A little extra debugging: bytenr mismatch, want=4332716032, have=0 ERROR: cannot read chunk root ERROR reading /dev/vdh failed /dev/vdh Which is cute because the very next command we run fscks /dev/vdh and succeeds. Checked the code paths both btrfs fi show -d and btrfs check, both are calling flush during relative open_ctree in progs. However the flush is called after we have read superblock. That means the read_superblock during 'show' cli (only) will read superblock without flush, and 'check' won't, because 011 calls 'check' after 'show'. But it still does not explain the above error, which is during open_ctree not at read superblock. Remains strange case as of now. It's because we're just not done writing it out yet when btrfs fi show is run. I think replace is special here. Also. I can't reproduce. I'm in a relatively new test rig using kvm, which probably explains why I haven't seen it before. You can probably make it easier by adding a sleep inside the actual __free_device() func. So the page cache is stale and this isn't related to any of our patches. close_ctree() calls into btrfs_close_devices(), which calls btrfs_close_one_device(), which uses: call_rcu(>rcu, free_device); close_ctree() also does an rcu_barrier() to make sure and wait for free_device() to finish. But, free_device() just puts the work into schedule_work(), so we don't know for sure the blkdev_put is done when we exit. Right, saw that before. Any idea why its like that ? Or if it should be fixed? It's just trying to limit the work that is done from call_rcu, and it should definitely be fixed. It might cause EBUSY or other problems. Probably easiest to add a counter or completion object that gets changed by the __free_device function. yes indeed sleep made the problem to reproduce, Also looks like this problem was identified by below commit before, however the fix wasn't correct. commit bc178622d40d87e75abc131007342429c9b03351 btrfs: use rcu_barrier() to wait for bdev puts at unmount :: Adding an rcu_barrier() to btrfs_close_devices() causes unmount to wait until all blkdev_put()s are done, and the device is truly free once unmount complet As free_devces() spinoff __free_device() to make the actual bdev put we need to wait on __free_device(). But rcu_barrier() just waits for free_device() to complete, so at the end of rcu_barrier() the blkdev_put() may not be completed. Wrote a new fix as in the patches, [PATH 2/2] btrfs: wait for bdev put For review comments. Thanks, -Anand -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On Sat, May 28, 2016 at 01:14:13PM +0800, Anand Jain wrote: On 05/27/2016 11:42 PM, Chris Mason wrote: I'm getting errors from btrfs fi show -d, after the very last round of device replaces. A little extra debugging: bytenr mismatch, want=4332716032, have=0 ERROR: cannot read chunk root ERROR reading /dev/vdh failed /dev/vdh Which is cute because the very next command we run fscks /dev/vdh and succeeds. Checked the code paths both btrfs fi show -d and btrfs check, both are calling flush during relative open_ctree in progs. However the flush is called after we have read superblock. That means the read_superblock during 'show' cli (only) will read superblock without flush, and 'check' won't, because 011 calls 'check' after 'show'. But it still does not explain the above error, which is during open_ctree not at read superblock. Remains strange case as of now. It's because we're just not done writing it out yet when btrfs fi show is run. I think replace is special here. Also. I can't reproduce. I'm in a relatively new test rig using kvm, which probably explains why I haven't seen it before. You can probably make it easier by adding a sleep inside the actual __free_device() func. So the page cache is stale and this isn't related to any of our patches. close_ctree() calls into btrfs_close_devices(), which calls btrfs_close_one_device(), which uses: call_rcu(>rcu, free_device); close_ctree() also does an rcu_barrier() to make sure and wait for free_device() to finish. But, free_device() just puts the work into schedule_work(), so we don't know for sure the blkdev_put is done when we exit. Right, saw that before. Any idea why its like that ? Or if it should be fixed? It's just trying to limit the work that is done from call_rcu, and it should definitely be fixed. It might cause EBUSY or other problems. Probably easiest to add a counter or completion object that gets changed by the __free_device function. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On 05/27/2016 11:42 PM, Chris Mason wrote: On Fri, May 27, 2016 at 10:35:27AM -0400, Chris Mason wrote: On Fri, May 27, 2016 at 01:18:22PM +0200, David Sterba wrote: On Thu, May 26, 2016 at 08:14:14PM -0400, Chris Mason wrote: On Thu, May 26, 2016 at 11:27:06AM +0200, David Sterba wrote: Hi, please pull a few more patches that did not go to pull #1 for 4.7, minor cleanups and fixes. Thanks. Thanks Dave! Trying to figure out why we're failing btrfs/011, but I don't see how it could be related to this bunch. I'll nail it down. 011 passes here, there are some unrelated soft-failures (mismatching output with new progs). I'm now testing a branch without "btrfs: scrub: Set bbio to NULL before calling btrfs_map_block", that seems to be the only likely offender. I'm getting errors from btrfs fi show -d, after the very last round of device replaces. A little extra debugging: bytenr mismatch, want=4332716032, have=0 ERROR: cannot read chunk root ERROR reading /dev/vdh failed /dev/vdh >> Which is cute because the very next command we run fscks /dev/vdh and succeeds. Checked the code paths both btrfs fi show -d and btrfs check, both are calling flush during relative open_ctree in progs. However the flush is called after we have read superblock. That means the read_superblock during 'show' cli (only) will read superblock without flush, and 'check' won't, because 011 calls 'check' after 'show'. But it still does not explain the above error, which is during open_ctree not at read superblock. Remains strange case as of now. Also. I can't reproduce. So the page cache is stale and this isn't related to any of our patches. close_ctree() calls into btrfs_close_devices(), which calls btrfs_close_one_device(), which uses: call_rcu(>rcu, free_device); close_ctree() also does an rcu_barrier() to make sure and wait for free_device() to finish. But, free_device() just puts the work into schedule_work(), so we don't know for sure the blkdev_put is done when we exit. Right, saw that before. Any idea why its like that ? Or if it should be fixed? It's been this way for a while, so its not holding up my pull request to Linus. But I'll fix it up. Yes. Its been like that. Thanks, Anand -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On Fri, May 27, 2016 at 10:35:27AM -0400, Chris Mason wrote: > On Fri, May 27, 2016 at 01:18:22PM +0200, David Sterba wrote: > > On Thu, May 26, 2016 at 08:14:14PM -0400, Chris Mason wrote: > > > On Thu, May 26, 2016 at 11:27:06AM +0200, David Sterba wrote: > > > > Hi, > > > > > > > > please pull a few more patches that did not go to pull #1 for 4.7, minor > > > > cleanups and fixes. Thanks. > > > > > > Thanks Dave! Trying to figure out why we're failing btrfs/011, but I > > > don't see how it could be related to this bunch. I'll nail it down. > > > > 011 passes here, there are some unrelated soft-failures (mismatching > > output with new progs). I'm now testing a branch without "btrfs: scrub: > > Set bbio to NULL before calling btrfs_map_block", that seems to be the > > only likely offender. > > I'm getting errors from btrfs fi show -d, after the very last round of > device replaces. A little extra debugging: > > bytenr mismatch, want=4332716032, have=0 > ERROR: cannot read chunk root > ERROR reading /dev/vdh > failed /dev/vdh > > Which is cute because the very next command we run fscks /dev/vdh and > succeeds. > > So the page cache is stale and this isn't related to any of our patches. close_ctree() calls into btrfs_close_devices(), which calls btrfs_close_one_device(), which uses: call_rcu(>rcu, free_device); close_ctree() also does an rcu_barrier() to make sure and wait for free_device() to finish. But, free_device() just puts the work into schedule_work(), so we don't know for sure the blkdev_put is done when we exit. It's been this way for a while, so its not holding up my pull request to Linus. But I'll fix it up. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On Fri, May 27, 2016 at 01:18:22PM +0200, David Sterba wrote: > On Thu, May 26, 2016 at 08:14:14PM -0400, Chris Mason wrote: > > On Thu, May 26, 2016 at 11:27:06AM +0200, David Sterba wrote: > > > Hi, > > > > > > please pull a few more patches that did not go to pull #1 for 4.7, minor > > > cleanups and fixes. Thanks. > > > > Thanks Dave! Trying to figure out why we're failing btrfs/011, but I > > don't see how it could be related to this bunch. I'll nail it down. > > 011 passes here, there are some unrelated soft-failures (mismatching > output with new progs). I'm now testing a branch without "btrfs: scrub: > Set bbio to NULL before calling btrfs_map_block", that seems to be the > only likely offender. I'm getting errors from btrfs fi show -d, after the very last round of device replaces. A little extra debugging: bytenr mismatch, want=4332716032, have=0 ERROR: cannot read chunk root ERROR reading /dev/vdh failed /dev/vdh Which is cute because the very next command we run fscks /dev/vdh and succeeds. So the page cache is stale and this isn't related to any of our patches. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On Thu, May 26, 2016 at 08:14:14PM -0400, Chris Mason wrote: > On Thu, May 26, 2016 at 11:27:06AM +0200, David Sterba wrote: > > Hi, > > > > please pull a few more patches that did not go to pull #1 for 4.7, minor > > cleanups and fixes. Thanks. > > Thanks Dave! Trying to figure out why we're failing btrfs/011, but I > don't see how it could be related to this bunch. I'll nail it down. 011 passes here, there are some unrelated soft-failures (mismatching output with new progs). I'm now testing a branch without "btrfs: scrub: Set bbio to NULL before calling btrfs_map_block", that seems to be the only likely offender. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7, part 2
On Thu, May 26, 2016 at 11:27:06AM +0200, David Sterba wrote: > Hi, > > please pull a few more patches that did not go to pull #1 for 4.7, minor > cleanups and fixes. Thanks. Thanks Dave! Trying to figure out why we're failing btrfs/011, but I don't see how it could be related to this bunch. I'll nail it down. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Btrfs for 4.7
On Mon, May 16, 2016 at 04:14:23PM +0200, David Sterba wrote: > Hi, > > please queue the following branch to 4.7. > > New features or user-visible changes: > * device delete by id, a v2 ioctl for device deletion (this was held back from > 4.6 pull due to possibly related crashes that haven't appeared for a long > time) > * DUP allowed on multiple-device filesystem (help in case we want to go to > SINGLE from RAID1/.. but do not want to lose all redundancy while > converting) > * GETFLAGS/SETFLAGS/GETVERSION ioctls work on 32bit > > Developer-visible changes: > * definitions that are shared with userspace are moved to the uapi directory > > Other: > * assorted bugfixes and cleanups > > The branch is merged from several topic branches, grouped by functionality > or purpose. The patches have been in for-next, most of them for a few weeks at > least, tested together with Filipe's branch. Great, thanks Dave! I've got things merged up and I'm running more tests. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
