All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/2] block: implement an unprep function corresponding directly to prep
@ 2010-06-30 17:01 James Bottomley
  2010-06-30 17:10 ` [RFC 2/2] sd: free discard page in unprep function James Bottomley
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: James Bottomley @ 2010-06-30 17:01 UTC (permalink / raw)
  To: Mike Snitzer, Christoph Hellwig, Jens Axboe, FUJITA Tomonori, linux-scsi

So this is more directly what I'm thinking.  It gives us an exactly
correct place to hang the discard allocation in SCSI.  The next patch
shows a potential implementation in sd.

I think it should avoid all the leaks people have been seeing trying to
move the discard allocation/free into scsi.  I also think it should
facilitate sending discard through SCSI as a REQ_TYPE_FS.

James

---

diff --git a/block/blk-core.c b/block/blk-core.c
index f84cce4..aab798e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -608,6 +608,7 @@ blk_init_allocated_queue_node(struct request_queue *q, request_fn_proc *rfn,
 
 	q->request_fn		= rfn;
 	q->prep_rq_fn		= NULL;
+	q->unprep_rq_fn		= NULL;
 	q->unplug_fn		= generic_unplug_device;
 	q->queue_flags		= QUEUE_FLAG_DEFAULT;
 	q->queue_lock		= lock;
@@ -2119,6 +2120,26 @@ static bool blk_update_bidi_request(struct request *rq, int error,
 	return false;
 }
 
+/**
+ * blk_unprep_request - unprepare a request
+ * @req:	the request
+ *
+ * This function makes a request ready for complete resubmission (or
+ * completion).  It happens only after all error handling is complete,
+ * so represents the appropriate moment to deallocate any resources
+ * that were allocated to the request in the prep_rq_fn.  The queue
+ * lock is held when calling this.
+ */
+void blk_unprep_request(struct request *req)
+{
+	struct request_queue *q = req->q;
+
+	req->cmd_flags &= ~REQ_DONTPREP;
+	if (q->unprep_rq_fn)
+		q->unprep_rq_fn(q, req);
+}
+EXPORT_SYMBOL_GPL(blk_unprep_request);
+
 /*
  * queue lock must be held
  */
@@ -2134,6 +2155,10 @@ static void blk_finish_request(struct request *req, int error)
 
 	blk_delete_timer(req);
 
+	if (req->cmd_flags & REQ_DONTPREP)
+		blk_unprep_request(req);
+
+
 	blk_account_io_done(req);
 
 	if (req->end_io)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index f5ed5a1..a234f4b 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -37,6 +37,23 @@ void blk_queue_prep_rq(struct request_queue *q, prep_rq_fn *pfn)
 EXPORT_SYMBOL(blk_queue_prep_rq);
 
 /**
+ * blk_queue_unprep_rq - set an unprepare_request function for queue
+ * @q:		queue
+ * @ufn:	unprepare_request function
+ *
+ * It's possible for a queue to register an unprepare_request callback
+ * which is invoked before the request is finally completed. The goal
+ * of the function is to deallocate any data that was allocated in the
+ * prepare_request callback.
+ *
+ */
+void blk_queue_unprep_rq(struct request_queue *q, unprep_rq_fn *ufn)
+{
+	q->unprep_rq_fn = ufn;
+}
+EXPORT_SYMBOL(blk_queue_unprep_rq);
+
+/**
  * blk_queue_merge_bvec - set a merge_bvec function for queue
  * @q:		queue
  * @mbfn:	merge_bvec_fn
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1646fe7..868b2f2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -85,7 +85,7 @@ static void scsi_unprep_request(struct request *req)
 {
 	struct scsi_cmnd *cmd = req->special;
 
-	req->cmd_flags &= ~REQ_DONTPREP;
+	blk_unprep_request(req);
 	req->special = NULL;
 
 	scsi_put_command(cmd);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 09a8402..16d97ee 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -264,6 +264,7 @@ struct request_pm_state
 typedef void (request_fn_proc) (struct request_queue *q);
 typedef int (make_request_fn) (struct request_queue *q, struct bio *bio);
 typedef int (prep_rq_fn) (struct request_queue *, struct request *);
+typedef void (unprep_rq_fn) (struct request_queue *, struct request *);
 typedef void (unplug_fn) (struct request_queue *);
 
 struct bio_vec;
@@ -346,6 +347,7 @@ struct request_queue
 	request_fn_proc		*request_fn;
 	make_request_fn		*make_request_fn;
 	prep_rq_fn		*prep_rq_fn;
+	unprep_rq_fn		*unprep_rq_fn;
 	unplug_fn		*unplug_fn;
 	merge_bvec_fn		*merge_bvec_fn;
 	prepare_flush_fn	*prepare_flush_fn;
@@ -915,6 +917,7 @@ extern void blk_complete_request(struct request *);
 extern void __blk_complete_request(struct request *);
 extern void blk_abort_request(struct request *);
 extern void blk_abort_queue(struct request_queue *);
+extern void blk_unprep_request(struct request *);
 
 /*
  * Access functions for manipulating queue properties
@@ -959,6 +962,7 @@ extern int blk_queue_dma_drain(struct request_queue *q,
 extern void blk_queue_lld_busy(struct request_queue *q, lld_busy_fn *fn);
 extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
 extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
+extern void blk_queue_unprep_rq(struct request_queue *, unprep_rq_fn *ufn);
 extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
 extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);




^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC 2/2] sd: free discard page in unprep function
  2010-06-30 17:01 [RFC 1/2] block: implement an unprep function corresponding directly to prep James Bottomley
@ 2010-06-30 17:10 ` James Bottomley
  2010-06-30 17:23 ` [RFC 1/2] block: implement an unprep function corresponding directly to prep Mike Snitzer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: James Bottomley @ 2010-06-30 17:10 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Christoph Hellwig, Jens Axboe, FUJITA Tomonori, linux-scsi, dm-devel

This isn't a proper patch, but really just an illustration how freeing
should be done ... it still needs to be paired correctly with the
allocation (and error handling on the allocation path).

James

---

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8802e48..7907be8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -461,6 +461,12 @@ static int sd_prepare_discard(struct request *rq)
 	return BLKPREP_OK;
 }
 
+static void sd_unprep_fn(struct request_queue *q, struct request *rq)
+{
+	if (rq->cmd_flags & REQ_DISCARD)
+		__free_page(bio_page(rq->bio));
+}
+
 /**
  *	sd_init_command - build a scsi (read or write) command from
  *	information in the request structure.
@@ -2226,6 +2232,7 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 	sd_revalidate_disk(gd);
 
 	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
+	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
 
 	gd->driverfs_dev = &sdp->sdev_gendev;
 	gd->flags = GENHD_FL_EXT_DEVT;



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-06-30 17:01 [RFC 1/2] block: implement an unprep function corresponding directly to prep James Bottomley
  2010-06-30 17:10 ` [RFC 2/2] sd: free discard page in unprep function James Bottomley
