Re: list/count mismatch warning in rcu_do_batch (possibly triggered by a btrfs bug).
On Tue, Nov 15, 2016 at 10:53:15AM +0200, Nikolay Borisov wrote: > Hello Paul, > > I'm currently going through a crashdump which seems to indicate some > memory corruption, possibly triggered by btrfs. I have several > entries such as : > > [1626691.276310] BUG: Bad page state in process fstrim pfn:230ee7 > [1626691.276488] page:ea0008c3b9c0 count:0 mapcount:0 > mapping:8801106439c8 index:0x22d3 > [1626691.276774] flags: 0x2fffc0c(referenced|uptodate) > [1626691.277084] page dumped because: non-NULL mapping > [1626691.280607] CPU: 8 PID: 303 Comm: fstrim Tainted: PW O > 4.4.26-clouder1 #3 > [1626691.280885] Hardware name: Supermicro X9DRD-iF/LF/X9DRD-iF, BIOS 3.0b > 12/05/2013 > [1626691.281163] 8803b1b97a10 812f4b79 > ea0008c3b9c0 > [1626691.281623] 81a0b20e 8803b1b97a38 8112dcc0 > > [1626691.282085] 0001 88047fffa000 8803b1b97a88 > 8112e7b6 > [1626691.282554] Call Trace: > [1626691.282727] [] dump_stack+0x67/0x9e > [1626691.282899] [] bad_page.part.64+0xb0/0x100 > [1626691.283071] [] free_pages_prepare+0x2f6/0x310 > [1626691.283244] [] free_hot_cold_page+0x35/0x1f0 > [1626691.283416] [] ? mem_cgroup_uncharge+0x29/0x30 > [1626691.283588] [] ? __page_cache_release+0x28/0x150 > [1626691.283760] [] put_page+0x40/0x50 > [1626691.283956] [] > btrfs_release_extent_buffer_page+0x6b/0x100 [btrfs] > [1626691.284259] [] release_extent_buffer+0x4d/0xc0 [btrfs] > [1626691.28] [] free_extent_buffer+0x4b/0x90 [btrfs] > [1626691.284626] [] btrfs_release_path+0x27/0x90 [btrfs] > [1626691.284814] [] __lookup_free_space_inode+0xc2/0x150 > [btrfs] > [1626691.285113] [] lookup_free_space_inode+0x5d/0xd0 > [btrfs] > [1626691.285302] [] load_free_space_cache+0x81/0x1c0 > [btrfs] > [1626691.285486] [] cache_block_group+0x1b8/0x3b0 [btrfs] > [1626691.285660] [] ? wait_woken+0xb0/0xb0 > [1626691.285842] [] btrfs_trim_fs+0xdb/0x3d0 [btrfs] > [1626691.286017] [] ? terminate_walk+0x6d/0xe0 > [1626691.286189] [] ? __might_sleep+0x52/0xb0 > [1626691.286375] [] btrfs_ioctl_fitrim+0x130/0x180 [btrfs] > [1626691.286561] [] btrfs_ioctl+0x1276/0x2710 [btrfs] > [1626691.286734] [] ? do_filp_open+0x92/0xe0 > [1626691.286905] [] ? __might_sleep+0x52/0xb0 > [1626691.287078] [] do_vfs_ioctl+0x30f/0x560 > [1626691.287248] [] ? putname+0x53/0x60 > [1626691.287420] [] SyS_ioctl+0x79/0x90 > [1626691.287591] [] entry_SYSCALL_64_fastpath+0x16/0x6e > > But eventually the following warning is triggered: > > [1626691.326167] [ cut here ] > [1626691.326344] WARNING: CPU: 1 PID: 17122 at kernel/rcu/tree.c:2733 > rcu_process_callbacks+0x5e2/0x620() > [1626691.329962] CPU: 1 PID: 17122 Comm: btrfs-transacti Tainted: PB W > O4.4.26-clouder1 #3 > [1626691.330255] Hardware name: Supermicro X9DRD-iF/LF/X9DRD-iF, BIOS 3.0b > 12/05/2013 > [1626691.330546] 88047fc23e68 812f4b79 > > [1626691.331023] 81a04a61 88047fc23ea0 81052bd6 > 81c40280 > [1626691.331504] 88047fc35d38 0005 > 88047fc35d00 > [1626691.331978] Call Trace: > [1626691.332146][] dump_stack+0x67/0x9e > [1626691.332373] [] warn_slowpath_common+0x86/0xc0 > [1626691.332548] [] warn_slowpath_null+0x1a/0x20 > [1626691.332723] [] rcu_process_callbacks+0x5e2/0x620 > [1626691.332898] [] __do_softirq+0x147/0x310 > [1626691.333072] [] irq_exit+0x5f/0x70 > [1626691.333246] [] smp_apic_timer_interrupt+0x42/0x50 > [1626691.333422] [] apic_timer_interrupt+0x89/0x90 > [1626691.333591][] ? __crc32c_le+0x65/0x110 > [1626691.333814] [] chksum_update+0x17/0x20 > [1626691.333986] [] crypto_shash_update+0x36/0x100 > [1626691.334186] [] btrfs_crc32c+0x55/0x70 [btrfs] > [1626691.334374] [] ? __btrfs_map_block+0x61e/0x10f0 > [btrfs] > [1626691.334560] [] csum_tree_block+0x6b/0x180 [btrfs] > [1626691.334735] [] ? mempool_alloc+0x5f/0x150 > [1626691.334906] [] ? mempool_alloc_slab+0x15/0x20 > [1626691.335091] [] btree_csum_one_bio.isra.41+0xc1/0xd0 > [btrfs] > [1626691.335389] [] btree_submit_bio_hook+0x4d/0x110 > [btrfs] > [1626691.335577] [] submit_one_bio+0x6e/0xa0 [btrfs] > [1626691.335762] [] submit_extent_page+0xa0/0x230 [btrfs] > [1626691.335949] [] write_one_eb.isra.36+0x175/0x200 > [btrfs] > [1626691.336137] [] ? > end_extent_buffer_writeback+0x30/0x30 [btrfs] > [1626691.336438] [] btree_write_cache_pages+0x312/0x400 > [btrfs] > [1626691.336738] [] btree_writepages+0x5d/0x70 [btrfs] > [1626691.336912] [] do_writepages+0x1e/0x30 > [1626691.337082] [] __filemap_fdatawrite_range+0xaa/0xf0 > [1626691.337255] [] filemap_fdatawrite_range+0x13/0x20 > [1626691.337440] [] btrfs_write_marked_extents+0x10c/0x130 > [btrfs] > [1626691.337733] [] > btrfs_write_and_wait_transaction.isra.22+0x49/0x90 [btrfs] > [1626691.338025] []
Re: [PATCH RESEND v8 0/3] Introduce RCU string API
On Fri, Jan 09, 2015 at 12:35:57PM -0800, Omar Sandoval wrote: Hi, everyone, Now that the merge window and the holidays are over, I've rebased this on v3.19-rc3 for the next merge window. Wasn't btrfs going to take this one? If I am supposed to take it, please split out the RCU infrastructure portion so that I can push it separately. Thanx, Paul This patch series introduces the RCU string API and cleans up the wreckage of sparse warnings which follow from it (shown here from when the patch was briefly in the btrfs integration tree): On Thu, Nov 27, 2014 at 06:45:20AM +0800, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git integration head: c7a37618b60026121255c69e042d74ae5631470c commit: 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3 [38/39] Move BTRFS RCU string to common library reproduce: # apt-get install sparse git checkout 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3 make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by ) fs/btrfs/check-integrity.c:848:25: sparse: incorrect type in argument 1 (different address spaces) fs/btrfs/check-integrity.c:848:25:expected struct rcu_string [noderef] asn:4*rcu_str fs/btrfs/check-integrity.c:848:25:got struct rcu_string *name [snip] Version 8 combines the original patch with another patch series I posted to fix these warnings, which fixes the botched __rcu annotations that caused some of the warnings and refactors the existing uses of rcustring to get rid of the rest. There's also a patch to fix an RCU misuse. Thanks! v8: Get the __rcu annotations right, clean up RCU string-related sparse noise v7: Add arguments to kernel doc for printk wrappers, use ##__VA_ARGS v6: Add header dependencies to rcustring.h v5: Rebase against v3.18-rc3 v4: Don't return anything from the printk wrappers on the assumption that printk will return void someday v3: Add __rcu annotation to relevant functions, add Paul's ack and Josh's review Omar Sandoval (3): Move BTRFS RCU string to common library btrfs: refactor btrfs_device-name updates btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO fs/btrfs/check-integrity.c | 6 +-- fs/btrfs/dev-replace.c | 19 fs/btrfs/disk-io.c | 6 +-- fs/btrfs/extent_io.c | 4 +- fs/btrfs/ioctl.c | 14 +++--- fs/btrfs/raid56.c | 2 +- fs/btrfs/rcu-string.h | 56 fs/btrfs/scrub.c | 15 --- fs/btrfs/super.c | 2 +- fs/btrfs/volumes.c | 107 ++--- fs/btrfs/volumes.h | 2 +- include/linux/rcustring.h | 97 12 files changed, 204 insertions(+), 126 deletions(-) delete mode 100644 fs/btrfs/rcu-string.h create mode 100644 include/linux/rcustring.h -- 2.2.1 -- 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: [PATCH v2 0/2] Move BTRFS RCU string to common library
On Fri, Sep 19, 2014 at 12:22:57PM -0400, Chris Mason wrote:
On 09/19/2014 12:05 PM, Paul E. McKenney wrote:
On Fri, Sep 19, 2014 at 11:47:53AM -0400, Chris Mason wrote:
On 09/19/2014 11:45 AM, Paul E. McKenney wrote:
On Fri, Sep 19, 2014 at 02:01:28AM -0700, Omar Sandoval wrote:
This patch series moves the generic RCU string library used internally
by BTRFS
to be accessible by anyone. It provides printk_in_rcu and
printk_ratelimited_in_rcu to print these strings. In order to avoid a
weird
inconsistency between the two, the first patch fixes printk_ratelimited
so it
passes on the return value from printk.
The second patch actually moves the RCU string library. Version 2 passes
on the
return values from printk{,_ratelimited} and fixes some style issues.
Omar Sandoval (2):
For the series:
Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Fine by me too, Paul, do you want to merge it in?
I would be happy to.
Are you thinking in terms of 3.18 or 3.19? These look OK either way, but
thought I should check.
Either way is fine with me. Actually this will have minor conflicts
with my current branch headed for-next, so I can resolve and send as a
stand alone pull.
There are no conflicts with RCU, just adding a file, so I am just as
happy to have you send this via your tree.
Thanx, Paul
--
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: [PATCH v2 1/2] Return a value from printk_ratelimited
On Fri, Sep 19, 2014 at 11:15:53AM -0700, Joe Perches wrote: On Fri, 2014-09-19 at 13:21 -0400, Steven Rostedt wrote: On Fri, 19 Sep 2014 02:01:29 -0700 Omar Sandoval osan...@osandov.com wrote: printk returns an integer; there's no reason for printk_ratelimited to swallow it. Except for the lack of usefulness of the return value itself. See: https://lkml.org/lkml/2009/10/7/275 When printk()'s return value is changed to void, then yes, we should clearly change this code to match that. So, I have to ask... What happened to the patch later in that series that was to remove the uses of the printk() return value? Thanx, Paul -- 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: [PATCH v2 0/2] Move BTRFS RCU string to common library
On Fri, Sep 19, 2014 at 02:01:28AM -0700, Omar Sandoval wrote:
This patch series moves the generic RCU string library used internally by
BTRFS
to be accessible by anyone. It provides printk_in_rcu and
printk_ratelimited_in_rcu to print these strings. In order to avoid a weird
inconsistency between the two, the first patch fixes printk_ratelimited so it
passes on the return value from printk.
The second patch actually moves the RCU string library. Version 2 passes on
the
return values from printk{,_ratelimited} and fixes some style issues.
Omar Sandoval (2):
For the series:
Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Return a value from printk_ratelimited
Move BTRFS RCU string to common library
fs/btrfs/check-integrity.c | 6 +--
fs/btrfs/dev-replace.c | 19 +-
fs/btrfs/disk-io.c | 6 +--
fs/btrfs/extent_io.c | 4 +-
fs/btrfs/ioctl.c | 4 +-
fs/btrfs/raid56.c | 2 +-
fs/btrfs/rcu-string.h | 56
fs/btrfs/scrub.c | 15
fs/btrfs/super.c | 2 +-
fs/btrfs/volumes.c | 14 +++
include/linux/printk.h | 4 +-
include/linux/rcustring.h | 91
++
12 files changed, 131 insertions(+), 92 deletions(-)
delete mode 100644 fs/btrfs/rcu-string.h
create mode 100644 include/linux/rcustring.h
--
2.1.0
--
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: [PATCH v2 0/2] Move BTRFS RCU string to common library
On Fri, Sep 19, 2014 at 11:47:53AM -0400, Chris Mason wrote:
On 09/19/2014 11:45 AM, Paul E. McKenney wrote:
On Fri, Sep 19, 2014 at 02:01:28AM -0700, Omar Sandoval wrote:
This patch series moves the generic RCU string library used internally by
BTRFS
to be accessible by anyone. It provides printk_in_rcu and
printk_ratelimited_in_rcu to print these strings. In order to avoid a weird
inconsistency between the two, the first patch fixes printk_ratelimited so
it
passes on the return value from printk.
The second patch actually moves the RCU string library. Version 2 passes
on the
return values from printk{,_ratelimited} and fixes some style issues.
Omar Sandoval (2):
For the series:
Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Fine by me too, Paul, do you want to merge it in?
I would be happy to.
Are you thinking in terms of 3.18 or 3.19? These look OK either way, but
thought I should check.
Thanx, Paul
--
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: [GIT PULL] adaptive spinning mutexes
On Thu, Jan 15, 2009 at 05:01:32PM -0800, Linus Torvalds wrote: On Thu, 15 Jan 2009, Paul E. McKenney wrote: On Thu, Jan 15, 2009 at 10:16:53AM -0800, Linus Torvalds wrote: IOW, if you do pre-allocation instead of holding a lock over the allocation, you win. So yes, spin-mutexes makes it easier to write the code, but it also makes it easier to just plain be lazy. In infrequently invoked code such as some error handling, lazy/simple can be a big win. Sure. I don't disagree at all. On such code we don't even care about locking. If it _really_ is fundamentally very rarely invoked. But if we're talking things like core filesystem locks, it's _really_ irritating when one of those (supposedly rare) allocation delays or the need to do IO then blocks all those (supposedly common) nice cached cases. Certainly if there was one big mutex covering all the operations, it would indeed be bad. On the other hand, if the filesystem/cache was partitioned (perhaps hashed) so that there was a large number of such locks, then if should be OK. Yes, I am making the perhaps naive assumption that hot spots such as the root inode would be in the cache. And that they would rarely collide with allocation or I/O, which might also be naive. But on this point I must defer to the filesystem folks. So I don't dispute at all that mutex with spinning performs better than a mutex, but I _do_ claim that it has some potentially huge downsides compared to a real spinlock. It may perform as well as a spinlock in the nice common case, but then when you hit the non-common case you see the difference between well-written code and badly written code. And sadly, while allocations _usually_ are nice and immediate, and while our caches _usually_ mean that we don't need to do IO, bad behavior when we do need to do IO is what really kills interactive feel. Suddenly everything else is hurting too, because they wanted that lock - even if they didn't need to do IO or allocate anything. I certainly agree that there are jobs that a spin-mutex is ill-suited for. Thanx, Paul -- 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
