All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
@ 2018-01-24 12:24 Minwoo Im
  2018-01-24 12:31 ` Johannes Thumshirn
  0 siblings, 1 reply; 7+ messages in thread
From: Minwoo Im @ 2018-01-24 12:24 UTC (permalink / raw)


It would be convenient to use abort command by a subcommand.
This command will strongly used by device developers to test device.

Minwoo Im (2):
  nvme-cli: add support for an abort command
  nvme-cli: add documentation for abort command

 Documentation/cmds-main.txt   |   2 +
 Documentation/nvme-abort.1    |  80 ++++
 Documentation/nvme-abort.html | 834 ++++++++++++++++++++++++++++++++++++++++++
 Documentation/nvme-abort.txt  |  51 +++
 nvme-builtin.h                |   1 +
 nvme-ioctl.c                  |  13 +
 nvme-ioctl.h                  |   1 +
 nvme.c                        |  50 +++
 8 files changed, 1032 insertions(+)
 create mode 100644 Documentation/nvme-abort.1
 create mode 100644 Documentation/nvme-abort.html
 create mode 100644 Documentation/nvme-abort.txt

-- 
2.7.4

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 12:24 [PATCH 0/2] nvme-cli: add support for an abort command with documentation Minwoo Im
@ 2018-01-24 12:31 ` Johannes Thumshirn
  2018-01-24 12:55   ` Minwoo Im
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Thumshirn @ 2018-01-24 12:31 UTC (permalink / raw)


On Wed, Jan 24, 2018@09:24:42PM +0900, Minwoo Im wrote:
> It would be convenient to use abort command by a subcommand.
> This command will strongly used by device developers to test device.

Ahm... please no.

If you need it for development just hack a small tool to create the
passthrough ioctl() call but don't hand out loaded guns to children^Wusers.

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 12:31 ` Johannes Thumshirn
@ 2018-01-24 12:55   ` Minwoo Im
  2018-01-24 13:03     ` Johannes Thumshirn
  0 siblings, 1 reply; 7+ messages in thread
From: Minwoo Im @ 2018-01-24 12:55 UTC (permalink / raw)


Hello Johannes,

On 01/24/2018 09:31 PM, Johannes Thumshirn wrote:
> On Wed, Jan 24, 2018@09:24:42PM +0900, Minwoo Im wrote:
>> It would be convenient to use abort command by a subcommand.
>> This command will strongly used by device developers to test device.
> 
> Ahm... please no.
> 
> If you need it for development just hack a small tool to create the
> passthrough ioctl() call but don't hand out loaded guns to children^Wusers.
> 

Thanks for your kindly comment.

I thought nvme-cli tool would be used mostly by developers, not children 
users. Also thought that it will not need any helps from kernel to abort 
a specified command not just like delete I/O SQ/CQ so that nvme-cli 
could handle this kind of thing.

But, actually there is no limit for anybody to use this interface tool.
I agree what you're saying. So, please skip it.

Thanks,

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 12:55   ` Minwoo Im
@ 2018-01-24 13:03     ` Johannes Thumshirn
  2018-01-24 13:15       ` Minwoo Im
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Thumshirn @ 2018-01-24 13:03 UTC (permalink / raw)


On Wed, Jan 24, 2018@09:55:20PM +0900, Minwoo Im wrote:
> Hello Johannes,
> 
> On 01/24/2018 09:31 PM, Johannes Thumshirn wrote:
> > On Wed, Jan 24, 2018@09:24:42PM +0900, Minwoo Im wrote:
> > > It would be convenient to use abort command by a subcommand.
> > > This command will strongly used by device developers to test device.
> > 
> > Ahm... please no.
> > 
> > If you need it for development just hack a small tool to create the
> > passthrough ioctl() call but don't hand out loaded guns to children^Wusers.
> > 
> 
> Thanks for your kindly comment.
> 
> I thought nvme-cli tool would be used mostly by developers, not children
> users. Also thought that it will not need any helps from kernel to abort a
> specified command not just like delete I/O SQ/CQ so that nvme-cli could
> handle this kind of thing.

nvme-cli will also be used by administrators intending to setup nvme over
fabrics connections or debug nvme multipathing as well. It is certainly not a
developer only tool.

> But, actually there is no limit for anybody to use this interface tool.

Yes, actually thinking more of it, the whole passthrough interface is
dangerous. What happens if a user sends a delete CQ/SQ command via this
interface and the kernel still tries to do I/O to it?

I think I'll need to come up with a testcase for this.

	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 13:03     ` Johannes Thumshirn