@ 2010-06-30 17:23 ` Mike Snitzer
  2010-06-30 17:51 ` Jens Axboe
  2010-07-01  1:47 ` FUJITA Tomonori
  3 siblings, 0 replies; 17+ messages in thread
From: Mike Snitzer @ 2010-06-30 17:23 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, Jens Axboe, FUJITA Tomonori, linux-scsi, dm-devel

On Wed, Jun 30 2010 at  1:01pm -0400,
James Bottomley <James.Bottomley@suse.de> wrote:

> So this is more directly what I'm thinking.  It gives us an exactly
> correct place to hang the discard allocation in SCSI.  The next patch
> shows a potential implementation in sd.
> 
> I think it should avoid all the leaks people have been seeing trying to
> move the discard allocation/free into scsi.  I also think it should
> facilitate sending discard through SCSI as a REQ_TYPE_FS.

Thanks james!

I'll look to take this template and layer in Christoph's approach to
pushing the page allocation down into SCSI.

I'll also look at Tomo's REQ_TYPE_FS changes for discard.

I'll report back as soon as I have something.

Mike

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-06-30 17:01 [RFC 1/2] block: implement an unprep function corresponding directly to prep James Bottomley
  2010-06-30 17:10 ` [RFC 2/2] sd: free discard page in unprep function James Bottomley
  2010-06-30 17:23 ` [RFC 1/2] block: implement an unprep function corresponding directly to prep Mike Snitzer
@ 2010-06-30 17:51 ` Jens Axboe
  2010-06-30 17:54   ` Martin K. Petersen
  2010-07-01  1:47 ` FUJITA Tomonori
  3 siblings, 1 reply; 17+ messages in thread
From: Jens Axboe @ 2010-06-30 17:51 UTC (permalink / raw)
  To: James Bottomley
  Cc: Mike Snitzer, Christoph Hellwig, FUJITA Tomonori, linux-scsi, dm-devel

On 30/06/10 19.01, James Bottomley wrote:
> So this is more directly what I'm thinking.  It gives us an exactly
> correct place to hang the discard allocation in SCSI.  The next patch
> shows a potential implementation in sd.
> 
> I think it should avoid all the leaks people have been seeing trying to
> move the discard allocation/free into scsi.  I also think it should
> facilitate sending discard through SCSI as a REQ_TYPE_FS.

This is so much better than the other hacks, thanks for doing this.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding  directly to prep
  2010-06-30 17:51 ` Jens Axboe
@ 2010-06-30 17:54   ` Martin K. Petersen
  0 siblings, 0 replies; 17+ messages in thread
From: Martin K. Petersen @ 2010-06-30 17:54 UTC (permalink / raw)
  To: Jens Axboe
  Cc: James Bottomley, Mike Snitzer, Christoph Hellwig,
	FUJITA Tomonori, linux-scsi, dm-devel

>>>>> "Jens" == Jens Axboe <axboe@kernel.dk> writes:

Jens> On 30/06/10 19.01, James Bottomley wrote:
>> So this is more directly what I'm thinking.  It gives us an exactly
>> correct place to hang the discard allocation in SCSI.  The next patch
>> shows a potential implementation in sd.
>> 
>> I think it should avoid all the leaks people have been seeing trying
>> to move the discard allocation/free into scsi.  I also think it
>> should facilitate sending discard through SCSI as a REQ_TYPE_FS.

Jens> This is so much better than the other hacks, thanks for doing
Jens> this.

Agreed!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-06-30 17:01 [RFC 1/2] block: implement an unprep function corresponding directly to prep James Bottomley
                   ` (2 preceding siblings ...)
  2010-06-30 17:51 ` Jens Axboe
