Changing the way MMC block request ends

Changing the way MMC block request ends

[Subhash Jadavani]

Hi,

For completing any block request, MMC block driver is calling:
       spin_lock_irq(queue-lock)
       __blk_end_request()
       spin_unlock_irq(queue-lock)

But if we analyze the sources of latency in kernel using ftrace, __blk_end_request() function seems to hold a spinlock with interrupts disabled for up to 6.5 ms sometimes. __blk_end_request() calls couple of functions and ftrace output shows that blk_update_bidi_request() function is almost taking 6ms. So I was wondering why can't we use the blk_end_request() rather than __blk_end_request(). Both function does the same thing except blk_end_request() doesn't take up the spinlock while calling the blk_update_bidi_request(). Is there any race condition which could occur if we call blk_update_bidi_request() without queue lock? 

I looked into blk_update_bidi_request() function and it mainly updates bio's of a request and doesn't look to do any manipulation with request queue structure of block device. There are many block drivers (SCSI, IDE etc .) other than MMC uses blk_end_request() rather than __blk_end_request(). Was there any special reason we are using __blk_end_request() in MMC block driver? If there is no specific reason, I would like to post a patch which would make MMC driver to use blk_end_request().

Let me know your thoughts on this.

Regards,
Subhash

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

RE: Changing the way MMC block request ends

[Subhash Jadavani]

Hi Chris,

Any thoughts or suggestions on this?

Regards,
Subhash

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Subhash Jadavani
> Sent: Thursday, April 05, 2012 11:16 AM
> To: linux-mmc@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Subject: Changing the way MMC block request ends
> 
> Hi,
> 
> For completing any block request, MMC block driver is calling:
>        spin_lock_irq(queue-lock)
>        __blk_end_request()
>        spin_unlock_irq(queue-lock)
> 
> But if we analyze the sources of latency in kernel using ftrace,
> __blk_end_request() function seems to hold a spinlock with interrupts
> disabled for up to 6.5 ms sometimes. __blk_end_request() calls couple of
> functions and ftrace output shows that blk_update_bidi_request() function
> is almost taking 6ms. So I was wondering why can't we use the
> blk_end_request() rather than __blk_end_request(). Both function does the
> same thing except blk_end_request() doesn't take up the spinlock while
> calling the blk_update_bidi_request(). Is there any race condition which
> could occur if we call blk_update_bidi_request() without queue lock?
> 
> I looked into blk_update_bidi_request() function and it mainly updates bio's
> of a request and doesn't look to do any manipulation with request queue
> structure of block device. There are many block drivers (SCSI, IDE etc .) other
> than MMC uses blk_end_request() rather than __blk_end_request(). Was
> there any special reason we are using __blk_end_request() in MMC block
> driver? If there is no specific reason, I would like to post a patch which would
> make MMC driver to use blk_end_request().
> 
> Let me know your thoughts on this.
> 
> Regards,
> Subhash
> 
> --
> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: Changing the way MMC block request ends

[Chris Ball]

Hi Subhash,

On Fri, Apr 06 2012, Subhash Jadavani wrote:
> I looked into blk_update_bidi_request() function and it mainly updates bio's
> of a request and doesn't look to do any manipulation with request queue
> structure of block device. There are many block drivers (SCSI, IDE etc .) other
> than MMC uses blk_end_request() rather than __blk_end_request(). Was
> there any special reason we are using __blk_end_request() in MMC block
> driver? If there is no specific reason, I would like to post a patch which would
> make MMC driver to use blk_end_request().

It looks like we've been using __blk_end_request() since fd539832c in
2007, which is as long as we've been using the blk_end_request API.

I haven't looked into it more closely, but it seems so far like there
is no specific reason, and you should go ahead and post a patch along
with your experimental results.  Thanks!

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

RE: Changing the way MMC block request ends

[Subhash Jadavani]

Hi Chris,

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Chris Ball
> Sent: Friday, April 06, 2012 7:24 PM
> To: Subhash Jadavani
> Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org
> Subject: Re: Changing the way MMC block request ends
> 
> Hi Subhash,
> 
> On Fri, Apr 06 2012, Subhash Jadavani wrote:
> > I looked into blk_update_bidi_request() function and it mainly updates
> > bio's of a request and doesn't look to do any manipulation with
> > request queue structure of block device. There are many block drivers
> > (SCSI, IDE etc .) other than MMC uses blk_end_request() rather than
> > __blk_end_request(). Was there any special reason we are using
> > __blk_end_request() in MMC block driver? If there is no specific
> > reason, I would like to post a patch which would make MMC driver to use
> blk_end_request().
> 
> It looks like we've been using __blk_end_request() since fd539832c in
2007,
> which is as long as we've been using the blk_end_request API.
> 
> I haven't looked into it more closely, but it seems so far like there is
no
> specific reason, and you should go ahead and post a patch along with your
> experimental results.  Thanks!

Thanks. Sure, will post the patch along with my test results.

Regards,
Subhash

> 
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html