RE: [PATCH V3 2/2] scsi: ufs: Set fDeviceInit flag to initiate device initialization
> Hi,
>
> Sorry for the late response. I had a few days off.
>
> On Thu, July 11, 2013, Dolev Raviv wrote:
>> > On Tuesday, July 09, 2013, Sujit Reddy Thumma wrote:
>> >> From: Dolev Raviv
>> >> Allow UFS device to complete its initialization and accept
>> >> SCSI commands by setting fDeviceInit flag. The device may take
>> >> time for this operation and hence the host should poll until
>> >> fDeviceInit flag is toggled to zero. This step is mandated by
>> >> UFS device specification for device initialization completion.
>> >> Signed-off-by: Dolev Raviv
>> >> Signed-off-by: Sujit Reddy Thumma
>> >> ---
>> >> drivers/scsi/ufs/ufs.h| 88 +-
>> >> drivers/scsi/ufs/ufshcd.c | 292
>> >> -
>> >> drivers/scsi/ufs/ufshcd.h | 14 ++
>> >> drivers/scsi/ufs/ufshci.h |2 +-
>> >> 4 files changed, 390 insertions(+), 6 deletions(-)
>> >> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>> >> index 14c0a4e..db5bde4 100644
>> >> --- a/drivers/scsi/ufs/ufs.h
>> >> +++ b/drivers/scsi/ufs/ufs.h
>> >> @@ -43,6 +43,8 @@
>> >> #define GENERAL_UPIU_REQUEST_SIZE 32
>> >> #define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE((ALIGNED_UPIU_SIZE) - \
>> >> (GENERAL_UPIU_REQUEST_SIZE))
>> >> +#define QUERY_OSF_SIZE ((GENERAL_UPIU_REQUEST_SIZE) - \
>> >> + (sizeof(struct utp_upiu_header)))
>> >> #define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
>> >> cpu_to_be32((byte3 << 24) | (byte2 << 16) |\
>> >> @@ -68,7 +70,7 @@ enum {
>> >> UPIU_TRANSACTION_COMMAND= 0x01,
>> >> UPIU_TRANSACTION_DATA_OUT = 0x02,
>> >> UPIU_TRANSACTION_TASK_REQ = 0x04,
>> >> - UPIU_TRANSACTION_QUERY_REQ = 0x26,
>> >> + UPIU_TRANSACTION_QUERY_REQ = 0x16,
>> >> };
>> >> /* UTP UPIU Transaction Codes Target to Initiator */
>> >> @@ -97,8 +99,19 @@ enum {
>> >> UPIU_TASK_ATTR_ACA = 0x03,
>> >> };
>> >> -/* UTP QUERY Transaction Specific Fields OpCode */
>> >> +/* UPIU Query request function */
>> >> enum {
>> >> + UPIU_QUERY_FUNC_STANDARD_READ_REQUEST = 0x01,
>> >> + UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
>> >> +};
>> >> +
>> >> +/* Flag idn for Query Requests*/
>> >> +enum flag_idn {
>> >> + QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
>> >> +};
>> >> +
>> >> +/* UTP QUERY Transaction Specific Fields OpCode */
>> >> +enum query_opcode {
>> >> UPIU_QUERY_OPCODE_NOP = 0x0,
>> >> UPIU_QUERY_OPCODE_READ_DESC = 0x1,
>> >> UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
>> >> @@ -110,6 +123,21 @@ enum {
>> >> UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
>> >> };
>> >> +/* Query response result code */
>> >> +enum {
>> >> + QUERY_RESULT_SUCCESS= 0x00,
>> >> + QUERY_RESULT_NOT_READABLE = 0xF6,
>> >> + QUERY_RESULT_NOT_WRITEABLE = 0xF7,
>> >> + QUERY_RESULT_ALREADY_WRITTEN= 0xF8,
>> >> + QUERY_RESULT_INVALID_LENGTH = 0xF9,
>> >> + QUERY_RESULT_INVALID_VALUE = 0xFA,
>> >> + QUERY_RESULT_INVALID_SELECTOR = 0xFB,
>> >> + QUERY_RESULT_INVALID_INDEX = 0xFC,
>> >> + QUERY_RESULT_INVALID_IDN= 0xFD,
>> >> + QUERY_RESULT_INVALID_OPCODE = 0xFE,
>> >> + QUERY_RESULT_GENERAL_FAILURE= 0xFF,
>> >> +};
>> >> +
>> >> /* UTP Transfer Request Command Type (CT) */
>> >> enum {
>> >> UPIU_COMMAND_SET_TYPE_SCSI = 0x0,
>> >> @@ -127,6 +155,7 @@ enum {
>> >> MASK_SCSI_STATUS= 0xFF,
>> >> MASK_TASK_RESPONSE = 0xFF00,
>> >> MASK_RSP_UPIU_RESULT= 0x,
>> >> + MASK_QUERY_DATA_SEG_LEN = 0x,
>> >> };
>> >> /* Task management service response */
>> >> @@ -160,13 +189,40 @@ struct utp_upiu_cmd {
>> >> };
>> >> /**
>> >> + * struct utp_upiu_query - upiu request buffer structure for
>> >> + * query request.
>> >> + * @opcode: command to perform B-0
>> >> + * @idn: a value that indicates the particular type of data B-1 + *
>> @index: Index to further identify data B-2
>> >> + * @selector: Index to further identify data B-3
>> >> + * @reserved_osf: spec reserved field B-4,5
>> >> + * @length: number of descriptor bytes to read/write B-6,7
>> >> + * @value: Attribute value to be written DW-5
>> >> + * @reserved: spec reserved DW-6,7
>> >> + */
>> >> +struct utp_upiu_query {
>> >> + u8 opcode;
>> >> + u8 idn;
>> >> + u8 index;
>> >> + u8 selector;
>> >> + u16 reserved_osf;
>> >> + u16 length;
>> >> + u32 value;
>> >> + u32 reserved[2];
>> >> +};
>> >> +
>> >> +/**
>> >> * struct utp_upiu_req - general upiu request structure
>> >> * @header:UPIU header structure DW-0 to DW-2
>> >> * @sc: fields structure for scsi command DW-3 to DW-7
>> >> + * @qr: fields structure for query request DW-3 to DW-7
>> >> */
>> >> struct utp_upiu_req {
>> >> struct utp_upiu_header header;
>> >> - struct utp_upiu_cmd sc;
>> >> + union {
>> >> + struct utp_upiu_cmd sc;
>> >> + str
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: > On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: > > [7.927818] scsi_execute(): Calling blk_mq_free_request >>> > > [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS > > CC03 PQ: 0 ANSI: 5 > > > > OK, so INQUIRY response payload is looking as expected here. > > Yep. It is not on the top of my head, but I remember something like INQUIRYs > are emulated and thus do not have payload. > > > [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. > > [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 B) > > [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks > > > > Strange.. READ_CAPACITY appears to be returning a payload as zeros..? > > Yep. Because blk_execute_rq() does not put the proper callback and data do > not get copied from sg's to bounce buffer. That is why I tried to use > blk_mq_execute_rq() instead. Once I do that, data start getting read and > booting stops elsewhere. Mm. The call to blk_queue_bounce() exists within blk_mq_make_request(), but AFAICT this should still be getting invoked regardless of if the struct request is dispatched into blk-mq via the modified blk_execute_rq() -> blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly via blk_mq_execute_rq().. Jens..? > > Of course, I was suspecting that change alone is not valid and wondered > about the status of scsi-mq in the first place, and if more changes are > coming. Most certainly. ;) > > So I it turns out "req->errors + req->resid_len" issue (you described > earlier) needs to be addressed before going forward with libata (only?). AFAICT, getting an initial conversion of libata up does not depend upon this specific issue being addressed first. I could be wrong however.. > > > Not sure why yet some control CDBs are getting back the expected > > payload, while others are returning zeros.. > > Bio buffers do not get updated from callback. > > > Also, looking at the included stack back-trace: > > [...] > > Thanks a lot for these and other your comments, Nicholas! > Sure. I should have a few extra cycles to hack on this over the weekend. Also, thinking about this some more, trying to convert ahci to scsi-mq first (using QEMU emulation), while keeping a rootfs on PIIX_IDE might make debugging slightly easier.. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, 2013-07-18 at 11:51 -0700, Nicholas A. Bellinger wrote: > On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: > > On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: > > > [7.927818] scsi_execute(): Calling blk_mq_free_request >>> > > > [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS > > > CC03 PQ: 0 ANSI: 5 > > > > > > OK, so INQUIRY response payload is looking as expected here. > > > > Yep. It is not on the top of my head, but I remember something like INQUIRYs > > are emulated and thus do not have payload. > > > > > [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. > > > [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 B) > > > [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks > > > > > > Strange.. READ_CAPACITY appears to be returning a payload as zeros..? > > > > Yep. Because blk_execute_rq() does not put the proper callback and data do > > not get copied from sg's to bounce buffer. That is why I tried to use > > blk_mq_execute_rq() instead. Once I do that, data start getting read and > > booting stops elsewhere. > > Mm. > > The call to blk_queue_bounce() exists within blk_mq_make_request(), but > AFAICT this should still be getting invoked regardless of if the struct > request is dispatched into blk-mq via the modified blk_execute_rq() -> > blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly > via blk_mq_execute_rq().. > > Jens..? > Actually sorry, your right. A call to blk_mq_insert_request() for REQ_TYPE_BLOCK_PC will not invoke blk_queue_bounce() located near the top of blk_mq_execute_rq(), which means that only REQ_TYPE_FS is currently using bounce buffers, if required. Need to think a bit more about what to do here for REQ_TYPE_BLOCK_PC bounce buffer special case with blk_execute_rq(), but I'm thinking that blk_mq_execute_rq() should really not be used here.. Jens..? --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote: > On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: >> On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: >>> [7.927818] scsi_execute(): Calling blk_mq_free_request >>> >>> [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS >>> CC03 PQ: 0 ANSI: 5 >>> >>> OK, so INQUIRY response payload is looking as expected here. >> >> Yep. It is not on the top of my head, but I remember something like INQUIRYs >> are emulated and thus do not have payload. >> >>> [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. >>> [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 B) >>> [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks >>> >>> Strange.. READ_CAPACITY appears to be returning a payload as zeros..? >> >> Yep. Because blk_execute_rq() does not put the proper callback and data do >> not get copied from sg's to bounce buffer. That is why I tried to use >> blk_mq_execute_rq() instead. Once I do that, data start getting read and >> booting stops elsewhere. > > Mm. > > The call to blk_queue_bounce() exists within blk_mq_make_request(), but > AFAICT this should still be getting invoked regardless of if the struct > request is dispatched into blk-mq via the modified blk_execute_rq() -> > blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly > via blk_mq_execute_rq().. > blk_mq_make_request is not called from the blk insert/execute paths. blk_mq_make_request takes a bio and tries to merge it with a request and adds it to the queue. It is only called when the make_request_fn is called like when generic_make_request is called. blk_mq_insert_request adds a already formed request to the queue. It is already formed so that is why that path does not bounce bios. The bios/pages should already be added within the drivers restrictions. So for the read_cap path, the call to blk_rq_map_kern in scsi_execute does the blk_queue_bounce call. Just saw this while trying out iscsi with the scsi-mq stuff :) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On 07/18/2013 01:14 PM, Nicholas A. Bellinger wrote: > On Thu, 2013-07-18 at 11:51 -0700, Nicholas A. Bellinger wrote: >> On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: >>> On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: [7.927818] scsi_execute(): Calling blk_mq_free_request >>> [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS CC03 PQ: 0 ANSI: 5 OK, so INQUIRY response payload is looking as expected here. >>> >>> Yep. It is not on the top of my head, but I remember something like INQUIRYs >>> are emulated and thus do not have payload. >>> [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 B) [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks Strange.. READ_CAPACITY appears to be returning a payload as zeros..? >>> >>> Yep. Because blk_execute_rq() does not put the proper callback and data do >>> not get copied from sg's to bounce buffer. That is why I tried to use >>> blk_mq_execute_rq() instead. Once I do that, data start getting read and >>> booting stops elsewhere. >> >> Mm. >> >> The call to blk_queue_bounce() exists within blk_mq_make_request(), but >> AFAICT this should still be getting invoked regardless of if the struct >> request is dispatched into blk-mq via the modified blk_execute_rq() -> >> blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly >> via blk_mq_execute_rq().. >> >> Jens..? >> > > Actually sorry, your right. A call to blk_mq_insert_request() for > REQ_TYPE_BLOCK_PC will not invoke blk_queue_bounce() located near the > top of blk_mq_execute_rq(), which means that only REQ_TYPE_FS is > currently using bounce buffers, if required. > > Need to think a bit more about what to do here for REQ_TYPE_BLOCK_PC > bounce buffer special case with blk_execute_rq(), but I'm thinking that > blk_mq_execute_rq() should really not be used here.. > > Jens..? It needs to be pre-bounced, blk-mq will only bounce incoming bios and not requests merely added to the queue(s). Might be useful to add an equiv blk_mq_make_request() for this. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, 2013-07-18 at 13:12 -0600, Mike Christie wrote: > On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote: > > On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: > >> On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: > >>> [7.927818] scsi_execute(): Calling blk_mq_free_request >>> > >>> [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS > >>> CC03 PQ: 0 ANSI: 5 > >>> > >>> OK, so INQUIRY response payload is looking as expected here. > >> > >> Yep. It is not on the top of my head, but I remember something like > >> INQUIRYs > >> are emulated and thus do not have payload. > >> > >>> [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. > >>> [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 B) > >>> [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks > >>> > >>> Strange.. READ_CAPACITY appears to be returning a payload as zeros..? > >> > >> Yep. Because blk_execute_rq() does not put the proper callback and data do > >> not get copied from sg's to bounce buffer. That is why I tried to use > >> blk_mq_execute_rq() instead. Once I do that, data start getting read and > >> booting stops elsewhere. > > > > Mm. > > > > The call to blk_queue_bounce() exists within blk_mq_make_request(), but > > AFAICT this should still be getting invoked regardless of if the struct > > request is dispatched into blk-mq via the modified blk_execute_rq() -> > > blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly > > via blk_mq_execute_rq().. > > > > blk_mq_make_request is not called from the blk insert/execute paths. > blk_mq_make_request takes a bio and tries to merge it with a request and > adds it to the queue. It is only called when the make_request_fn is > called like when generic_make_request is called. > > blk_mq_insert_request adds a already formed request to the queue. It is > already formed so that is why that path does not bounce bios. The > bios/pages should already be added within the drivers restrictions. So > for the read_cap path, the call to blk_rq_map_kern in scsi_execute does > the blk_queue_bounce call. > , just noticed the blk_queue_bounce() in blk_rq_map_kern(). Not sure why this doesn't seem to be doing what it's supposed to for libata just yet.. > Just saw this while trying out iscsi with the scsi-mq stuff :) > Took at stab at this a while back, but ended getting distracted on other items. Do you have an initial conversion running yet..? --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, Jul 18 2013, Nicholas A. Bellinger wrote: > On Thu, 2013-07-18 at 13:12 -0600, Mike Christie wrote: > > On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote: > > > On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: > > >> On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: > > >>> [7.927818] scsi_execute(): Calling blk_mq_free_request >>> > > >>> [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS > > >>> CC03 PQ: 0 ANSI: 5 > > >>> > > >>> OK, so INQUIRY response payload is looking as expected here. > > >> > > >> Yep. It is not on the top of my head, but I remember something like > > >> INQUIRYs > > >> are emulated and thus do not have payload. > > >> > > >>> [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. > > >>> [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 B/512 > > >>> B) > > >>> [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks > > >>> > > >>> Strange.. READ_CAPACITY appears to be returning a payload as zeros..? > > >> > > >> Yep. Because blk_execute_rq() does not put the proper callback and data > > >> do > > >> not get copied from sg's to bounce buffer. That is why I tried to use > > >> blk_mq_execute_rq() instead. Once I do that, data start getting read and > > >> booting stops elsewhere. > > > > > > Mm. > > > > > > The call to blk_queue_bounce() exists within blk_mq_make_request(), but > > > AFAICT this should still be getting invoked regardless of if the struct > > > request is dispatched into blk-mq via the modified blk_execute_rq() -> > > > blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly > > > via blk_mq_execute_rq().. > > > > > > > blk_mq_make_request is not called from the blk insert/execute paths. > > blk_mq_make_request takes a bio and tries to merge it with a request and > > adds it to the queue. It is only called when the make_request_fn is > > called like when generic_make_request is called. > > > > blk_mq_insert_request adds a already formed request to the queue. It is > > already formed so that is why that path does not bounce bios. The > > bios/pages should already be added within the drivers restrictions. So > > for the read_cap path, the call to blk_rq_map_kern in scsi_execute does > > the blk_queue_bounce call. > > > > , just noticed the blk_queue_bounce() in blk_rq_map_kern(). > > Not sure why this doesn't seem to be doing what it's supposed to for > libata just yet.. How are you make the request from the bio? It'd be pretty trivial to ensure that it gets bounced properly... blk_mq_execute_rq() assumes a fully complete request, so it wont bounce anything. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, 2013-07-18 at 18:30 -0600, Jens Axboe wrote: > On Thu, Jul 18 2013, Nicholas A. Bellinger wrote: > > On Thu, 2013-07-18 at 13:12 -0600, Mike Christie wrote: > > > On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote: > > > > On Wed, 2013-07-17 at 18:19 +0200, Alexander Gordeev wrote: > > > >> On Tue, Jul 16, 2013 at 02:38:03PM -0700, Nicholas A. Bellinger wrote: > > > >>> [7.927818] scsi_execute(): Calling blk_mq_free_request >>> > > > >>> [7.927826] scsi 0:0:0:0: Direct-Access ATA ST9500530NS > > > >>>CC03 PQ: 0 ANSI: 5 > > > >>> > > > >>> OK, so INQUIRY response payload is looking as expected here. > > > >> > > > >> Yep. It is not on the top of my head, but I remember something like > > > >> INQUIRYs > > > >> are emulated and thus do not have payload. > > > >> > > > >>> [7.927960] sd 0:0:0:0: [sda] Sector size 0 reported, assuming 512. > > > >>> [7.927964] sd 0:0:0:0: [sda] 1 512-byte logical blocks: (512 > > > >>> B/512 B) > > > >>> [7.927965] sd 0:0:0:0: [sda] 0-byte physical blocks > > > >>> > > > >>> Strange.. READ_CAPACITY appears to be returning a payload as zeros..? > > > >> > > > >> Yep. Because blk_execute_rq() does not put the proper callback and > > > >> data do > > > >> not get copied from sg's to bounce buffer. That is why I tried to use > > > >> blk_mq_execute_rq() instead. Once I do that, data start getting read > > > >> and > > > >> booting stops elsewhere. > > > > > > > > Mm. > > > > > > > > The call to blk_queue_bounce() exists within blk_mq_make_request(), but > > > > AFAICT this should still be getting invoked regardless of if the struct > > > > request is dispatched into blk-mq via the modified blk_execute_rq() -> > > > > blk_execute_rq_nowait() -> blk_mq_insert_request() codepath, or directly > > > > via blk_mq_execute_rq().. > > > > > > > > > > blk_mq_make_request is not called from the blk insert/execute paths. > > > blk_mq_make_request takes a bio and tries to merge it with a request and > > > adds it to the queue. It is only called when the make_request_fn is > > > called like when generic_make_request is called. > > > > > > blk_mq_insert_request adds a already formed request to the queue. It is > > > already formed so that is why that path does not bounce bios. The > > > bios/pages should already be added within the drivers restrictions. So > > > for the read_cap path, the call to blk_rq_map_kern in scsi_execute does > > > the blk_queue_bounce call. > > > > > > > , just noticed the blk_queue_bounce() in blk_rq_map_kern(). > > > > Not sure why this doesn't seem to be doing what it's supposed to for > > libata just yet.. > > How are you make the request from the bio? It'd be pretty trivial to > ensure that it gets bounced properly... blk_mq_execute_rq() assumes a > fully complete request, so it wont bounce anything. > >From what I gather for REQ_TYPE_BLOCK_PC, scsi_execute() -> blk_rq_map_kern() -> blk_rq_append_bio() -> blk_rq_bio_prep() is what does the request setup from the bios returned by bio_[copy,map]_kern() in blk_rq_map_kern() code. blk_queue_bounce() is called immediately after blk_rq_append_bio() here, which AFAICT looks like it's doing the correct thing for scsi-mq.. What is strange here is that libata-scsi.c CDB emulation code is doing the same stuff for both INQUIRY (that seems to be OK) and READ_CAPACITY (that is returning zeros), which makes me think that something else is going on.. Alexander, where you able to re-test using sdev->sdev_mq_reg.queue_depth = 1 in scsi_mq_alloc_queue()..? --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi/isci/phy.c: Code tidiness, delete the redundant function call "sci_change_state(&iphy->sm, SCI_PHY_STOPPED)" in "sci_phy_link_layer_initialization()"
The "sci_phy_link_layer_initialization()" was called only once in "sci_phy_initialize()" and it is called before a call of "sci_change_state(&iphy->sm, SCI_PHY_STOPPED)". So the same call in the end of "sci_phy_link_layer_initialization()" is redundant. Signed-off-by: Xinghai Yu --- drivers/scsi/isci/phy.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c index cb87b2e..82e8491 100644 --- a/drivers/scsi/isci/phy.c +++ b/drivers/scsi/isci/phy.c @@ -309,9 +309,6 @@ sci_phy_link_layer_initialization(struct isci_phy *iphy, */ writel(0, &llr->link_layer_hang_detection_timeout); - /* We can exit the initial state to the stopped state */ - sci_change_state(&iphy->sm, SCI_PHY_STOPPED); - return SCI_SUCCESS; } -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, 2013-07-18 at 18:03 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2013-07-18 at 18:30 -0600, Jens Axboe wrote:
> > On Thu, Jul 18 2013, Nicholas A. Bellinger wrote:
> > > On Thu, 2013-07-18 at 13:12 -0600, Mike Christie wrote:
> > > > On 07/18/2013 12:51 PM, Nicholas A. Bellinger wrote:
> > > , just noticed the blk_queue_bounce() in blk_rq_map_kern().
> > >
> > > Not sure why this doesn't seem to be doing what it's supposed to for
> > > libata just yet..
> >
> > How are you make the request from the bio? It'd be pretty trivial to
> > ensure that it gets bounced properly... blk_mq_execute_rq() assumes a
> > fully complete request, so it wont bounce anything.
> >
>
> From what I gather for REQ_TYPE_BLOCK_PC, scsi_execute() ->
> blk_rq_map_kern() -> blk_rq_append_bio() -> blk_rq_bio_prep() is what
> does the request setup from the bios returned by bio_[copy,map]_kern()
> in blk_rq_map_kern() code.
>
> blk_queue_bounce() is called immediately after blk_rq_append_bio() here,
> which AFAICT looks like it's doing the correct thing for scsi-mq..
>
> What is strange here is that libata-scsi.c CDB emulation code is doing
> the same stuff for both INQUIRY (that seems to be OK) and READ_CAPACITY
> (that is returning zeros), which makes me think that something else is
> going on..
>
> Alexander, where you able to re-test using sdev->sdev_mq_reg.queue_depth
> = 1 in scsi_mq_alloc_queue()..?
>
So after a bit more hacking tonight it appears the explicit setting of
dma_alignment by libata (sdev->sector_size - 1) is what is making
blk_rq_map_kern() invoke blk_copy_kern() instead of blk_map_kern(), and
triggering this scsi-mq specific bug with libata. I'm able to confirm
with QEMU IDE emulation the bug is occurring only after INQUIRY and
before READ_CAPACITY, as reported by Alexander.
Below is a quick hack to skip this setting in ata_scsi_dev_config() for
blk-mq, and leaves the default dma_alignment=0x03 for REQ_TYPE_BLOCK_PC
requests as initially set by scsi-core in scsi_init_request_queue().
Also included is the change for using queue_depth = min(SHT->can_queue,
SHT->cmd_per_lun) during scsi-mq request_queue initialization, along
with a very basic ata_piix conversion for testing purposes.
With these three changes in place, I'm able to register a single 1GB
ata_piix LUN using QEMU IDE emulation, and successfully run simple fio
writeverify tests with blocksize=4k @ queue_depth=1.
Obviously this is not a proper bugfix for unaligned blk_copy_kern() with
scsi-mq + REQ_TYPE_BLOCK_PC case, but should be enough to at least get
libata LUN scanning to work. Need to take a deeper look at the problem
here..
Alexander, care to give this work-around a shot on your bare-metal setup
using ata_piix..?
--nab
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 9a8a674..ac05cd6 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1066,6 +1066,8 @@ static u8 piix_vmw_bmdma_status(struct ata_port *ap)
static struct scsi_host_template piix_sht = {
ATA_BMDMA_SHT(DRV_NAME),
+ .scsi_mq= true,
+ .queuecommand_mq = ata_scsi_queuecmd,
};
static struct ata_port_operations piix_sata_ops = {
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0101af5..191bc15 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1144,7 +1144,11 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
"sector_size=%u > PAGE_SIZE, PIO may malfunction\n",
sdev->sector_size);
- blk_queue_update_dma_alignment(q, sdev->sector_size - 1);
+ if (!q->mq_ops) {
+ blk_queue_update_dma_alignment(q, sdev->sector_size - 1);
+ } else {
+ printk("Skipping dma_alignment for libata w/ scsi-mq\n");
+ }
if (dev->flags & ATA_DFLAG_AN)
set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
diff --git a/drivers/scsi/scsi-mq.c b/drivers/scsi/scsi-mq.c
index ca6ff67..81b2633 100644
--- a/drivers/scsi/scsi-mq.c
+++ b/drivers/scsi/scsi-mq.c
@@ -199,11 +199,11 @@ int scsi_mq_alloc_queue(struct Scsi_Host *sh, struct
scsi_device *sdev)
int i, j;
sdev->sdev_mq_reg.ops = &scsi_mq_ops;
- sdev->sdev_mq_reg.queue_depth = sdev->queue_depth;
+ sdev->sdev_mq_reg.queue_depth = min((short)sh->hostt->can_queue,
+ sh->hostt->cmd_per_lun);
sdev->sdev_mq_reg.cmd_size = sizeof(struct scsi_cmnd) +
sh->hostt->cmd_size;
sdev->sdev_mq_reg.numa_node = NUMA_NO_NODE;
sdev->sdev_mq_reg.nr_hw_queues = 1;
- sdev->sdev_mq_reg.queue_depth = 64;
sdev->sdev_mq_reg.flags = BLK_MQ_F_SHOULD_MERGE;
printk("Calling blk_mq_init_queue: scsi_mq_ops: %p, queue_depth: %d,"
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo
[PATCH] scsi: replace strict_strtoul() with kstrtoul()
The usage of strict_strtoul() is not preferred, because
strict_strtoul() is obsolete. Thus, kstrtoul() should be
used.
Signed-off-by: Jingoo Han
---
drivers/scsi/pmcraid.c|6 --
drivers/scsi/scsi_sysfs.c |6 --
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 1eb7b028..8cd98e6 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4203,9 +4203,11 @@ static ssize_t pmcraid_store_log_level(
struct Scsi_Host *shost;
struct pmcraid_instance *pinstance;
unsigned long val;
+ int ret;
- if (strict_strtoul(buf, 10, &val))
- return -EINVAL;
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return ret;
/* log-level should be from 0 to 2 */
if (val > 2)
return -EINVAL;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7e50061..a876e9b 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -819,9 +819,11 @@ sdev_store_queue_ramp_up_period(struct device *dev,
{
struct scsi_device *sdev = to_scsi_device(dev);
unsigned long period;
+ int ret;
- if (strict_strtoul(buf, 10, &period))
- return -EINVAL;
+ ret = kstrtoul(buf, 10, &period);
+ if (ret)
+ return ret;
sdev->queue_ramp_up_period = msecs_to_jiffies(period);
return period;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