@ 2018-01-24 13:15       ` Minwoo Im
  2018-01-24 17:26         ` James Smart
  0 siblings, 1 reply; 7+ messages in thread
From: Minwoo Im @ 2018-01-24 13:15 UTC (permalink / raw)


On 01/24/2018 10:03 PM, Johannes Thumshirn wrote:
> On Wed, Jan 24, 2018@09:55:20PM +0900, Minwoo Im wrote:
>> Hello Johannes,
>>
>> On 01/24/2018 09:31 PM, Johannes Thumshirn wrote:
>>> On Wed, Jan 24, 2018@09:24:42PM +0900, Minwoo Im wrote:
>>>> It would be convenient to use abort command by a subcommand.
>>>> This command will strongly used by device developers to test device.
>>>
>>> Ahm... please no.
>>>
>>> If you need it for development just hack a small tool to create the
>>> passthrough ioctl() call but don't hand out loaded guns to children^Wusers.
>>>
>>
>> Thanks for your kindly comment.
>>
>> I thought nvme-cli tool would be used mostly by developers, not children
>> users. Also thought that it will not need any helps from kernel to abort a
>> specified command not just like delete I/O SQ/CQ so that nvme-cli could
>> handle this kind of thing.
> 
> nvme-cli will also be used by administrators intending to setup nvme over
> fabrics connections or debug nvme multipathing as well. It is certainly not a
> developer only tool.

Got it, Thanks for your kindly comment. :)

>> But, actually there is no limit for anybody to use this interface tool.
> 
> Yes, actually thinking more of it, the whole passthrough interface is
> dangerous. What happens if a user sends a delete CQ/SQ command via this
> interface and the kernel still tries to do I/O to it

Yes, That situation is one of the reason why I tried to add abort
command to nvme-cli because it does not need any helps from kernel.
As you said, delete CQ/SQ obviously needs helps from kernel to free
resources about queues.

Or.. Is there any cases that breaks behavior of kernel by an abort
command from user space?

Thanks,

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 13:15       ` Minwoo Im
@ 2018-01-24 17:26         ` James Smart
  2018-01-25 13:14           ` Minwoo Im
  0 siblings, 1 reply; 7+ messages in thread
From: James Smart @ 2018-01-24 17:26 UTC (permalink / raw)


On 1/24/2018 5:15 AM, Minwoo Im wrote:
> Or.. Is there any cases that breaks behavior of kernel by an abort
> command from user space?
>

Is it possible yes. Is it simple, no.?? Depends on what the command is 
and when.? You may confuse a transport which uses aborts - and you're 
also mandating the transport peek into the pass-through command to check 
for abort so that is can honor the # of outstanding aborts supported by 
the controller.

I agree with Johannes. this is an ugly thing to do . The first question 
I had is - how do you even know what the command and its CID value is ?? 
Also, many devices won't actually even do aborts.

-- james

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

* [PATCH 0/2] nvme-cli: add support for an abort command with documentation
  2018-01-24 17:26         ` James Smart
@ 2018-01-25 13:14           ` Minwoo Im
  0 siblings, 0 replies; 7+ messages in thread
From: Minwoo Im @ 2018-01-25 13:14 UTC (permalink / raw)


Hi James,

On 01/25/2018 02:26 AM, James Smart wrote:
> On 1/24/2018 5:15 AM, Minwoo Im wrote:
>> Or.. Is there any cases that breaks behavior of kernel by an abort
>> command from user space?
>>
> 
> Is it possible yes. Is it simple, no.?? Depends on what the command is 
> and when.? You may confuse a transport which uses aborts - and you're 
> also mandating the transport peek into the pass-through command to check 
> for abort so that is can honor the # of outstanding aborts supported by 
> the controller.
> 
> I agree with Johannes. this is an ugly thing to do . The first question 
> I had is - how do you even know what the command and its CID value is ? 
> Also, many devices won't actually even do aborts.
> 
> -- james
> 

I totally agree with you. I really appreciate your comments.
I thought abort command has nothing to do with behavior of kernel, but
CID is assigned by kernel when issuing an command to device.

Thanks again.

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

end of thread, other threads:[~2018-01-25 13:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 12:24 [PATCH 0/2] nvme-cli: add support for an abort command with documentation Minwoo Im
2018-01-24 12:31 ` Johannes Thumshirn
2018-01-24 12:55   ` Minwoo Im
2018-01-24 13:03     ` Johannes Thumshirn
2018-01-24 13:15       ` Minwoo Im
2018-01-24 17:26         ` James Smart
2018-01-25 13:14           ` Minwoo Im

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.