All of lore.kernel.org
 help / color / mirror / Atom feed
* Why removing REQ_FAILFAST_DRIVER in LightNVM?
@ 2017-10-06  9:19 Javier González
  2017-10-06 11:59 ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Javier González @ 2017-10-06  9:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Matias Bjørling, linux-block

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Hi Christoph,

I'm cleaning up lightnvm.c to use as much as possible the nvme helpers.

I see that in Commit: d49187e97e94 "nvme: introduce struct nvme_request"

you introduced:
  rq->cmd_flags &= ~REQ_FAILFAST_DRIVER

on the lightnvm I/O path and that has propagated through the code as we
added more functionality. Can you explain why this is necessary? If I
can just remove it, it is much easier to do the cleanup.

I have tested on or HW and all works fine without it.

Thanks,
Javier


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Why removing REQ_FAILFAST_DRIVER in LightNVM?
  2017-10-06  9:19 Why removing REQ_FAILFAST_DRIVER in LightNVM? Javier González
@ 2017-10-06 11:59 ` Christoph Hellwig
  2017-10-06 12:01   ` Javier González
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2017-10-06 11:59 UTC (permalink / raw)
  To: Javier González; +Cc: Christoph Hellwig, Matias Bjørling, linux-block

On Fri, Oct 06, 2017 at 11:19:09AM +0200, Javier Gonz�lez wrote:
> on the lightnvm I/O path and that has propagated through the code as we
> added more functionality. Can you explain why this is necessary? If I
> can just remove it, it is much easier to do the cleanup.
> 
> I have tested on or HW and all works fine without it.

nvme_alloc_request always sets REQ_FAILFAST_DRIVER and that srops
it to stick to the previous behavior.

If you actually want all your lightnvm requests to fail fast you can
remove it.

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

* Re: Why removing REQ_FAILFAST_DRIVER in LightNVM?
  2017-10-06 11:59 ` Christoph Hellwig
@ 2017-10-06 12:01   ` Javier González
  2017-10-06 12:06     ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Javier González @ 2017-10-06 12:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Matias Bjørling, linux-block

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

> On 6 Oct 2017, at 13.59, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Fri, Oct 06, 2017 at 11:19:09AM +0200, Javier González wrote:
>> on the lightnvm I/O path and that has propagated through the code as we
>> added more functionality. Can you explain why this is necessary? If I
>> can just remove it, it is much easier to do the cleanup.
>> 
>> I have tested on or HW and all works fine without it.
> 
> nvme_alloc_request always sets REQ_FAILFAST_DRIVER and that srops
> it to stick to the previous behavior.
> 
> If you actually want all your lightnvm requests to fail fast you can
> remove it.

I think it is good to fail fast as any other nvme I/O command and then
recover in pblk if necessary.

Thanks Christoph!
Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Why removing REQ_FAILFAST_DRIVER in LightNVM?
  2017-10-06 12:01   ` Javier González
@ 2017-10-06 12:06     ` Christoph Hellwig
  2017-10-06 12:08       ` Javier González
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2017-10-06 12:06 UTC (permalink / raw)
  To: Javier González; +Cc: Christoph Hellwig, Matias Bjørling, linux-block

On Fri, Oct 06, 2017 at 02:01:46PM +0200, Javier Gonz�lez wrote:
> I think it is good to fail fast as any other nvme I/O command and then
> recover in pblk if necessary.

Note that we only do it for other nvme _passthrough_ commands - the
actual I/O commands dot not get the failfast flag.

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

* Re: Why removing REQ_FAILFAST_DRIVER in LightNVM?
  2017-10-06 12:06     ` Christoph Hellwig
@ 2017-10-06 12:08       ` Javier González
  2017-10-06 12:12         ` Javier González
  0 siblings, 1 reply; 6+ messages in thread
From: Javier González @ 2017-10-06 12:08 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Matias Bjørling, linux-block

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

> On 6 Oct 2017, at 14.06, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Fri, Oct 06, 2017 at 02:01:46PM +0200, Javier González wrote:
>> I think it is good to fail fast as any other nvme I/O command and then
>> recover in pblk if necessary.
> 
> Note that we only do it for other nvme _passthrough_ commands - the
> actual I/O commands dot not get the failfast flag.

Yes. I was thinking of the LightNVM-specific _passthrough_ used by
liblightnvm since I'm refactoring the sync. path at the moment; the
"normal" async. path will be untouched for now.

Thanks,
Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Why removing REQ_FAILFAST_DRIVER in LightNVM?
  2017-10-06 12:08       ` Javier González
@ 2017-10-06 12:12         ` Javier González
  0 siblings, 0 replies; 6+ messages in thread
From: Javier González @ 2017-10-06 12:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Matias Bjørling, linux-block

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

> On 6 Oct 2017, at 14.08, Javier González <jg@lightnvm.io> wrote:
> 
>> On 6 Oct 2017, at 14.06, Christoph Hellwig <hch@infradead.org> wrote:
>> 
>> On Fri, Oct 06, 2017 at 02:01:46PM +0200, Javier González wrote:
>>> I think it is good to fail fast as any other nvme I/O command and then
>>> recover in pblk if necessary.
>> 
>> Note that we only do it for other nvme _passthrough_ commands - the
>> actual I/O commands dot not get the failfast flag.
> 
> Yes. I was thinking of the LightNVM-specific _passthrough_ used by
> liblightnvm since I'm refactoring the sync. path at the moment; the
> "normal" async. path will be untouched for now.

In fact, for the normal I/O path the whole idea is to do this directly
in pblk, when we use struct request, and drop all these conversions in
lightnvm.c. But that will be later on...

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2017-10-06 12:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-06  9:19 Why removing REQ_FAILFAST_DRIVER in LightNVM? Javier González
2017-10-06 11:59 ` Christoph Hellwig
2017-10-06 12:01   ` Javier González
2017-10-06 12:06     ` Christoph Hellwig
2017-10-06 12:08       ` Javier González
2017-10-06 12:12         ` Javier González

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.