@ 2010-07-01  1:47 ` FUJITA Tomonori
  2010-07-01  3:47   ` Mike Snitzer
  2010-07-02 11:03   ` Christoph Hellwig
  3 siblings, 2 replies; 17+ messages in thread
From: FUJITA Tomonori @ 2010-07-01  1:47 UTC (permalink / raw)
  To: James.Bottomley
  Cc: snitzer, hch, axboe, fujita.tomonori, linux-scsi, dm-devel

On Wed, 30 Jun 2010 12:01:04 -0500
James Bottomley <James.Bottomley@suse.de> wrote:

> So this is more directly what I'm thinking.  It gives us an exactly
> correct place to hang the discard allocation in SCSI.  The next patch
> shows a potential implementation in sd.

Yeah, making the prep_rq_fn API symmetrical makes sense lots.

 
> I think it should avoid all the leaks people have been seeing trying to
> move the discard allocation/free into scsi.  I also think it should
> facilitate sending discard through SCSI as a REQ_TYPE_FS.

I think so. If I can figure out why qemu scsi driver is broken, the
job is done.

I'll see how this patchset works and update my FS discard patchset on
the top of this.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-01  1:47 ` FUJITA Tomonori
@ 2010-07-01  3:47   ` Mike Snitzer
  2010-07-01  4:44     ` [dm-devel] " FUJITA Tomonori
  2010-07-02 11:03   ` Christoph Hellwig
  1 sibling, 1 reply; 17+ messages in thread
From: Mike Snitzer @ 2010-07-01  3:47 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: James.Bottomley, hch, axboe, linux-scsi, dm-devel

Hi,

On Wed, Jun 30 2010 at  9:47pm -0400,
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Wed, 30 Jun 2010 12:01:04 -0500
> James Bottomley <James.Bottomley@suse.de> wrote:
> 
> > So this is more directly what I'm thinking.  It gives us an exactly
> > correct place to hang the discard allocation in SCSI.  The next patch
> > shows a potential implementation in sd.
> 
> Yeah, making the prep_rq_fn API symmetrical makes sense lots.
> 
>  
> > I think it should avoid all the leaks people have been seeing trying to
> > move the discard allocation/free into scsi.  I also think it should
> > facilitate sending discard through SCSI as a REQ_TYPE_FS.
> 
> I think so. If I can figure out why qemu scsi driver is broken, the
> job is done.

When I combine this patch 1/2 and patch 2/2 with Christoph's discard
payload rework (a1d949f5f448) already staged in for-2.6.36 I'm finding
that: sd_unprep_fn's __free_page causes the system to hard hang/crash.
I need to get serial connected and/or kdump configured to see if I can
catch what is happening.

But if I comment out sd_unprep_fn's __free_page() from James' patch 2/2
I don't see a crash.  Seems some other code is altering the request
before James' new sd_unprep_fn hhook?

Are James' patches meant to be dependent on your REQ_TYPE_FS conversion
work?  I wouldn't think so.. but I haven't traced the call chains close
enough to know.
 
> I'll see how this patchset works and update my FS discard patchset on
> the top of this.

I look forward to your results!

Thanks,
Mike

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dm-devel] [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-01  3:47   ` Mike Snitzer
@ 2010-07-01  4:44     ` FUJITA Tomonori
  0 siblings, 0 replies; 17+ messages in thread
From: FUJITA Tomonori @ 2010-07-01  4:44 UTC (permalink / raw)
  To: dm-devel, axboe; +Cc: fujita.tomonori, James.Bottomley, hch, linux-scsi

On Wed, 30 Jun 2010 23:47:31 -0400
Mike Snitzer <snitzer@redhat.com> wrote:

> Hi,
> 
> On Wed, Jun 30 2010 at  9:47pm -0400,
> FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> 
> > On Wed, 30 Jun 2010 12:01:04 -0500
> > James Bottomley <James.Bottomley@suse.de> wrote:
> > 
> > > So this is more directly what I'm thinking.  It gives us an exactly
> > > correct place to hang the discard allocation in SCSI.  The next patch
> > > shows a potential implementation in sd.
> > 
> > Yeah, making the prep_rq_fn API symmetrical makes sense lots.
> > 
> >  
> > > I think it should avoid all the leaks people have been seeing trying to
> > > move the discard allocation/free into scsi.  I also think it should
> > > facilitate sending discard through SCSI as a REQ_TYPE_FS.
> > 
> > I think so. If I can figure out why qemu scsi driver is broken, the
> > job is done.
> 
> When I combine this patch 1/2 and patch 2/2 with Christoph's discard
> payload rework (a1d949f5f448) already staged in for-2.6.36 I'm finding
> that: sd_unprep_fn's __free_page causes the system to hard hang/crash.
> I need to get serial connected and/or kdump configured to see if I can
> catch what is happening.

I've not looked at the details of James' work yet but I think that we
can't call blk_uprep_request() in blk_finish_request(). req->bio has
already gone after blk_update_bidi_request().


> But if I comment out sd_unprep_fn's __free_page() from James' patch 2/2
> I don't see a crash.  Seems some other code is altering the request
> before James' new sd_unprep_fn hhook?
> 
> Are James' patches meant to be dependent on your REQ_TYPE_FS conversion
> work?  I wouldn't think so.. but I haven't traced the call chains close
> enough to know.

James' patch isn't dependent on my patchset.


> > I'll see how this patchset works and update my FS discard patchset on
> > the top of this.
> 
> I look forward to your results!

I make sure that you'll have something to test tomorrow. :)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-01  1:47 ` FUJITA Tomonori
  2010-07-01  3:47   ` Mike Snitzer
@ 2010-07-02 11:03   ` Christoph Hellwig
  2010-07-05  7:00     ` FUJITA Tomonori
  1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2010-07-02 11:03 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: James.Bottomley, snitzer, hch, axboe, linux-scsi, dm-devel

On Thu, Jul 01, 2010 at 10:47:12AM +0900, FUJITA Tomonori wrote:
> I think so. If I can figure out why qemu scsi driver is broken, the
> job is done.

Well, I still have problems even with libata + a trim capable SSD.
Dmesg from the last attept with your patch to use FS requests all the
way:

[   27.669404] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   27.669533] ata2.00: irq_stat 0x08000000, interface fatal error
[   27.669627] ata2: SError: { UnrecovData Handshk }
[   27.669720] ata2.00: failed command: WRITE FPDMA QUEUED
[   27.669819] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   27.669821]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   27.670096] ata2.00: status: { DRDY }
[   27.670196] ata2: hard resetting link
[   27.975118] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[   27.978618] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   27.978628] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   27.978756] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   27.984299] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   27.984309] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   27.984437] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   27.984814] ata2.00: configured for UDMA/133
[   27.987548] ata2.00: configured for UDMA/133
[   27.987645] ata2: EH complete
[   27.987784] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   27.987915] ata2.00: irq_stat 0x08000000, interface fatal error
[   27.988026] ata2: SError: { UnrecovData Handshk }
[   27.988128] ata2.00: failed command: WRITE FPDMA QUEUED
[   27.988234] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   27.988236]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   27.988499] ata2.00: status: { DRDY }
[   27.988591] ata2: hard resetting link
[   28.293109] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[   28.297509] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.297518] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.297648] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.301854] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.301863] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.301991] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.302378] ata2.00: configured for UDMA/133
[   28.305024] ata2.00: configured for UDMA/133
[   28.305123] ata2: EH complete
[   28.305292] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   28.305421] ata2.00: irq_stat 0x08000000, interface fatal error
[   28.305513] ata2: SError: { UnrecovData Handshk }
[   28.305605] ata2.00: failed command: WRITE FPDMA QUEUED
[   28.305703] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   28.305705]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   28.305948] ata2.00: status: { DRDY }
[   28.306066] ata2: hard resetting link
[   28.611111] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[   28.613232] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.613241] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.613378] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.619103] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.619112] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.619240] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.619616] ata2.00: configured for UDMA/133
[   28.623193] ata2.00: configured for UDMA/133
[   28.623289] ata2: EH complete
[   28.623427] ata2: limiting SATA link speed to 1.5 Gbps
[   28.623528] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   28.623651] ata2.00: irq_stat 0x08000000, interface fatal error
[   28.623745] ata2: SError: { UnrecovData Handshk }
[   28.623836] ata2.00: failed command: WRITE FPDMA QUEUED
[   28.623934] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   28.623937]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   28.624202] ata2.00: status: { DRDY }
[   28.624303] ata2: hard resetting link
[   28.929103] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[   28.932449] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.932458] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.932587] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.939190] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   28.939200] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   28.939328] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   28.939763] ata2.00: configured for UDMA/133
[   28.941154] ata2.00: configured for UDMA/133
[   28.941250] ata2: EH complete
[   28.941414] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   28.941538] ata2.00: irq_stat 0x08000000, interface fatal error
[   28.941631] ata2: SError: { UnrecovData Handshk }
[   28.941722] ata2.00: failed command: WRITE FPDMA QUEUED
[   28.941821] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   28.941823]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   28.942091] ata2.00: status: { DRDY }
[   28.942187] ata2: hard resetting link
[   29.247116] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[   29.250536] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   29.250545] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   29.250673] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   29.256129] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   29.256138] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   29.256266] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   29.256651] ata2.00: configured for UDMA/133
[   29.259263] ata2.00: configured for UDMA/133
[   29.259359] ata2: EH complete
[   29.259533] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
[   29.259659] ata2.00: irq_stat 0x08000000, interface fatal error
[   29.259752] ata2: SError: { UnrecovData Handshk }
[   29.259843] ata2.00: failed command: WRITE FPDMA QUEUED
[   29.259942] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
[   29.259945]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
[   29.260218] ata2.00: status: { DRDY }
[   29.260319] ata2: hard resetting link
[   29.565102] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[   29.569593] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   29.569603] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   29.569731] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   29.573969] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[   29.573978] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
[   29.574127] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
[   29.574543] ata2.00: configured for UDMA/133
[   29.577217] ata2.00: configured for UDMA/133
[   29.577313] sd 1:0:0:0: [sdb] Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[   29.577460] sd 1:0:0:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
[   29.577786] Descriptor sense data with sense descriptors (in hex):
[   29.577928]         72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
[   29.578923]         00 00 00 00
[   29.579301] sd 1:0:0:0: [sdb] Add. Sense: No additional sense information
[   29.579496] sd 1:0:0:0: [sdb] CDB: Write(10): 2a 00 00 00 00 00 00 ff ff 00
[   29.580281] end_request: I/O error, dev sdb, sector 0
[   29.580395] ata2: EH complete


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-02 11:03   ` Christoph Hellwig
@ 2010-07-05  7:00     ` FUJITA Tomonori
  2010-07-05 19:24       ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: FUJITA Tomonori @ 2010-07-05  7:00 UTC (permalink / raw)
  To: hch
  Cc: fujita.tomonori, James.Bottomley, snitzer, axboe, linux-scsi,
	dm-devel, linux-ide

Cc'ed to linux-ide,

On Fri, 2 Jul 2010 13:03:44 +0200
Christoph Hellwig <hch@lst.de> wrote:

> On Thu, Jul 01, 2010 at 10:47:12AM +0900, FUJITA Tomonori wrote:
> > I think so. If I can figure out why qemu scsi driver is broken, the
> > job is done.
> 
> Well, I still have problems even with libata + a trim capable SSD.
> Dmesg from the last attept with your patch to use FS requests all the
> way:

Thanks for testing.

Did you tested my discard branch, right?

Wired, I've just got Intel SSD X25-M drives and mkfs.xfs worked well.

REQ_TYPE_FS should give the same scsi_cmnd struct as
REQ_TYPE_BLOCK_PC... I've attached a patch to print some debug info.

Ide people, any idea about what the following error message means?

I've been trying to convert WRITE_SAME_16 from REQ_TYPE_BLOCK_PC to
REQ_TYPE_FS.


> [   27.669404] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   27.669533] ata2.00: irq_stat 0x08000000, interface fatal error
> [   27.669627] ata2: SError: { UnrecovData Handshk }
> [   27.669720] ata2.00: failed command: WRITE FPDMA QUEUED
> [   27.669819] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   27.669821]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   27.670096] ata2.00: status: { DRDY }
> [   27.670196] ata2: hard resetting link
> [   27.975118] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [   27.978618] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   27.978628] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   27.978756] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   27.984299] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   27.984309] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   27.984437] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   27.984814] ata2.00: configured for UDMA/133
> [   27.987548] ata2.00: configured for UDMA/133
> [   27.987645] ata2: EH complete
> [   27.987784] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   27.987915] ata2.00: irq_stat 0x08000000, interface fatal error
> [   27.988026] ata2: SError: { UnrecovData Handshk }
> [   27.988128] ata2.00: failed command: WRITE FPDMA QUEUED
> [   27.988234] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   27.988236]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   27.988499] ata2.00: status: { DRDY }
> [   27.988591] ata2: hard resetting link
> [   28.293109] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [   28.297509] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.297518] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.297648] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.301854] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.301863] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.301991] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.302378] ata2.00: configured for UDMA/133
> [   28.305024] ata2.00: configured for UDMA/133
> [   28.305123] ata2: EH complete
> [   28.305292] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   28.305421] ata2.00: irq_stat 0x08000000, interface fatal error
> [   28.305513] ata2: SError: { UnrecovData Handshk }
> [   28.305605] ata2.00: failed command: WRITE FPDMA QUEUED
> [   28.305703] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   28.305705]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   28.305948] ata2.00: status: { DRDY }
> [   28.306066] ata2: hard resetting link
> [   28.611111] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [   28.613232] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.613241] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.613378] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.619103] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.619112] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.619240] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.619616] ata2.00: configured for UDMA/133
> [   28.623193] ata2.00: configured for UDMA/133
> [   28.623289] ata2: EH complete
> [   28.623427] ata2: limiting SATA link speed to 1.5 Gbps
> [   28.623528] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   28.623651] ata2.00: irq_stat 0x08000000, interface fatal error
> [   28.623745] ata2: SError: { UnrecovData Handshk }
> [   28.623836] ata2.00: failed command: WRITE FPDMA QUEUED
> [   28.623934] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   28.623937]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   28.624202] ata2.00: status: { DRDY }
> [   28.624303] ata2: hard resetting link
> [   28.929103] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [   28.932449] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.932458] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.932587] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.939190] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   28.939200] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   28.939328] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   28.939763] ata2.00: configured for UDMA/133
> [   28.941154] ata2.00: configured for UDMA/133
> [   28.941250] ata2: EH complete
> [   28.941414] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   28.941538] ata2.00: irq_stat 0x08000000, interface fatal error
> [   28.941631] ata2: SError: { UnrecovData Handshk }
> [   28.941722] ata2.00: failed command: WRITE FPDMA QUEUED
> [   28.941821] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   28.941823]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   28.942091] ata2.00: status: { DRDY }
> [   28.942187] ata2: hard resetting link
> [   29.247116] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [   29.250536] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   29.250545] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   29.250673] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   29.256129] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   29.256138] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   29.256266] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   29.256651] ata2.00: configured for UDMA/133
> [   29.259263] ata2.00: configured for UDMA/133
> [   29.259359] ata2: EH complete
> [   29.259533] ata2.00: exception Emask 0x10 SAct 0x1 SErr 0x400100 action 0x6 frozen
> [   29.259659] ata2.00: irq_stat 0x08000000, interface fatal error
> [   29.259752] ata2: SError: { UnrecovData Handshk }
> [   29.259843] ata2.00: failed command: WRITE FPDMA QUEUED
> [   29.259942] ata2.00: cmd 61/ff:00:00:00:00/ff:00:00:00:00/40 tag 0 ncq 33553920 out
> [   29.259945]          res 40/00:00:00:00:00/00:00:00:00:00/40 Emask 0x10 (ATA bus error)
> [   29.260218] ata2.00: status: { DRDY }
> [   29.260319] ata2: hard resetting link
> [   29.565102] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [   29.569593] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   29.569603] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   29.569731] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   29.573969] ata2.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
> [   29.573978] ata2.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
> [   29.574127] ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
> [   29.574543] ata2.00: configured for UDMA/133
> [   29.577217] ata2.00: configured for UDMA/133
> [   29.577313] sd 1:0:0:0: [sdb] Result: hostbyte=DID_OK
> driverbyte=DRIVER_SENSE
> [   29.577460] sd 1:0:0:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
> [   29.577786] Descriptor sense data with sense descriptors (in hex):
> [   29.577928]         72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
> [   29.578923]         00 00 00 00
> [   29.579301] sd 1:0:0:0: [sdb] Add. Sense: No additional sense information
> [   29.579496] sd 1:0:0:0: [sdb] CDB: Write(10): 2a 00 00 00 00 00 00 ff ff 00
> [   29.580281] end_request: I/O error, dev sdb, sector 0
> [   29.580395] ata2: EH complete


diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0a8cd34..70b6b3d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3020,6 +3020,9 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 
 	ata_qc_set_pc_nbytes(qc);
 
+	printk("%s %d: %p %d %d %d\n", __func__, __LINE__, scmd,
+	       scsi_bufflen(scmd), qc->extrabytes, size);
+
 	return 0;
 
  invalid_fld:
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index ad0ed21..9e98d0b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -838,6 +838,13 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 				"(result %x)\n", cmd->result));
 
 	good_bytes = scsi_bufflen(cmd);
+
+	if (cmd->request->cmd_flags & REQ_DISCARD)
+		printk("%s %d: %p %d %d %d %u %d\n", __func__, __LINE__,
+		       cmd->request, scsi_bufflen(cmd), good_bytes,
+		       scsi_get_resid(cmd), cmd->request->errors,
+		       cmd->result);
+
         if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC) {
 		int old_good_bytes = good_bytes;
 		drv = scsi_cmd_to_driver(cmd);
@@ -849,9 +856,16 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 		 * residue if drv->done() error processing indicates no
 		 * change to the completion length.
 		 */
+
+		if (cmd->request->cmd_flags & REQ_DISCARD)
+			printk("%s %d: %p %d %d %d\n", __func__, __LINE__,
+			       cmd->request, good_bytes, old_good_bytes,
+			       scsi_get_resid(cmd));
+
 		if (good_bytes == old_good_bytes)
 			good_bytes -= scsi_get_resid(cmd);
 	}
+
 	scsi_io_completion(cmd, good_bytes);
 }
 EXPORT_SYMBOL(scsi_finish_command);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8c1b084..f92bbff 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -464,6 +464,10 @@ static int scsi_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
 	}
 
 	blk_add_request_payload(rq, page, len);
+
+	printk("%s %d: %p %d %d\n", __func__, __LINE__, rq, sdkp->unmap,
+		len);
+
 	return scsi_setup_blk_pc_cmnd(sdp, rq);
 }
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05  7:00     ` FUJITA Tomonori
@ 2010-07-05 19:24       ` Christoph Hellwig
  2010-07-05 21:50         ` Mark Lord
  2010-07-06  7:04         ` FUJITA Tomonori
  0 siblings, 2 replies; 17+ messages in thread
From: Christoph Hellwig @ 2010-07-05 19:24 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: hch, James.Bottomley, snitzer, axboe, linux-scsi, dm-devel, linux-ide

On Mon, Jul 05, 2010 at 04:00:44PM +0900, FUJITA Tomonori wrote:
> Did you tested my discard branch, right?

I tested the patch that you sent out back then.

> Wired, I've just got Intel SSD X25-M drives and mkfs.xfs worked well.

What codebase were you testing on?  Sorry, but curently I'm a bit lost
in the maze of patches.  I've got both and intel and an OCZ SSD (right
now I'm travelling with only access to the OCZ actually) and I'd like
to test the latest variant again.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05 19:24       ` Christoph Hellwig
@ 2010-07-05 21:50         ` Mark Lord
  2010-07-05 21:54           ` Mark Lord
  2010-07-06  7:04         ` FUJITA Tomonori
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Lord @ 2010-07-05 21:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: FUJITA Tomonori, James.Bottomley, snitzer, axboe, linux-scsi,
	dm-devel, linux-ide

On 05/07/10 03:24 PM, Christoph Hellwig wrote:
>
> What codebase were you testing on?  Sorry, but curently I'm a bit lost
> in the maze of patches.  I've got both and intel and an OCZ SSD (right
> now I'm travelling with only access to the OCZ actually) and I'd like
> to test the latest variant again.
..

Speaking of which..  I have recently noticed that some OCZ drives
fail TRIM commands when issued for the final sector of the drive..
which happens by default with "mke2fs -t ext4".

So I now partition those to not include the final sector.

Keep your eyes peeled for stuff like that.

Cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05 21:50         ` Mark Lord
@ 2010-07-05 21:54           ` Mark Lord
  2010-07-05 22:00             ` Jeff Garzik
  2010-08-03  3:01             ` Mark Lord
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Lord @ 2010-07-05 21:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: FUJITA Tomonori, James.Bottomley, snitzer, axboe, linux-scsi,
	dm-devel, linux-ide

> On 05/07/10 03:24 PM, Christoph Hellwig wrote:
>>
>> What codebase were you testing on? Sorry, but curently I'm a bit lost
>> in the maze of patches. I've got both and intel and an OCZ SSD
..

Do you, or anyone else with one, know what the upper limit is on
TRIM operations with the Intel SSDs ?

I need to know the maximum amount of TRIM ranges they will process
in a single command.

Eg. Indilinx-based SSDs don't appear to have a limit -- they'll accept
a TRIM command with thousands of LBA ranges included.

Sandforce-based SSDs appear to restrict things to max 4KB of range data.

So.. what about Intel?

Thanks

  

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05 21:54           ` Mark Lord
@ 2010-07-05 22:00             ` Jeff Garzik
  2010-08-03  3:01             ` Mark Lord
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Garzik @ 2010-07-05 22:00 UTC (permalink / raw)
  To: Mark Lord
  Cc: Christoph Hellwig, FUJITA Tomonori, James.Bottomley, snitzer,
	axboe, linux-scsi, dm-devel, linux-ide

