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