On 11/20/2015 08:14 PM, Liu Bo wrote: > On Fri, Nov 20, 2015 at 07:26:45PM -0700, Jens Axboe wrote: >> On 11/20/2015 04:08 PM, Liu Bo wrote: >>> On Fri, Nov 20, 2015 at 02:30:43PM -0700, Jens Axboe wrote: >>>> On 11/20/2015 06:13 AM, Chris Mason wrote: >>>>> On Thu, Nov 19, 2015 at 05:49:37PM -0800, Liu Bo wrote: >>>>>> while xfstesting, this bug[1] is spotted by both btrfs/061 and btrfs/063, >>>>>> so those sub-stripe writes are gatherred into plug callback list and >>>>>> hopefully we can have a full stripe writes. >>>>>> >>>>>> However, while processing these plugged callbacks, it's within an atomic >>>>>> context which is provided by blk_sq_make_request() because of a get_cpu() >>>>>> in blk_mq_get_ctx(). >>>>>> >>>>>> This changes to always use btrfs_rmw_helper to complete the pending writes. >>>>>> >>>>> >>>>> Thanks Liu, but MD raid has the same troubles, we're not atomic in our unplugs. >>>>> >>>>> Jens? >>>> >>>> Yeah, blk-mq does have preemption disabled when it flushes, for the single >>>> queue setup. That's a bug. Attached is an untested patch that should fix it, >>>> can you try it? >>>> >>> >>> Although it runs into a warning one time of 50 tries, that was not atomic warning but another racy issue. >>> >>> WARNING: CPU: 2 PID: 8531 at fs/btrfs/ctree.c:1162 __btrfs_cow_block+0x431/0x610 [btrfs]() >>> >>> So overall the patch is good. >>> >>>> I'll rework this to be a proper patch, not convinced we want to add the new >>>> request before flush, that might destroy merging opportunities. I'll unify >>>> the mq/sq parts. >>> >>> That's true, xfstests didn't notice any performance difference but that cannot prove anything. >>> >>> I'll test the new patch when you send it out. >> >> Try this one, that should retain the plug issue characteristics we care >> about as well. > > The test does not complain any more, thank for the quick patch. > > Tested-by: Liu Bo Can I talk you into trying this one? It's simpler, does the same thing. We don't need to overcomplicate it, it's fine not having preempt disabled for adding to the list. -- Jens Axboe