On 07/05/2010 05:54 PM, Mark Lord wrote:
>> On 05/07/10 03:24 PM, Christoph Hellwig wrote:
>>>
>>> What codebase were you testing on? Sorry, but curently I'm a bit lost
>>> in the maze of patches. I've got both and intel and an OCZ SSD
> ..
>
> Do you, or anyone else with one, know what the upper limit is on
> TRIM operations with the Intel SSDs ?
>
> I need to know the maximum amount of TRIM ranges they will process
> in a single command.
>
> Eg. Indilinx-based SSDs don't appear to have a limit -- they'll accept
> a TRIM command with thousands of LBA ranges included.
>
> Sandforce-based SSDs appear to restrict things to max 4KB of range data.
>
> So.. what about Intel?

Lovely :(  I wonder if we could test for that limit at mke2fs time, or 
similar.

Seeing a 4k limit on other SSDs would not surprise me.  Or even a 512-b 
limit.

	Jeff




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05 19:24       ` Christoph Hellwig
  2010-07-05 21:50         ` Mark Lord
@ 2010-07-06  7:04         ` FUJITA Tomonori
  1 sibling, 0 replies; 17+ messages in thread
From: FUJITA Tomonori @ 2010-07-06  7:04 UTC (permalink / raw)
  To: hch
  Cc: fujita.tomonori, James.Bottomley, snitzer, axboe, linux-scsi,
	dm-devel, linux-ide

On Mon, 5 Jul 2010 21:24:13 +0200
Christoph Hellwig <hch@lst.de> wrote:

> > Wired, I've just got Intel SSD X25-M drives and mkfs.xfs worked well.
> 
> What codebase were you testing on?  Sorry, but curently I'm a bit lost
> in the maze of patches.  I've got both and intel and an OCZ SSD (right
> now I'm travelling with only access to the OCZ actually) and I'd like
> to test the latest variant again.

I tested:

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git discard


I rebased the changes against Jens' for-2.6.36 and I've just sent the
latest patch.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 1/2] block: implement an unprep function corresponding directly to prep
  2010-07-05 21:54           ` Mark Lord
  2010-07-05 22:00             ` Jeff Garzik
@ 2010-08-03  3:01             ` Mark Lord
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Lord @ 2010-08-03  3:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: FUJITA Tomonori, James.Bottomley, snitzer, axboe, linux-scsi,
	dm-devel, linux-ide

On 10-07-05 05:54 PM, Mark Lord wrote:
>> On 05/07/10 03:24 PM, Christoph Hellwig wrote:
>>>
>>> What codebase were you testing on? Sorry, but curently I'm a bit lost
>>> in the maze of patches. I've got both and intel and an OCZ SSD
> ..
>
> Do you, or anyone else with one, know what the upper limit is on
> TRIM operations with the Intel SSDs ?
>
> I need to know the maximum amount of TRIM ranges they will process
> in a single command.
>
> Eg. Indilinx-based SSDs don't appear to have a limit -- they'll accept
> a TRIM command with thousands of LBA ranges included.
>
> Sandforce-based SSDs appear to restrict things to max 4KB of range data.
>
> So.. what about Intel?


Well, I still don't know about Intel drives -- anyone?

But for the latest hardware, some manufacturers are now reporting
their TRIM limits in word[105] of the identify data,
as described (optional) in the latest draft ATA ACS-2 specs.

It appears that Sandforce drives officially only allow 512-bytes of range data.
So I'll limit hdparm's TRIM functions to match word[105] when it is specified.

The Indilinx drives I have here don't report anything in word[105], though.

Cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC 1/2] block: implement an unprep function corresponding directly to prep
@ 2010-06-30 17:01 James Bottomley
  0 siblings, 0 replies; 17+ messages in thread
