On Aug 7, 2013, at 7:35 PM, Andrew Morton wrote:
> On Wed, 7 Aug 2013 19:27:40 -0400 Ed Cashin <ecas...@coraid.com> wrote:
>
>> On Aug 7, 2013, at 5:26 PM, Andrew Morton wrote:
>>
>>> On Wed, 7 Aug 2013 17:21:32 -0400 Ed Cashin <ecas...@coraid.com> wrote:
>> ...
>>>> Sorry. I tried to cover that but maybe should have separated it more
>>>> clearly from the running text.
>>>>
>>>> The patch changes the current behavior encountered with direct I/O and aoe:
>>>>
>>>> aoe can BUG when direct I/O used
>>>>
>>>> ... to a new behavior:
>>>>
>>>> aoe does not BUG when direct I/O is used
>>>>
>>>> And the user will never know that instead aoe is manipulating the _count
>>>> of the head page, even though that's what's happening with this patch
>>>> applied.
>>>>
>>>> (The user-visible effects that motivated the older patch that added the
>>>> BUG to aoe when page tails are used for block I/O was a VM_BUG_ON that
>>>> occurred when the aoe driver manipulated the _count of a compound page
>>>> tail.)
>>>
>>> OK, I added "Fix a BUG which can trigger when direct-IO is used with
>>> AOE". Which kernel versions are affected?
>>
>>
>> That BUG was added to the aoe driver in a commit that first appears in v3.7.
>> It persists until this patch adds the code that handles the situation that
>> the BUG was intended to detect.
>>
>> [ecashin@marino linux]$ git describe --contains --match 'v*'
>> 69cf2d85de773d998798e47e3335b85e5645d157
>> v3.7-rc1~110^2~22
>> [ecashin@marino linux]$
>>
>
> ah, is it this?
>
> +bio_pageinc(struct bio *bio)
> +{
> + struct bio_vec *bv;
> + struct page *page;
> + int i;
> +
> + bio_for_each_segment(bv, bio, i) {
> + page = bv->bv_page;
> + /* Non-zero page count for non-head members of
> + * compound pages is no longer allowed by the kernel,
> + * but this has never been seen here.
> + */
> + if (unlikely(PageCompound(page)))
> + if (compound_trans_head(page) != page) {
> + pr_crit("page tail used for block I/O\n");
> + BUG();
> + }
>
> But get_page/put_page against a tail page of a compound page should
> Just Work. The core MM will hunt down the head page and manipulate its
> refcount.
>
> Perhaps the problem here is that AOE is diving under the covers and is
> using low-level page refcount alterations:
>
> + atomic_inc(&page->_count);
>
> Why does AOE do this? It would be better if it were to use the
> official published MM interfaces. If those interfaces need
> alteration/augmentation then we can do that.
The get_page cannot be used when the _count is 0 (which is exactly when we need
to increment it), because it will VM_BUG_ON.
/*
* Getting a normal page or the head of a compound page
* requires to already have an elevated page->_count.
*/
VM_BUG_ON(atomic_read(&page->_count) <= 0);
(And seeing the code reminds me that I forgot to include atomic_read in my last
email's proposal. LKML + dinner isn't a good mix. ;)
--
Ed Cashin
ecas...@coraid.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
