diff mbox

commit 762380a "block: add notion of a chunk size for request merging" stops io on btrfs

Message ID 53A0F54F.2060205@fb.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Jens Axboe June 18, 2014, 2:11 a.m. UTC
On 2014-06-17 14:35, Konstantinos Skarlatos wrote:
> Hi all,
> with 3.16-rc1  rsync stops writing to my btrfs filesystem and stays at a
> D+ state.
> git bisect showed that the problematic commit is:
>
> 762380ad9322951cea4ce9d24864265f9c66a916 is the first bad commit
> commit 762380ad9322951cea4ce9d24864265f9c66a916
> Author: Jens Axboe <axboe@fb.com>
> Date:   Thu Jun 5 13:38:39 2014 -0600
>
>      block: add notion of a chunk size for request merging
>
>      Some drivers have different limits on what size a request should
>      optimally be, depending on the offset of the request. Similar to
>      dividing a device into chunks. Add a setting that allows the driver
>      to inform the block layer of such a chunk size. The block layer will
>      then prevent merging across the chunks.
>
>      This is needed to optimally support NVMe with a non-zero stripe size.
>
>      Signed-off-by: Jens Axboe <axboe@fb.com>

That's odd, should not have any effect since nobody enables stripe sizes 
in the kernel. I'll double check, perhaps it's not always being cleared.

Ah wait, does the attached help?

Comments

Konstantinos Skarlatos June 18, 2014, 7:21 a.m. UTC | #1
On 18/6/2014 5:11 ??, Jens Axboe wrote:
> On 2014-06-17 14:35, Konstantinos Skarlatos wrote:
>> Hi all,
>> with 3.16-rc1  rsync stops writing to my btrfs filesystem and stays at a
>> D+ state.
>> git bisect showed that the problematic commit is:
>>
>> 762380ad9322951cea4ce9d24864265f9c66a916 is the first bad commit
>> commit 762380ad9322951cea4ce9d24864265f9c66a916
>> Author: Jens Axboe <axboe@fb.com>
>> Date:   Thu Jun 5 13:38:39 2014 -0600
>>
>>      block: add notion of a chunk size for request merging
>>
>>      Some drivers have different limits on what size a request should
>>      optimally be, depending on the offset of the request. Similar to
>>      dividing a device into chunks. Add a setting that allows the driver
>>      to inform the block layer of such a chunk size. The block layer 
>> will
>>      then prevent merging across the chunks.
>>
>>      This is needed to optimally support NVMe with a non-zero stripe 
>> size.
>>
>>      Signed-off-by: Jens Axboe <axboe@fb.com>
>
> That's odd, should not have any effect since nobody enables stripe 
> sizes in the kernel. I'll double check, perhaps it's not always being 
> cleared.
>
> Ah wait, does the attached help?

Yes, it works! I recompiled at commit 
762380ad9322951cea4ce9d24864265f9c66a916 with your patch and it looks 
ok. Rebooted back to the unpatched kernel and the bug showed up again 
immediately.

The funny thing is that the problem only showed on my (multi-disk) btrfs 
filesystem. / which is on ext4 seems to work fine.

>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe June 18, 2014, 4:02 p.m. UTC | #2
On 2014-06-18 00:21, Konstantinos Skarlatos wrote:
> On 18/6/2014 5:11 ??, Jens Axboe wrote:
>> On 2014-06-17 14:35, Konstantinos Skarlatos wrote:
>>> Hi all,
>>> with 3.16-rc1  rsync stops writing to my btrfs filesystem and stays at a
>>> D+ state.
>>> git bisect showed that the problematic commit is:
>>>
>>> 762380ad9322951cea4ce9d24864265f9c66a916 is the first bad commit
>>> commit 762380ad9322951cea4ce9d24864265f9c66a916
>>> Author: Jens Axboe <axboe@fb.com>
>>> Date:   Thu Jun 5 13:38:39 2014 -0600
>>>
>>>      block: add notion of a chunk size for request merging
>>>
>>>      Some drivers have different limits on what size a request should
>>>      optimally be, depending on the offset of the request. Similar to
>>>      dividing a device into chunks. Add a setting that allows the driver
>>>      to inform the block layer of such a chunk size. The block layer
>>> will
>>>      then prevent merging across the chunks.
>>>
>>>      This is needed to optimally support NVMe with a non-zero stripe
>>> size.
>>>
>>>      Signed-off-by: Jens Axboe <axboe@fb.com>
>>
>> That's odd, should not have any effect since nobody enables stripe
>> sizes in the kernel. I'll double check, perhaps it's not always being
>> cleared.
>>
>> Ah wait, does the attached help?
>
> Yes, it works! I recompiled at commit
> 762380ad9322951cea4ce9d24864265f9c66a916 with your patch and it looks
> ok. Rebooted back to the unpatched kernel and the bug showed up again
> immediately.
>
> The funny thing is that the problem only showed on my (multi-disk) btrfs
> filesystem. / which is on ext4 seems to work fine.

Probably because the multi-disk setup doesn't have hw_sectors set, I'm 
guessing. But great, I'll get this upstream asap. Thanks for testing!
diff mbox

Patch

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 31e11051f1ba..713f8b62b435 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -920,7 +920,7 @@  static inline unsigned int blk_max_size_offset(struct request_queue *q,
 					       sector_t offset)
 {
 	if (!q->limits.chunk_sectors)
-		return q->limits.max_hw_sectors;
+		return q->limits.max_sectors;
 
 	return q->limits.chunk_sectors -
 			(offset & (q->limits.chunk_sectors - 1));