Message-ID: <f35f04238ad2e00e976d5f99b4b29648.squirrel@www.codeaurora.org>
Date:	Thu, 26 Jul 2012 11:54:54 -0700 (PDT)
From:	merez@...eaurora.org
To:	"S, Venkatraman" <svenkatr@...com>
Cc:	merez@...eaurora.org, "Chris Ball" <cjb@...top.org>,
	"Muthu Kumar" <muthu.lkml@...il.com>, linux-mmc@...r.kernel.org,
	linux-arm-msm@...r.kernel.org,
	"open list" <linux-kernel@...r.kernel.org>,
	"Seungwon Jeon" <tgih.jun@...sung.com>
Subject: Re: [PATCH v2 1/1] mmc: block: Add write packing control


On Thu, July 26, 2012 8:28 am, S, Venkatraman wrote:
> On Tue, Jul 24, 2012 at 2:14 PM,  <merez@...eaurora.org> wrote:
>> On Mon, July 23, 2012 5:22 am, S, Venkatraman wrote:
>>> On Mon, Jul 23, 2012 at 5:13 PM,  <merez@...eaurora.org> wrote:
>>>> On Wed, July 18, 2012 12:26 am, Chris Ball wrote:
>>>>> Hi,  [removing Jens and the documentation list, since now we're
>> talking about the MMC side only]
>>>>> On Wed, Jul 18 2012, merez@...eaurora.org wrote:
>>>>>> Is there anything else that holds this patch from being pushed to
>>>> mmc-next?
>>>>> Yes, I'm still uncomfortable with the write packing patchsets for a
>>>> couple of reasons, and I suspect that the sum of those reasons means
>>>> that
>>>> we should probably plan on holding off merging it until after 3.6.
>>>>> Here are the open issues; please correct any misunderstandings: With
>> Seungwon's patchset ("Support packed write command"):
>>>>> * I still don't have a good set of representative benchmarks showing
>>>>>   what kind of performance changes come with this patchset.  It seems
>>>> like we've had a small amount of testing on one controller/eMMC part
>>>> combo
>>>> from Seungwon, and an entirely different test from Maya, and the
>> results
>>>> aren't documented fully anywhere to the level of describing what the
>> hardware was, what the test was, and what the results were before and
>> after the patchset.
>>>> Currently, there is only one card vendor that supports packed
>>>> commands.
>> Following are our sequential write (LMDD) test results on 2 of our
>> targets
>>>> (in MB/s):
>>>>                        No packing        packing
>>>> Target 1 (SDR 50MHz)     15               25
>>>> Target 2 (DDR 50MHz)     20               30
>>>>> With the reads-during-writes regression:
>>>>> * Venkat still has open questions about the nature of the read
>>>>>   regression, and thinks we should understand it with blktrace before
>>>> trying to fix it.  Maya has a theory about writes overwhelming reads,
>>>> but
>>>> Venkat doesn't understand why this would explain the observed
>>>> bandwidth drop.
>>>> The degradation of read due to writes is not a new behavior and exists
>> also without the write packing feature (which only increases the
>> degradation). Our investigation of this phenomenon led us to the
>> Conclusion that a new scheduling policy should be used for mobile
>> devices,
>>>> but this is not related to the current discussion of the write packing
>> feature.
>>>> The write packing feature increases the degradation of read due to
>> write
>>>> since it allows the MMC to fetch many write requests in a row, instead
>>>> of
>>>> fetching only one at a time.  Therefore some of the read requests will
>> have to wait for the completion of more write requests before they can
>> be
>>>> issued.
>>>
>>> I am a bit puzzled by this claim. One thing I checked carefully when
>> reviewing write packing patches from SJeon was that the code didn't
>> plough through a mixed list of reads and writes and selected only
>> writes.
>>> This section of the code in "mmc_blk_prep_packed_list()", from v8
>> patchset..
>>> <Quote>
>>> +               if (rq_data_dir(cur) != rq_data_dir(next)) {
>>> +                       put_back = 1;
>>> +                       break;
>>> +               }
>>> </Quote>
>>>
>>> means that once a read is encountered in the middle of write packing,
>> the packing is stopped at that point and it is executed. Then the next
>> blk_fetch_request should get the next read and continue as before.
>>>
>>> IOW, the ordering of reads and writes is _not_ altered when using
>>> packed
>> commands.
>>> For example if there were 5 write requests, followed by 1 read,
>>> followed by 5 more write requests in the request_queue, the first 5
>> writes will be executed as one "packed command", then the read will be
>> executed, and then the remaining 5 writes will be executed as one
>> "packed command". So the read does not have to wait any more than it
>> waited before (packing feature)
>>
>> Let me try to better explain with your example.
>> Without packing the MMC layer will fetch 2 write requests and wait for
>> the
>> first write request completion before fetching another write request.
>> During this time the read request could be inserted into the CFQ and
>> since
>> it has higher priority than the async write it will be dispatched in the
>> next fetch. So, the result would be 2 write requests followed by one
>> read
>> request and the read would have to wait for completion of only 2 write
>> requests.
>> With packing, all the 5 write requests will be fetched in a row, and
>> then
>> the read will arrive and be dispatched in the next fetch. Then the read
>> will have to wait for the completion of 5 write requests.
>>
>> Few more clarifications:
>> Due to the plug list mechanism in the block layer the applications can
>> "aggregate" several requests to be inserted into the scheduler before
>> waking the MMC queue thread.
>> This leads to a situation where there are several write requests in the
>> CFQ queue when MMC starts to do the fetches.
>>
>> If the read was inserted while we are building the packed command then I
>> agree that we should have seen less effect on the read performance.
>> However, the write packing statistics show that in most of the cases the
>> packing stopped due to an empty queue, meaning that the read was
>> inserted
>> to the CFQ after all the pending write requests were fetched and packed.
>>
>> Following is an example for write packing statistics of a READ/WRITE
>> parallel scenario:
>> write packing statistics:
>> Packed 1 reqs - 448 times
>> Packed 2 reqs - 38 times
>> Packed 3 reqs - 23 times
>> Packed 4 reqs - 30 times
>> Packed 5 reqs - 14 times
>> Packed 6 reqs - 8 times
>> Packed 7 reqs - 4 times
>> Packed 8 reqs - 1 times
>> Packed 10 reqs - 1 times
>> Packed 34 reqs - 1 times
>> stopped packing due to the following reasons:
>> 2 times: wrong data direction (meaning a READ was fetched and stopped
>> the
>> packing)
>> 1 times: flush or discard
>> 565 times: empty queue (meaning blk_fetch_request returned NULL)
>>
>>>
>>> And I requested blktrace to confirm that this is indeed the behaviour.
>>
>> The trace logs show that in case of no packing, there are maximum of 3-4
>> requests issued before a read request, while with packing there are also
>> cases of 6 and 7 requests dispatched before a read request.
>>
>> I'm waiting for an approval for sharing the block trace logs.
>> Since this is a simple test to run you can collect the trace logs and
>> let
>> us know if you reach other conclusions.
>>
> Thanks for the brief. I don't have the eMMC4.5 device with me yet, so
> I can't reproduce the result.

I sent the trace logs of both packing and non packing. Please let me know
if you have additional questions after reviewing them.

The problem you describe is most likely
> applicable
> to any block device driver with a large queue depth ( any queue depth >1).
> I'll check to see what knobs in block affect the result.
> Speaking of it, what is the host controller you use to test this ?

The controller I use is msm_sdcc.

> I was wondering if host->max_seg_size is taken into account while packed
> command
> is in use. If not, shouldn't it be ?  - it could act as a better
> throttle for "packing density".

The max segments (which is calculated from host->max_seg_size) is taking
into account when preparing the packed list (so that the whole packed
won't exceed the max number of segments).
I'm not sure I understand how host->max_seg_size can be used as a throttle
for "packing density". Can you please explain?

>
> Thanks,
> Venkat.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Maya
-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists