From: Adam Manzananares <adam.manzanares@wdc.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Dan Williams <dan.j.williams@intel.com>,
Adam Manzanares <adam.manzanares@hgst.com>,
Tejun Heo <tj@kernel.org>, Hannes Reinecke <hare@suse.de>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
<mchristi@redhat.com>, Toshi Kani <toshi.kani@hpe.com>,
Ming Lei <ming.lei@canonical.com>, <sathya.prakash@broadcom.com>,
<chaitra.basappa@broadcom.com>,
<suganath-prabu.subramani@broadcom.com>,
<linux-block@vger.kernel.org>,
IDE/ATA development list <linux-ide@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
<MPT-FusionLinux.pdl@broadcom.com>,
linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v4 1/4] block: Add iocontext priority to request
Date: Thu, 13 Oct 2016 15:02:43 -0700 [thread overview]
Message-ID: <20161013220243.GA2745@hgst.com> (raw)
In-Reply-To: <068e03c4-3558-a6b1-2008-d13bde4958a1@kernel.dk>
VGhlIDEwLzEzLzIwMTYgMTQ6MDksIEplbnMgQXhib2Ugd3JvdGU6Cj4gT24gMTAvMTMvMjAxNiAw
MjowNiBQTSwgRGFuIFdpbGxpYW1zIHdyb3RlOgo+ID5PbiBUaHUsIE9jdCAxMywgMjAxNiBhdCAx
Mjo1MyBQTSwgQWRhbSBNYW56YW5hcmVzCj4gPjxhZGFtLm1hbnphbmFyZXNAaGdzdC5jb20+IHdy
b3RlOgo+ID4+UGF0Y2ggYWRkcyBhbiBhc3NvY2lhdGlvbiBiZXR3ZWVuIGlvY29udGV4dCBpb3By
aW8gYW5kIHRoZSBpb3ByaW8gb2YgYQo+ID4+cmVxdWVzdC4gVGhpcyB2YWx1ZSBpcyBzZXQgaW4g
YmxrX3JxX3NldF9wcmlvIHdoaWNoIHRha2VzIHRoZSByZXF1ZXN0IGFuZAo+ID4+dGhlIGlvYyBh
cyBhcmd1bWVudHMuIElmIHRoZSBpb2MgaXMgdmFsaWQgaW4gYmxrX3JxX3NldF9wcmlvIHRoZW4g
dGhlCj4gPj5pb3ByaW9yaXR5IG9mIHRoZSByZXF1ZXN0IGlzIHNldCBhcyB0aGUgaW9wcmlvcml0
eSBvZiB0aGUgaW9jLiBJbgo+ID4+aW5pdF9yZXF1ZXN0X2Zyb21fYmlvIGEgY2hlY2sgaXMgbWFk
ZSB0byBzZWUgaWYgdGhlIGlvcHJpbyBvZiB0aGUgYmlvIGlzCj4gPj52YWxpZCBhbmQgaWYgc28g
dGhlbiB0aGUgcmVxdWVzdCBwcmlvIGNvbWVzIGZyb20gdGhlIGJpby4KPiA+Pgo+ID4+U2lnbmVk
LW9mZi1ieTogQWRhbSBNYW56YW5hbmFyZXMgPGFkYW0ubWFuemFuYXJlc0B3ZGMuY29tPgo+ID4+
LS0tCj4gPj4gYmxvY2svYmxrLWNvcmUuYyAgICAgICB8ICA0ICsrKy0KPiA+PiBpbmNsdWRlL2xp
bnV4L2Jsa2Rldi5oIHwgMTQgKysrKysrKysrKysrKysKPiA+PiAyIGZpbGVzIGNoYW5nZWQsIDE3
IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiA+Pgo+ID4+ZGlmZiAtLWdpdCBhL2Jsb2Nr
L2Jsay1jb3JlLmMgYi9ibG9jay9ibGstY29yZS5jCj4gPj5pbmRleCAxNGQ3YzA3Li4zNjFiMWI5
IDEwMDY0NAo+ID4+LS0tIGEvYmxvY2svYmxrLWNvcmUuYwo+ID4+KysrIGIvYmxvY2svYmxrLWNv
cmUuYwo+ID4+QEAgLTExNTMsNiArMTE1Myw3IEBAIHN0YXRpYyBzdHJ1Y3QgcmVxdWVzdCAqX19n
ZXRfcmVxdWVzdChzdHJ1Y3QgcmVxdWVzdF9saXN0ICpybCwgaW50IG9wLAo+ID4+Cj4gPj4gICAg
ICAgIGJsa19ycV9pbml0KHEsIHJxKTsKPiA+PiAgICAgICAgYmxrX3JxX3NldF9ybChycSwgcmwp
Owo+ID4+KyAgICAgICBibGtfcnFfc2V0X3ByaW8ocnEsIGlvYyk7Cj4gPj4gICAgICAgIHJlcV9z
ZXRfb3BfYXR0cnMocnEsIG9wLCBvcF9mbGFncyB8IFJFUV9BTExPQ0VEKTsKPiA+Pgo+ID4+ICAg
ICAgICAvKiBpbml0IGVsdnByaXYgKi8KPiA+PkBAIC0xNjU2LDcgKzE2NTcsOCBAQCB2b2lkIGlu
aXRfcmVxdWVzdF9mcm9tX2JpbyhzdHJ1Y3QgcmVxdWVzdCAqcmVxLCBzdHJ1Y3QgYmlvICpiaW8p
Cj4gPj4KPiA+PiAgICAgICAgcmVxLT5lcnJvcnMgPSAwOwo+ID4+ICAgICAgICByZXEtPl9fc2Vj
dG9yID0gYmlvLT5iaV9pdGVyLmJpX3NlY3RvcjsKPiA+Pi0gICAgICAgcmVxLT5pb3ByaW8gPSBi
aW9fcHJpbyhiaW8pOwo+ID4+KyAgICAgICBpZiAoaW9wcmlvX3ZhbGlkKGJpb19wcmlvKGJpbykp
KQo+ID4+KyAgICAgICAgICAgICAgIHJlcS0+aW9wcmlvID0gYmlvX3ByaW8oYmlvKTsKPiA+Cj4g
PlNob3VsZCB3ZSB1c2UgaW9wcmlvX2Jlc3QoKSBoZXJlPyAgSWYgcmVxLT5pb3ByaW8gYW5kIGJp
b19wcmlvKCkKPiA+ZGlzYWdyZWUgb25lIHNpZGUgaGFzIGV4cGxpY2l0bHkgYXNrZWQgZm9yIGEg
aGlnaGVyIHByaW9yaXR5Lgo+IAo+IEl0J3MgYSBnb29kIHF1ZXN0aW9uIC0gYnV0IGlmIHByaW9y
aXR5IGhhcyBiZWVuIHNldCBpbiB0aGUgYmlvLCBpdCBtYWtlcwo+IHNlbnNlIHRoYXQgdGhhdCB3
b3VsZCB0YWtlIHByaW9yaXR5IG92ZXIgdGhlIGdlbmVyYWwgc2V0dGluZyBmb3IgdGhlCj4gdGFz
ay9pbyBjb250ZXh0LiBTbyBJIHRoaW5rIHRoZSBwYXRjaCBpcyBjb3JyZWN0IGFzLWlzLgo+IAo+
IEFkYW0sIHlvdSdsbCB3YW50IHRvIHJld3JpdGUgdGhlIGNvbW1pdCBtZXNzYWdlIHRob3VnaC4g
QSBnb29kIGNvbW1pdAo+IG1lc3NhZ2Ugc2hvdWxkIGV4cGxhaW4gV0hZIHRoZSBjaGFuZ2UgaXMg
bWFkZSwgbm90IGRldGFpbCB0aGUgY29kZQo+IGltcGxlbWVudGF0aW9uIG9mIGl0LgoKR290IGl0
IEknbGwgc2VuZCBzb21ldGhpbmcgb3V0IHNvb24uCgo+IAo+IC0tIAo+IEplbnMgQXhib2UKPiAK
ClRha2UgY2FyZSwKQWRhbQpXZXN0ZXJuIERpZ2l0YWwgQ29ycG9yYXRpb24gKGFuZCBpdHMgc3Vi
c2lkaWFyaWVzKSBFLW1haWwgQ29uZmlkZW50aWFsaXR5IE5vdGljZSAmIERpc2NsYWltZXI6CgpU
aGlzIGUtbWFpbCBhbmQgYW55IGZpbGVzIHRyYW5zbWl0dGVkIHdpdGggaXQgbWF5IGNvbnRhaW4g
Y29uZmlkZW50aWFsIG9yIGxlZ2FsbHkgcHJpdmlsZWdlZCBpbmZvcm1hdGlvbiBvZiBXREMgYW5k
L29yIGl0cyBhZmZpbGlhdGVzLCBhbmQgYXJlIGludGVuZGVkIHNvbGVseSBmb3IgdGhlIHVzZSBv
ZiB0aGUgaW5kaXZpZHVhbCBvciBlbnRpdHkgdG8gd2hpY2ggdGhleSBhcmUgYWRkcmVzc2VkLiBJ
ZiB5b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQgcmVjaXBpZW50LCBhbnkgZGlzY2xvc3VyZSwgY29w
eWluZywgZGlzdHJpYnV0aW9uIG9yIGFueSBhY3Rpb24gdGFrZW4gb3Igb21pdHRlZCB0byBiZSB0
YWtlbiBpbiByZWxpYW5jZSBvbiBpdCwgaXMgcHJvaGliaXRlZC4gSWYgeW91IGhhdmUgcmVjZWl2
ZWQgdGhpcyBlLW1haWwgaW4gZXJyb3IsIHBsZWFzZSBub3RpZnkgdGhlIHNlbmRlciBpbW1lZGlh
dGVseSBhbmQgZGVsZXRlIHRoZSBlLW1haWwgaW4gaXRzIGVudGlyZXR5IGZyb20geW91ciBzeXN0
ZW0uCg==
WARNING: multiple messages have this Message-ID (diff)
From: Adam Manzananares <adam.manzanares@wdc.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Dan Williams <dan.j.williams@intel.com>,
Adam Manzanares <adam.manzanares@hgst.com>,
Tejun Heo <tj@kernel.org>, Hannes Reinecke <hare@suse.de>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
mchristi@redhat.com, Toshi Kani <toshi.kani@hpe.com>,
Ming Lei <ming.lei@canonical.com>,
sathya.prakash@broadcom.com, chaitra.basappa@broadcom.com,
suganath-prabu.subramani@broadcom.com,
linux-block@vger.kernel.org,
IDE/ATA development list <linux-ide@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
MPT-FusionLinux.pdl@broadcom.com,
linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v4 1/4] block: Add iocontext priority to request
Date: Thu, 13 Oct 2016 15:02:43 -0700 [thread overview]
Message-ID: <20161013220243.GA2745@hgst.com> (raw)
In-Reply-To: <068e03c4-3558-a6b1-2008-d13bde4958a1@kernel.dk>
The 10/13/2016 14:09, Jens Axboe wrote:
> On 10/13/2016 02:06 PM, Dan Williams wrote:
> >On Thu, Oct 13, 2016 at 12:53 PM, Adam Manzanares
> ><adam.manzanares@hgst.com> wrote:
> >>Patch adds an association between iocontext ioprio and the ioprio of a
> >>request. This value is set in blk_rq_set_prio which takes the request and
> >>the ioc as arguments. If the ioc is valid in blk_rq_set_prio then the
> >>iopriority of the request is set as the iopriority of the ioc. In
> >>init_request_from_bio a check is made to see if the ioprio of the bio is
> >>valid and if so then the request prio comes from the bio.
> >>
> >>Signed-off-by: Adam Manzananares <adam.manzanares@wdc.com>
> >>---
> >> block/blk-core.c | 4 +++-
> >> include/linux/blkdev.h | 14 ++++++++++++++
> >> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/block/blk-core.c b/block/blk-core.c
> >>index 14d7c07..361b1b9 100644
> >>--- a/block/blk-core.c
> >>+++ b/block/blk-core.c
> >>@@ -1153,6 +1153,7 @@ static struct request *__get_request(struct request_list *rl, int op,
> >>
> >> blk_rq_init(q, rq);
> >> blk_rq_set_rl(rq, rl);
> >>+ blk_rq_set_prio(rq, ioc);
> >> req_set_op_attrs(rq, op, op_flags | REQ_ALLOCED);
> >>
> >> /* init elvpriv */
> >>@@ -1656,7 +1657,8 @@ void init_request_from_bio(struct request *req, struct bio *bio)
> >>
> >> req->errors = 0;
> >> req->__sector = bio->bi_iter.bi_sector;
> >>- req->ioprio = bio_prio(bio);
> >>+ if (ioprio_valid(bio_prio(bio)))
> >>+ req->ioprio = bio_prio(bio);
> >
> >Should we use ioprio_best() here? If req->ioprio and bio_prio()
> >disagree one side has explicitly asked for a higher priority.
>
> It's a good question - but if priority has been set in the bio, it makes
> sense that that would take priority over the general setting for the
> task/io context. So I think the patch is correct as-is.
>
> Adam, you'll want to rewrite the commit message though. A good commit
> message should explain WHY the change is made, not detail the code
> implementation of it.
Got it I'll send something out soon.
>
> --
> Jens Axboe
>
Take care,
Adam
WARNING: multiple messages have this Message-ID (diff)
From: Adam Manzananares <adam.manzanares@wdc.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Dan Williams <dan.j.williams@intel.com>,
Adam Manzanares <adam.manzanares@hgst.com>,
Tejun Heo <tj@kernel.org>, Hannes Reinecke <hare@suse.de>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
<mchristi@redhat.com>, Toshi Kani <toshi.kani@hpe.com>,
Ming Lei <ming.lei@canonical.com>, <sathya.prakash@broadcom.com>,
<chaitra.basappa@broadcom.com>,
<suganath-prabu.subramani@broadcom.com>,
<linux-block@vger.kernel.org>,
IDE/ATA development list <linux-ide@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
<MPT-FusionLinux.pdl@broadcom.com>,
linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v4 1/4] block: Add iocontext priority to request
Date: Thu, 13 Oct 2016 15:02:43 -0700 [thread overview]
Message-ID: <20161013220243.GA2745@hgst.com> (raw)
In-Reply-To: <068e03c4-3558-a6b1-2008-d13bde4958a1@kernel.dk>
The 10/13/2016 14:09, Jens Axboe wrote:
> On 10/13/2016 02:06 PM, Dan Williams wrote:
> >On Thu, Oct 13, 2016 at 12:53 PM, Adam Manzanares
> ><adam.manzanares@hgst.com> wrote:
> >>Patch adds an association between iocontext ioprio and the ioprio of a
> >>request. This value is set in blk_rq_set_prio which takes the request and
> >>the ioc as arguments. If the ioc is valid in blk_rq_set_prio then the
> >>iopriority of the request is set as the iopriority of the ioc. In
> >>init_request_from_bio a check is made to see if the ioprio of the bio is
> >>valid and if so then the request prio comes from the bio.
> >>
> >>Signed-off-by: Adam Manzananares <adam.manzanares@wdc.com>
> >>---
> >> block/blk-core.c | 4 +++-
> >> include/linux/blkdev.h | 14 ++++++++++++++
> >> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/block/blk-core.c b/block/blk-core.c
> >>index 14d7c07..361b1b9 100644
> >>--- a/block/blk-core.c
> >>+++ b/block/blk-core.c
> >>@@ -1153,6 +1153,7 @@ static struct request *__get_request(struct request_list *rl, int op,
> >>
> >> blk_rq_init(q, rq);
> >> blk_rq_set_rl(rq, rl);
> >>+ blk_rq_set_prio(rq, ioc);
> >> req_set_op_attrs(rq, op, op_flags | REQ_ALLOCED);
> >>
> >> /* init elvpriv */
> >>@@ -1656,7 +1657,8 @@ void init_request_from_bio(struct request *req, struct bio *bio)
> >>
> >> req->errors = 0;
> >> req->__sector = bio->bi_iter.bi_sector;
> >>- req->ioprio = bio_prio(bio);
> >>+ if (ioprio_valid(bio_prio(bio)))
> >>+ req->ioprio = bio_prio(bio);
> >
> >Should we use ioprio_best() here? If req->ioprio and bio_prio()
> >disagree one side has explicitly asked for a higher priority.
>
> It's a good question - but if priority has been set in the bio, it makes
> sense that that would take priority over the general setting for the
> task/io context. So I think the patch is correct as-is.
>
> Adam, you'll want to rewrite the commit message though. A good commit
> message should explain WHY the change is made, not detail the code
> implementation of it.
Got it I'll send something out soon.
>
> --
> Jens Axboe
>
Take care,
Adam
next prev parent reply other threads:[~2016-10-13 22:02 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-13 19:53 [PATCH v4 0/4] Enabling ATA Command Priorities Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` [PATCH v4 1/4] block: Add iocontext priority to request Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 20:06 ` Dan Williams
2016-10-13 20:09 ` Jens Axboe
2016-10-13 20:19 ` Dan Williams
2016-10-13 20:24 ` Jens Axboe
2016-10-13 20:59 ` Dan Williams
2016-10-13 21:07 ` Jens Axboe
2016-10-13 22:02 ` Adam Manzananares [this message]
2016-10-13 22:02 ` Adam Manzananares
2016-10-13 22:02 ` Adam Manzananares
2016-10-14 5:54 ` Hannes Reinecke
2016-10-14 5:54 ` Hannes Reinecke
2016-10-14 18:35 ` Adam Manzananares
2016-10-14 18:35 ` Adam Manzananares
2016-10-14 18:35 ` Adam Manzananares
2016-10-15 8:43 ` Hannes Reinecke
2016-10-13 19:53 ` [PATCH v4 2/4] fusion: remove iopriority handling Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 21:05 ` Sathya Prakash Veerichetty
2016-10-13 22:12 ` Adam Manzanares
2016-10-13 22:12 ` Adam Manzanares
2016-10-13 22:12 ` Adam Manzanares
2016-10-14 5:55 ` Hannes Reinecke
2016-10-14 5:55 ` Hannes Reinecke
2016-10-13 19:53 ` [PATCH v4 3/4] ata: Enabling ATA Command Priorities Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` [PATCH v4 4/4] ata: ATA Command Priority Disabled By Default Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
2016-10-13 19:53 ` Adam Manzanares
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161013220243.GA2745@hgst.com \
--to=adam.manzanares@wdc.com \
--cc=MPT-FusionLinux.pdl@broadcom.com \
--cc=adam.manzanares@hgst.com \
--cc=axboe@kernel.dk \
--cc=chaitra.basappa@broadcom.com \
--cc=dan.j.williams@intel.com \
--cc=hare@suse.de \
--cc=linux-block@vger.kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mchristi@redhat.com \
--cc=ming.lei@canonical.com \
--cc=sathya.prakash@broadcom.com \
--cc=suganath-prabu.subramani@broadcom.com \
--cc=tj@kernel.org \
--cc=toshi.kani@hpe.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.