All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: alan.adamson@oracle.com
Cc: Sagi Grimberg <sagi@grimberg.me>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	"kbusch@kernel.org" <kbusch@kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH V8 1/1] nvme: allow passthru cmd error logging
Date: Wed, 24 Jan 2024 10:06:16 +0100	[thread overview]
Message-ID: <20240124090616.GC27760@lst.de> (raw)
In-Reply-To: <718a0a43-40b2-41a3-98d5-dc2eebb41681@oracle.com>

On Tue, Jan 23, 2024 at 09:11:32AM -0800, alan.adamson@oracle.com wrote:
>>> This allows us to get the ctrl or ns object associated with the struct
>>> device
>>> we get in the sysfs, then based on the device class we update
>>> logging_enabled
>>> flags for either ctrl or ns respectively. In nvme_init_request() I use
>>> ctrl->logging_enabled and ns->logging_enabled based on admin or io cmd.
>>
>> I was asking why should we have a show/store that operate on both ns and
>> ctrl?
>>
>> Why not have a show/store in nvme_dev_attrs and a separate one in
>> nvme_ns_id_attrs ? Then you don't need the awkward is_nvme_class() ?
>> the ns attrs can access the ns in a normal way like the rest? Or am
>> I missing something?
>
> I did have a version that used nvme_ns_id_attrs but didn't think it was an 
> appropriate place for it.  I can go back to that.

nvme_ns_id_attrs has been renamed now that we're using it for more than
just ids, so adding more attributes if they fit is fine.


  parent reply	other threads:[~2024-01-24  9:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-11  0:08 [PATCH V8 0/1] nvme: allow passthru cmd error logging Alan Adamson
2024-01-11  0:08 ` [PATCH V8 1/1] " Alan Adamson
2024-01-11  7:04   ` Christoph Hellwig
2024-01-17  3:46     ` Chaitanya Kulkarni
2024-01-17 14:14       ` Sagi Grimberg
2024-01-18  3:31         ` Chaitanya Kulkarni
2024-01-23 11:33           ` Sagi Grimberg
2024-01-23 17:11             ` alan.adamson
2024-01-24  3:41               ` Chaitanya Kulkarni
2024-01-24 17:10                 ` alan.adamson
2024-01-24  9:06               ` Christoph Hellwig [this message]
2024-01-24  3:37             ` Chaitanya Kulkarni
2024-01-25  0:52             ` alan.adamson
2024-01-29 10:24               ` Sagi Grimberg
2024-01-16  6:33   ` Chaitanya Kulkarni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240124090616.GC27760@lst.de \
    --to=hch@lst.de \
    --cc=alan.adamson@oracle.com \
    --cc=chaitanyak@nvidia.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.