From: James Bottomley @ 2010-06-30 17:01 UTC (permalink / raw)
  To: Mike Snitzer, Christoph Hellwig, Jens Axboe, FUJITA Tomonori, linux-scsi

So this is more directly what I'm thinking.  It gives us an exactly
correct place to hang the discard allocation in SCSI.  The next patch
shows a potential implementation in sd.

I think it should avoid all the leaks people have been seeing trying to
move the discard allocation/free into scsi.  I also think it should
facilitate sending discard through SCSI as a REQ_TYPE_FS.

James

---

diff --git a/block/blk-core.c b/block/blk-core.c
index f84cce4..aab798e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -608,6 +608,7 @@ blk_init_allocated_queue_node(struct request_queue *q, request_fn_proc *rfn,
 
 	q->request_fn		= rfn;
 	q->prep_rq_fn		= NULL;
+	q->unprep_rq_fn		= NULL;
 	q->unplug_fn		= generic_unplug_device;
 	q->queue_flags		= QUEUE_FLAG_DEFAULT;
 	q->queue_lock		= lock;
@@ -2119,6 +2120,26 @@ static bool blk_update_bidi_request(struct request *rq, int error,
 	return false;
 }
 
+/**
+ * blk_unprep_request - unprepare a request
+ * @req:	the request
+ *
+ * This function makes a request ready for complete resubmission (or
+ * completion).  It happens only after all error handling is complete,
+ * so represents the appropriate moment to deallocate any resources
+ * that were allocated to the request in the prep_rq_fn.  The queue
+ * lock is held when calling this.
+ */
+void blk_unprep_request(struct request *req)
+{
+	struct request_queue *q = req->q;
+
+	req->cmd_flags &= ~REQ_DONTPREP;
+	if (q->unprep_rq_fn)
+		q->unprep_rq_fn(q, req);
+}
+EXPORT_SYMBOL_GPL(blk_unprep_request);
+
 /*
  * queue lock must be held
  */
@@ -2134,6 +2155,10 @@ static void blk_finish_request(struct request *req, int error)
 
 	blk_delete_timer(req);
 
+	if (req->cmd_flags & REQ_DONTPREP)
+		blk_unprep_request(req);
+
+
 	blk_account_io_done(req);
 
 	if (req->end_io)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index f5ed5a1..a234f4b 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -37,6 +37,23 @@ void blk_queue_prep_rq(struct request_queue *q, prep_rq_fn *pfn)
 EXPORT_SYMBOL(blk_queue_prep_rq);
 
 /**
+ * blk_queue_unprep_rq - set an unprepare_request function for queue
+ * @q:		queue
+ * @ufn:	unprepare_request function
+ *
+ * It's possible for a queue to register an unprepare_request callback
+ * which is invoked before the request is finally completed. The goal
+ * of the function is to deallocate any data that was allocated in the
+ * prepare_request callback.
+ *
+ */
+void blk_queue_unprep_rq(struct request_queue *q, unprep_rq_fn *ufn)
+{
+	q->unprep_rq_fn = ufn;
+}
+EXPORT_SYMBOL(blk_queue_unprep_rq);
+
+/**
  * blk_queue_merge_bvec - set a merge_bvec function for queue
  * @q:		queue
  * @mbfn:	merge_bvec_fn
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1646fe7..868b2f2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -85,7 +85,7 @@ static void scsi_unprep_request(struct request *req)
 {
 	struct scsi_cmnd *cmd = req->special;
 
-	req->cmd_flags &= ~REQ_DONTPREP;
+	blk_unprep_request(req);
 	req->special = NULL;
 
 	scsi_put_command(cmd);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 09a8402..16d97ee 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -264,6 +264,7 @@ struct request_pm_state
 typedef void (request_fn_proc) (struct request_queue *q);
 typedef int (make_request_fn) (struct request_queue *q, struct bio *bio);
 typedef int (prep_rq_fn) (struct request_queue *, struct request *);
+typedef void (unprep_rq_fn) (struct request_queue *, struct request *);
 typedef void (unplug_fn) (struct request_queue *);
 
 struct bio_vec;
@@ -346,6 +347,7 @@ struct request_queue
 	request_fn_proc		*request_fn;
 	make_request_fn		*make_request_fn;
 	prep_rq_fn		*prep_rq_fn;
+	unprep_rq_fn		*unprep_rq_fn;
 	unplug_fn		*unplug_fn;
 	merge_bvec_fn		*merge_bvec_fn;
 	prepare_flush_fn	*prepare_flush_fn;
@@ -915,6 +917,7 @@ extern void blk_complete_request(struct request *);
 extern void __blk_complete_request(struct request *);
 extern void blk_abort_request(struct request *);
 extern void blk_abort_queue(struct request_queue *);
+extern void blk_unprep_request(struct request *);
 
 /*
  * Access functions for manipulating queue properties
@@ -959,6 +962,7 @@ extern int blk_queue_dma_drain(struct request_queue *q,
 extern void blk_queue_lld_busy(struct request_queue *q, lld_busy_fn *fn);
 extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
 extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
+extern void blk_queue_unprep_rq(struct request_queue *, unprep_rq_fn *ufn);
 extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
 extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-08-03  3:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-30 17:01 [RFC 1/2] block: implement an unprep function corresponding directly to prep James Bottomley
2010-06-30 17:10 ` [RFC 2/2] sd: free discard page in unprep function James Bottomley
2010-06-30 17:23 ` [RFC 1/2] block: implement an unprep function corresponding directly to prep Mike Snitzer
2010-06-30 17:51 ` Jens Axboe
2010-06-30 17:54   ` Martin K. Petersen
2010-07-01  1:47 ` FUJITA Tomonori
2010-07-01  3:47   ` Mike Snitzer
2010-07-01  4:44     ` [dm-devel] " FUJITA Tomonori
2010-07-02 11:03   ` Christoph Hellwig
2010-07-05  7:00     ` FUJITA Tomonori
2010-07-05 19:24       ` Christoph Hellwig
2010-07-05 21:50         ` Mark Lord
2010-07-05 21:54           ` Mark Lord
2010-07-05 22:00             ` Jeff Garzik
2010-08-03  3:01             ` Mark Lord
2010-07-06  7:04         ` FUJITA Tomonori
  -- strict thread matches above, loose matches on Subject: below --
2010-06-30 17:01 James Bottomley

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.