| Message ID | 1357461697-4219-4-git-send-email-aaron.lu@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sun, 6 Jan 2013, Aaron Lu wrote: > From: Lin Ming <ming.m.lin@intel.com> > > When a request is added: > If device is suspended or is suspending and the request is not a > PM request, resume the device. > > When the last request finishes: > Call pm_runtime_mark_last_busy() and pm_runtime_autosuspend(). > > When pick a request: > If device is resuming/suspending, then only PM request is allowed to go. > Return NULL for other cases. > > [aaron.lu@intel.com: PM request does not involve nr_pending counting] > [aaron.lu@intel.com: No need to check q->dev] > [aaron.lu@intel.com: Autosuspend when the last request finished] > Signed-off-by: Lin Ming <ming.m.lin@intel.com> > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q); > extern void blk_post_runtime_suspend(struct request_queue *q, int err); > extern void blk_pre_runtime_resume(struct request_queue *q); > extern void blk_post_runtime_resume(struct request_queue *q, int err); > + > +static inline void blk_pm_put_request(struct request *rq) > +{ > + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) { > + pm_runtime_mark_last_busy(rq->q->dev); > + pm_runtime_autosuspend(rq->q->dev); > + } > +} > + > +static inline struct request *blk_pm_peek_request( > + struct request_queue *q, struct request *rq) > +{ > + if (q->rpm_status == RPM_SUSPENDED || > + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))) > + return NULL; > + else > + return rq; > +} > + > +static inline void blk_pm_requeue_request(struct request *rq) > +{ > + if (!(rq->cmd_flags & REQ_PM)) > + rq->q->nr_pending--; > +} > + > +static inline void blk_pm_add_request(struct request_queue *q, > + struct request *rq) > +{ > + if (!(rq->cmd_flags & REQ_PM) && > + q->nr_pending++ == 0 && > + (q->rpm_status == RPM_SUSPENDED || > + q->rpm_status == RPM_SUSPENDING)) > + pm_request_resume(q->dev); > +} These routines also don't belong in include/linux. And they don't need to be marked inline. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/08/2013 01:21 AM, Alan Stern wrote: > On Sun, 6 Jan 2013, Aaron Lu wrote: > >> From: Lin Ming <ming.m.lin@intel.com> >> >> When a request is added: >> If device is suspended or is suspending and the request is not a >> PM request, resume the device. >> >> When the last request finishes: >> Call pm_runtime_mark_last_busy() and pm_runtime_autosuspend(). >> >> When pick a request: >> If device is resuming/suspending, then only PM request is allowed to go. >> Return NULL for other cases. >> >> [aaron.lu@intel.com: PM request does not involve nr_pending counting] >> [aaron.lu@intel.com: No need to check q->dev] >> [aaron.lu@intel.com: Autosuspend when the last request finished] >> Signed-off-by: Lin Ming <ming.m.lin@intel.com> >> Signed-off-by: Aaron Lu <aaron.lu@intel.com> > >> --- a/include/linux/blkdev.h >> +++ b/include/linux/blkdev.h >> @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q); >> extern void blk_post_runtime_suspend(struct request_queue *q, int err); >> extern void blk_pre_runtime_resume(struct request_queue *q); >> extern void blk_post_runtime_resume(struct request_queue *q, int err); >> + >> +static inline void blk_pm_put_request(struct request *rq) >> +{ >> + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) { >> + pm_runtime_mark_last_busy(rq->q->dev); >> + pm_runtime_autosuspend(rq->q->dev); >> + } >> +} >> + >> +static inline struct request *blk_pm_peek_request( >> + struct request_queue *q, struct request *rq) >> +{ >> + if (q->rpm_status == RPM_SUSPENDED || >> + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))) >> + return NULL; >> + else >> + return rq; >> +} >> + >> +static inline void blk_pm_requeue_request(struct request *rq) >> +{ >> + if (!(rq->cmd_flags & REQ_PM)) >> + rq->q->nr_pending--; >> +} >> + >> +static inline void blk_pm_add_request(struct request_queue *q, >> + struct request *rq) >> +{ >> + if (!(rq->cmd_flags & REQ_PM) && >> + q->nr_pending++ == 0 && >> + (q->rpm_status == RPM_SUSPENDED || >> + q->rpm_status == RPM_SUSPENDING)) >> + pm_request_resume(q->dev); >> +} > > These routines also don't belong in include/linux. And they don't need > to be marked inline. OK, will move them. What about create a new file blk-pm.c for all these block pm related code? Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 8 Jan 2013, Aaron Lu wrote: > >> --- a/include/linux/blkdev.h > >> +++ b/include/linux/blkdev.h > >> @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q); > >> extern void blk_post_runtime_suspend(struct request_queue *q, int err); > >> extern void blk_pre_runtime_resume(struct request_queue *q); > >> extern void blk_post_runtime_resume(struct request_queue *q, int err); > >> + > >> +static inline void blk_pm_put_request(struct request *rq) > >> +{ > >> + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) { > >> + pm_runtime_mark_last_busy(rq->q->dev); > >> + pm_runtime_autosuspend(rq->q->dev); > >> + } > >> +} > >> + > >> +static inline struct request *blk_pm_peek_request( > >> + struct request_queue *q, struct request *rq) > >> +{ > >> + if (q->rpm_status == RPM_SUSPENDED || > >> + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))) > >> + return NULL; > >> + else > >> + return rq; > >> +} > >> + > >> +static inline void blk_pm_requeue_request(struct request *rq) > >> +{ > >> + if (!(rq->cmd_flags & REQ_PM)) > >> + rq->q->nr_pending--; > >> +} > >> + > >> +static inline void blk_pm_add_request(struct request_queue *q, > >> + struct request *rq) > >> +{ > >> + if (!(rq->cmd_flags & REQ_PM) && > >> + q->nr_pending++ == 0 && > >> + (q->rpm_status == RPM_SUSPENDED || > >> + q->rpm_status == RPM_SUSPENDING)) > >> + pm_request_resume(q->dev); > >> +} > > > > These routines also don't belong in include/linux. And they don't need > > to be marked inline. > > OK, will move them. > > What about create a new file blk-pm.c for all these block pm related > code? Sure, go ahead if Jens doesn't mind. Alternatively, you could put each of these functions in the file where it gets used. Just as importantly, all of the public routines added in patch 2/4 to blk-core.c should have kerneldoc explaining how and where to use them. In particular, the kerneldoc for blk_pm_runtime_init() has to mention that the block runtime PM implementation works only for drivers that use request structures for their I/O; it doesn't work for drivers that use bio's directly. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 08, 2013 at 10:22:45AM -0500, Alan Stern wrote: > Just as importantly, all of the public routines added in patch 2/4 to > blk-core.c should have kerneldoc explaining how and where to use them. > In particular, the kerneldoc for blk_pm_runtime_init() has to mention > that the block runtime PM implementation works only for drivers that > use request structures for their I/O; it doesn't work for drivers that > use bio's directly. How about the following description for them? /** * blk_pm_runtime_init - Block layer runtime PM initialization routine * @q: the queue of the device * @dev: the device the queue belongs to * * Description: * Initialize runtime PM related fields for @q and start auto suspend * for @dev. Drivers that want to take advantage of request based runtime * PM should call this function after @dev has been initialized, and its * request queue @q has been allocated, and runtime PM for it is not * ready yet(either disabled/forbidden or its usage count >= 0). * * The block layer runtime PM is request based, so only works for drivers * that use request as their IO unit instead of those directly use bio's. */ /** * blk_pre_runtime_suspend - Pre runtime suspend check * @q: the queue of the device * * Description: * This function will check if runtime suspend is allowed for the device * by examining if there are any requests pending in the queue. If there * are requests pending, the device can not be runtime suspended; otherwise, * the queue's status will be updated to SUSPENDING and the driver can * proceed to suspend the device. * * For the not allowed case, we mark last busy for the device so that * runtime PM core will try to autosuspend it some time later. * * This function should be called in the device's runtime suspend callback, * before its runtime suspend function is called. * * Return: * 0 - OK to runtime suspend the device * -EBUSY - Device should not be runtime suspended */ /** * blk_post_runtime_suspend - Post runtime suspend processing * @q: the queue of the device * @err: return value of the device's runtime suspend function * * Description: * Update the queue's runtime status according to the return value of the * device's runtime suspend function. * * This function should be called in the device's runtime suspend callback, * after its runtime suspend function is called. */ /** * blk_pre_runtime_resume - Pre runtime resume processing * @q: the queue of the device * * Description: * Update the queue's runtime status to RESUMING in preparation for the * runtime resume of the device. * * This function should be called in the device's runtime resume callback, * before its runtime resume function is called. */ /** * blk_post_runtime_resume - Post runtime resume processing * @q: the queue of the device * @err: return value of the device's runtime resume function * * Description: * Update the queue's runtime status according to the return value of the * device's runtime resume function. If it is successfully resumed, process * the requests that are queued into the device's queue when it is resuming * and then mark last busy and initiate autosuspend for it. * * This function should be called in the device's runtime resume callback, * after its runtime resume function is called. */ Please feel free to suggest, thanks. -Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 14 Jan 2013, Aaron Lu wrote: > On Tue, Jan 08, 2013 at 10:22:45AM -0500, Alan Stern wrote: > > Just as importantly, all of the public routines added in patch 2/4 to > > blk-core.c should have kerneldoc explaining how and where to use them. > > In particular, the kerneldoc for blk_pm_runtime_init() has to mention > > that the block runtime PM implementation works only for drivers that > > use request structures for their I/O; it doesn't work for drivers that > > use bio's directly. > > How about the following description for them? Overall this is very good. > /** > * blk_pm_runtime_init - Block layer runtime PM initialization routine > * @q: the queue of the device > * @dev: the device the queue belongs to > * > * Description: > * Initialize runtime PM related fields for @q and start auto suspend > * for @dev. Drivers that want to take advantage of request based runtime > * PM should call this function after @dev has been initialized, and its > * request queue @q has been allocated, and runtime PM for it is not > * ready yet(either disabled/forbidden or its usage count >= 0). > * > * The block layer runtime PM is request based, so only works for drivers > * that use request as their IO unit instead of those directly use bio's. > */ > > /** > * blk_pre_runtime_suspend - Pre runtime suspend check > * @q: the queue of the device > * > * Description: > * This function will check if runtime suspend is allowed for the device > * by examining if there are any requests pending in the queue. If there > * are requests pending, the device can not be runtime suspended; otherwise, > * the queue's status will be updated to SUSPENDING and the driver can > * proceed to suspend the device. > * > * For the not allowed case, we mark last busy for the device so that > * runtime PM core will try to autosuspend it some time later. > * > * This function should be called in the device's runtime suspend callback, > * before its runtime suspend function is called. This doesn't quite make sense, because the runtime_suspend callback _is_ the runtime-suspend function. How about "... should be called near the start of the device's runtime_suspend callback."? A similar comment applies to the other functions. > * > * Return: > * 0 - OK to runtime suspend the device > * -EBUSY - Device should not be runtime suspended > */ > > /** > * blk_post_runtime_suspend - Post runtime suspend processing > * @q: the queue of the device > * @err: return value of the device's runtime suspend function > * > * Description: > * Update the queue's runtime status according to the return value of the > * device's runtime suspend function. > * > * This function should be called in the device's runtime suspend callback, > * after its runtime suspend function is called. > */ > > /** > * blk_pre_runtime_resume - Pre runtime resume processing > * @q: the queue of the device > * > * Description: > * Update the queue's runtime status to RESUMING in preparation for the > * runtime resume of the device. > * > * This function should be called in the device's runtime resume callback, > * before its runtime resume function is called. > */ > > /** > * blk_post_runtime_resume - Post runtime resume processing > * @q: the queue of the device > * @err: return value of the device's runtime resume function > * > * Description: > * Update the queue's runtime status according to the return value of the > * device's runtime resume function. If it is successfully resumed, process > * the requests that are queued into the device's queue when it is resuming > * and then mark last busy and initiate autosuspend for it. > * > * This function should be called in the device's runtime resume callback, > * after its runtime resume function is called. > */ > > Please feel free to suggest, thanks. I would hypenate some of these words, such as "runtime-PM-related fields" or "request-based runtime PM". Also, "runtime_suspend" and "runtime_resume" generally should have either a '_' or a '-'. But that's a very minor point; your descriptions are quite good. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-core.c b/block/blk-core.c index 6fc24bb..93c1461 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1274,6 +1274,8 @@ void __blk_put_request(struct request_queue *q, struct request *req) if (unlikely(--req->ref_count)) return; + blk_pm_put_request(req); + elv_completed_request(q, req); /* this is a bio leak */ @@ -2080,6 +2082,11 @@ struct request *blk_peek_request(struct request_queue *q) int ret; while ((rq = __elv_next_request(q)) != NULL) { + + rq = blk_pm_peek_request(q, rq); + if (!rq) + break; + if (!(rq->cmd_flags & REQ_STARTED)) { /* * This is the first time the device driver diff --git a/block/elevator.c b/block/elevator.c index 9edba1b..61e9b49 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -544,6 +544,8 @@ void elv_requeue_request(struct request_queue *q, struct request *rq) rq->cmd_flags &= ~REQ_STARTED; + blk_pm_requeue_request(rq); + __elv_add_request(q, rq, ELEVATOR_INSERT_REQUEUE); } @@ -566,6 +568,8 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where) { trace_block_rq_insert(q, rq); + blk_pm_add_request(q, rq); + rq->q = q; if (rq->cmd_flags & REQ_SOFTBARRIER) { diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index a96e144..884c405 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q); extern void blk_post_runtime_suspend(struct request_queue *q, int err); extern void blk_pre_runtime_resume(struct request_queue *q); extern void blk_post_runtime_resume(struct request_queue *q, int err); + +static inline void blk_pm_put_request(struct request *rq) +{ + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) { + pm_runtime_mark_last_busy(rq->q->dev); + pm_runtime_autosuspend(rq->q->dev); + } +} + +static inline struct request *blk_pm_peek_request( + struct request_queue *q, struct request *rq) +{ + if (q->rpm_status == RPM_SUSPENDED || + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))) + return NULL; + else + return rq; +} + +static inline void blk_pm_requeue_request(struct request *rq) +{ + if (!(rq->cmd_flags & REQ_PM)) + rq->q->nr_pending--; +} + +static inline void blk_pm_add_request(struct request_queue *q, + struct request *rq) +{ + if (!(rq->cmd_flags & REQ_PM) && + q->nr_pending++ == 0 && + (q->rpm_status == RPM_SUSPENDED || + q->rpm_status == RPM_SUSPENDING)) + pm_request_resume(q->dev); +} #else static inline void blk_pm_runtime_init(struct request_queue *q, struct device *dev) {} @@ -984,6 +1018,13 @@ static inline int blk_pre_runtime_suspend(struct request_queue *q) static inline void blk_post_runtime_suspend(struct request_queue *q, int err) {} static inline void blk_pre_runtime_resume(struct request_queue *q) {} static inline void blk_post_runtime_resume(struct request_queue *q, int err) {} + +static inline void blk_pm_put_request(struct request *rq) {} +static inline struct request *blk_pm_peek_request( + struct request_queue *q, struct request *rq) { return rq; } +static inline void blk_pm_requeue_request(struct request *rq) {} +static inline void blk_pm_add_request(struct request_queue *q, + struct request *req) {} #endif /*